[kudu-CR] KUDU-3755 tool: add leave files to control shell

["Alexey Serbin (Code Review)" <ge...@cloudera.org>]

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24122 )

Change subject: KUDU-3755 tool: add leave_files to control shell
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/24122/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/24122/3/src/kudu/tools/kudu-tool-test.cc@8686
PS3, Line 8686: SIGKILL
I'm curious: is SIGKILL deliberately chosen to avoid running signal handlers in the shell process?  If there is a change in the behavior if using SIGTERM or SIGINT instead of SIGKILL here, consider adding a comment about choice of the signal.


http://gerrit.cloudera.org:8080/#/c/24122/3/src/kudu/tools/kudu-tool-test.cc@8697
PS3, Line 8697:   // Shell is already dead; prevent TearDown from waiting on it again.
              :   proto_.reset();
              :   shell_.reset();
Does it make sense to move these into an auto-cleanup scope to run even if any of ASSERT_TRUE() and ASSERT_OK() above triggered?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa09d63d614190d25cb9b1c6706fd0db992eb5eb
Gerrit-Change-Number: 24122
Gerrit-PatchSet: 3
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Wed, 01 Apr 2026 16:13:16 +0000
Gerrit-HasComments: Yes