You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2020/11/09 22:35:01 UTC

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization 

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16704


Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................

IMPALA-6861: Fix OpenSSL initialization

Impalads currently initialize OpenSSL twice: once when initializing
Thrift and once when initializing KRPC. The initialization is
theoretically idempotent but not thread-safe, so its better to clean
this up.

This patch disables the Thrift version as its older (last updated in
2015) and the KRPC version contains logic specific to more recent
versions of OpenSSL. The catalogd and statestored also now use the
KRPC version instead of the Thrift version.

It also improves debuggability by adding some additional startup
logging.

Testing:
- Passed existing SSL tests.

Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
---
M be/src/rpc/authentication.cc
M be/src/rpc/authentication.h
M be/src/rpc/thrift-server.cc
3 files changed, 11 insertions(+), 10 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

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

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................

IMPALA-6861: Fix OpenSSL initialization

Impalads currently initialize OpenSSL twice: once when initializing
Thrift and once when initializing KRPC. The initialization is
theoretically idempotent but not thread-safe, so its better to clean
this up.

This patch disables the Thrift version as its older (last updated in
2015) and the KRPC version contains logic specific to more recent
versions of OpenSSL. The catalogd and statestored also now use the
KRPC version instead of the Thrift version.

It also improves debuggability by adding some additional startup
logging.

Testing:
- Passed existing SSL tests.

Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Reviewed-on: http://gerrit.cloudera.org:8080/16704
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/authentication.cc
M be/src/rpc/authentication.h
M be/src/rpc/thrift-server.cc
3 files changed, 11 insertions(+), 10 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7624/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Nov 2020 22:57:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 1: Code-Review+2

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/16704/1/be/src/rpc/authentication.cc@1116
PS1, Line 1116: initilalize
Typos: initialize/initialization


http://gerrit.cloudera.org:8080/#/c/16704/1/be/src/rpc/authentication.cc@1119
PS1, Line 1119: Initiailed
typo


http://gerrit.cloudera.org:8080/#/c/16704/1/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/16704/1/be/src/rpc/thrift-server.cc@314
PS1, Line 314: intiailized
initialized



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Nov 2020 19:55:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 2: Code-Review+2

(2 comments)

carrying forward

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

http://gerrit.cloudera.org:8080/#/c/16704/1/be/src/rpc/authentication.cc@1116
PS1, Line 1116: initialize 
> Typos: initialize/initialization
Done


http://gerrit.cloudera.org:8080/#/c/16704/1/be/src/rpc/authentication.cc@1119
PS1, Line 1119: Initialize
> typo
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Nov 2020 18:56:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................

IMPALA-6861: Fix OpenSSL initialization

Impalads currently initialize OpenSSL twice: once when initializing
Thrift and once when initializing KRPC. The initialization is
theoretically idempotent but not thread-safe, so its better to clean
this up.

This patch disables the Thrift version as its older (last updated in
2015) and the KRPC version contains logic specific to more recent
versions of OpenSSL. The catalogd and statestored also now use the
KRPC version instead of the Thrift version.

It also improves debuggability by adding some additional startup
logging.

Testing:
- Passed existing SSL tests.

Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
---
M be/src/rpc/authentication.cc
M be/src/rpc/authentication.h
M be/src/rpc/thrift-server.cc
3 files changed, 11 insertions(+), 10 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6650/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Nov 2020 19:07:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Nov 2020 00:37:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Nov 2020 19:07:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6861: Fix OpenSSL initialization

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16704 )

Change subject: IMPALA-6861: Fix OpenSSL initialization
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7640/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Gerrit-Change-Number: 16704
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Nov 2020 19:05:43 +0000
Gerrit-HasComments: No