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 <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Wed, 01 Apr 2026 16:13:16 +0000 Gerrit-HasComments: Yes
