You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Matthew Jacobs (Code Review)" <ge...@cloudera.org> on 2017/02/16 00:52:26 UTC

[Impala-ASF-CR] IMPALA-4933: Force thrift to initialize SSL on process startup

Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-4933: Force thrift to initialize SSL on process startup
......................................................................

IMPALA-4933: Force thrift to initialize SSL on process startup

OpenSSL initialization functions are not threadsafe and are
currently called by both thrift, squeasel, and the Kudu
client. This change forces thrift to initialize OpenSSL on
startup by adding a TSSLSocketFactory to the AuthManager
which initializes OpenSSL upon creation and lives the
lifetime of the process. Then, squeasel is configured to
skip the OpenSSL initialization.

TODO: When the Kudu client supports a flag to disable its
initialization path (KUDU-1738), Impala will call that. In
the meantime, there will continue to be some small
likelihood of a race.

Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
---
M be/src/rpc/authentication.cc
M be/src/rpc/authentication.h
M be/src/util/webserver.cc
3 files changed, 18 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2: Code-Review+2

Assuming that Henry's comment implied a +2 after Sailesh reviewed it. Henry, feel free to -1 if that is incorrect.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has uploaded a new patch set (#2).

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................

IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

OpenSSL initialization functions are not threadsafe and are
currently called by both thrift, squeasel, and the Kudu
client. This change forces thrift to initialize OpenSSL on
startup by adding a TSSLSocketFactory to the AuthManager
which initializes OpenSSL upon creation and lives the
lifetime of the process. Then, squeasel is configured to
skip the OpenSSL initialization.

TODO: When the Kudu client supports a flag to disable its
initialization path (KUDU-1738), Impala will call that. In
the meantime, there will continue to be some small
likelihood of a race.

Also updates Squeasel in thirdparty to
c304d3f3481b07bf153979155f02e0aab24d01de
This is necessary to configure squeasel not to init OpenSSL.

Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
---
M be/src/rpc/authentication.cc
M be/src/rpc/authentication.h
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/CMakeLists.txt
M be/src/util/webserver.cc
5 files changed, 54 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/6027/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6027
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2:

private job succeeded

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933: Force thrift to initialize SSL on process startup

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4933: Force thrift to initialize SSL on process startup
......................................................................


Patch Set 1:

Running jenkins job now, will also deploy to a secure cluster for additional testing

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2: Code-Review+1

lgtm - can you get Sailesh to check as well?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2: Code-Review+2

That's what I meant, we're good to go.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2: Code-Review+1

Can't +2 because I'm not a component owner. So someone can upgrade.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

OpenSSL initialization functions are not threadsafe and are
currently called by both thrift, squeasel, and the Kudu
client. This change forces thrift to initialize OpenSSL on
startup by adding a TSSLSocketFactory to the AuthManager
which initializes OpenSSL upon creation and lives the
lifetime of the process. Then, squeasel is configured to
skip the OpenSSL initialization.

TODO: When the Kudu client supports a flag to disable its
initialization path (KUDU-1738), Impala will call that. In
the meantime, there will continue to be some small
likelihood of a race.

Also updates Squeasel in thirdparty to
c304d3f3481b07bf153979155f02e0aab24d01de
This is necessary to configure squeasel not to init OpenSSL.

Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Reviewed-on: http://gerrit.cloudera.org:8080/6027
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/rpc/authentication.cc
M be/src/rpc/authentication.h
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/CMakeLists.txt
M be/src/util/webserver.cc
5 files changed, 54 insertions(+), 29 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved
  Matthew Jacobs: Looks good to me, approved
  Sailesh Mukil: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4933, IMPALA-4931: Simplify SSL initialization on startup
......................................................................


Patch Set 2:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/280/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a8a001103ddca7f07349faa82bbb5b9fe3ab0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No