[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly

2018-03-11 Thread Bharath Vissapragada (Code Review) 
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9506 )

Change subject: IMPALA-6610: Impala shell fetches the value of 
ldap_password_cmd incorrectly
..


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/9506/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9506/6//COMMIT_MSG@10
PS6, Line 10:
Update commit message


http://gerrit.cloudera.org:8080/#/c/9506/6/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9506/6/shell/impala_shell.py@1541
PS6, Line 1541:
..contains a trailing


http://gerrit.cloudera.org:8080/#/c/9506/6/shell/impala_shell.py@1541
PS6, Line 1541: Ldap
nit:LDAP


http://gerrit.cloudera.org:8080/#/c/9506/6/shell/impala_shell.py@1542
PS6, Line 1542: echo
nit: Maybe single quote 'echo' and 'echo -n' ?


http://gerrit.cloudera.org:8080/#/c/9506/6/shell/impala_shell.py@1539
PS6, Line 1539: if options.ldap_password_cmd.strip().startswith('echo') and \
  :  options.ldap_password.endswith('\n'):
  : print_to_stderr("Warning: Ldap password contains 
trailing newline. "
  : "Did you use echo instead of echo -n?")
Should we do this only if the LDAP auth fails? Otherwise, it'd be an 
unnecessary warning.


http://gerrit.cloudera.org:8080/#/c/9506/6/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/9506/6/shell/option_parser.py@202
PS6, Line 202:  "which does not contain trailing 
newline.")
Don't think we can dictate this and it depends on the LDAP implementation. 
Maybe not mention this at all?



--
To view, visit http://gerrit.cloudera.org:8080/9506
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 6
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Mon, 12 Mar 2018 05:50:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly

2018-03-11 Thread Donghui Xu (Code Review) 
Hello Bharath Vissapragada, Fredy Wijaya, Alex Behm,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9506

to look at the new patch set (#6).

Change subject: IMPALA-6610: Impala shell fetches the value of 
ldap_password_cmd incorrectly
..

IMPALA-6610: Impala shell fetches the value of ldap_password_cmd incorrectly

The value of LDAP password in Impala shell contains extra line break.

I fixed the issue by removing extraneous '\n' in the LDAP password.

Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
---
M shell/impala_shell.py
M shell/option_parser.py
2 files changed, 6 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/9506/6
--
To view, visit http://gerrit.cloudera.org:8080/9506
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 6
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

2018-03-11 Thread David Knupp (Code Review) 
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9494/11/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/11/tests/run-tests.py@112
PS11, Line 112: d on the outcome of test run when no tests
> Patch Set 12:
>
> (7 comments)

Since you didn't include your command line, it's not clear what tests you were 
running, so I'm not sure what to make of your debug output. But here's an 
illustration of what I was trying to get at.

I've added a dummy test that simply asserts True (called "test_always_passes") 
to one of our test modules. Because I didn't mark it as a serial test, 
run-tests.py should find this test when it tries to run the parallel tests.

BEFORE your patch, when I try to run just this test in isolation, run-tests.py 
finds nothing in the serial tests, stress, or custom cluster tests (as 
expected), but it does successfully find and execute the test in the parallel 
test run.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_always_passes
   test session starts =
  [...]
  scheduling tests via LoadScheduling

  shell/test_shell_commandline.py::test_always_passes
  [gw1] PASSED shell/test_shell_commandline.py::test_always_passes
  generated xml file: /Impala/logs/ee_tests/results/TEST-impala-parallel.xml
  == 1 passed in 1.22 seconds ==


When I query the run-tests.py exit code, it should be zero, however:

  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  1  # < This is a bug. Everything ran as expected, and passed.


For a second use case, I'll try to run a test that I know is NOT a valid test.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_does_not_exist
  [...no tests are found in any group, obviously...]


Now when I query the exit code, it should show that there was an error, and it 
does.

  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  1  # < This is correct. We tried a non-existent test, so we should 
exit with an error.


AFTER your patch, when I try the same experiment, I  can see that the first 
issue has indeed been corrected.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_always_passes
   test session starts =
  [...]
  scheduling tests via LoadScheduling

  shell/test_shell_commandline.py::test_always_passes
  [gw1] PASSED shell/test_shell_commandline.py::test_always_passes
generated xml file: /Impala/logs/ee_tests/results/TEST-impala-parallel.xml
  == 1 passed in 1.22 seconds ==
  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  0  # < This is correct. We found the test, ran it, and it passed.


However, now there's a problem in the second use case. If I try to run an 
invalid test, run-tests.py tells me that everything is OK.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_does_not_exist
  [...no tests are found in any group, obviously...]
   no tests ran in 1.22 seconds 
  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  0  # < This is a new bug. Exit code should not be zero in this case.


This is also what I was trying to get at with the earlier suggestion that the 
pytest exit code and the overall run-tests.py exit code are different things. 
The pytest exit status is evaluated after each invocation of pytest. The 
overall run-tests.py exit code can only be evaluated at the very end, after 
we've gone through all of our pytest attempts.

I'm sorry that I didn't make this more clear before.



--
To view, visit http://gerrit.cloudera.org:8080/9494
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 12
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Sun, 11 Mar 2018 23:51:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention

2018-03-11 Thread Joe McDonnell (Code Review) 
Joe McDonnell has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9576


Change subject: IMPALA-6638: Reduce file handle cache lock contention
..

IMPALA-6638: Reduce file handle cache lock contention

FileHandleCache::OpenFileHandle() currently holds the
lock while opening a file handle. This lengthens the
duration holding the lock considerably, causing
contention when a lot of file handles are being
opened (i.e. when the cache is cold).

This changes FileHandleCache::OpenFileHandle() drops
the lock while opening the file handle, then
reacquires it to add the file handle to the cache.

Performance tests confirm that this fixes contention
when the file handle cache is cold. It does not change
performance when the cache is hot.

Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d
---
M be/src/runtime/io/handle-cache.inline.h
1 file changed, 28 insertions(+), 24 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/9576/1
--
To view, visit http://gerrit.cloudera.org:8080/9576
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d
Gerrit-Change-Number: 9576
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell