Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22462 )

Change subject: IMPALA-13687: Support shared secret key for cookies
......................................................................


Patch Set 14:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22462/14/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/22462/14/be/src/util/openssl-util.h@164
PS14, Line 164:   /// Destructor to shut down the reload thread and signal it 
to stop.
              :   ~AuthenticationHashFromFile() override {
              :     // Signal the reload thread to stop.
              :     if (stop_pipe_write_fd_ >= 0) {
              :       write(stop_pipe_write_fd_, "x", 1); // Write a byte to 
signal shutdown.
              :       close(stop_pipe_write_fd_);
              :     }
              :     if (reload_thread_.joinable()) {
              :       reload_thread_.join();
              :     }
              :   }
> Do some of our backend tests exercise this code? I'm wondering if it is wor
Several of the frontend custom-cluster tests exercise some part of this, such 
as 
https://gerrit.cloudera.org/c/22462/14/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java#1053.

Maybe need to make sure we use graceful shutdown and it actually exits cleanly, 
rather than requiring a forced stop? This probably could be exercised in a unit 
test too.


http://gerrit.cloudera.org:8080/#/c/22462/14/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:

http://gerrit.cloudera.org:8080/#/c/22462/14/be/src/util/openssl-util.cc@358
PS14, Line 358:         if (Status stat = LoadKey(); !stat.ok()) {
              :           LOG(ERROR) << "Failed to reload authentication key: " 
<< stat;
> If something overwrites the key with an invalid file, the current key is ke
Yeah, those make sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2e2345f771608069407e9dcf7ed4697fc0214e7
Gerrit-Change-Number: 22462
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Wed, 26 Nov 2025 22:40:03 +0000
Gerrit-HasComments: Yes

Reply via email to