impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5129: Use Kudu's Kinit code to avoid expensive fork
Date Thu, 19 Oct 2017 20:33:41 GMT
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/7938 )

Change subject: IMPALA-5129: Use Kudu's Kinit code to avoid expensive fork
......................................................................


Patch Set 9:

(2 comments)

> > (1 comment)
 > 
 > Manually setting Verified: -1.
 > 
 > Running the test in a loop shows that it's flaky. Not a functional
 > issue, but something to do with setting up and tearing down the KDC
 > so often. Or possibly a test infra issue because our renew threads
 > don't have a shutdown mechanism.
 > 
 > Will work on fixing it.

On more investigation, it looks like the cause of this problem is that our security code (and
Kudu's security code) does not have a shutdown mechanism. So, Init-ing the security library
multiple times can have undefined behavior, since every time it is init-ed, a new renewal
thread runs in the background and is never stopped, and these renewal threads have access
to common state. Our security code is written with the assumption that it will be init-ed
only once, which holds true for live clusters, but not for test purposes we aim to do in this
patch.

I've opened IMPALA-6085 to track this to fix it in the long term. In the short term, I've
changed the code to using kerberos only for one test per process. (in this case, the SslConnectivity
test).

Documenting the other things I tried to fix this:
- Start and stop the KDC in the main() function: The problem is that we can't run it with
and without kerberos in this case. Also, Gtest doesn't guarantee calling RUN_ALL_TESTS() twice
will work, if we want to run with and without kerberos.

- Keep around some extra global state to start and stop the KDC according to the test we're
in. This is possible, but is a big hack, and I'm against checking that kind of code in.

http://gerrit.cloudera.org:8080/#/c/7938/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/7938/8//COMMIT_MSG@7
PS8, Line 7: Kudu
> Kudu
Done


http://gerrit.cloudera.org:8080/#/c/7938/8/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/7938/8/be/src/rpc/authentication.cc@109
PS8, Line 109: // (IMPALA-5893)
> can you file a jira to remove this flag (and the old code) within a release
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cea56cc6e7412d87f4c2e92399a2f91ea6af6c7
Gerrit-Change-Number: 7938
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Oct 2017 20:33:41 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message