Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20514 )
Change subject: KUDU-3507 Fix mini ranger port detection ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/20514/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20514/2//COMMIT_MSG@10 PS2, Line 10: lsof might handle 0.0.0.0 as ipv6 I'm not sure I understand how lsof is involved here: the address patterns provided for utilities like WaitForBindAtPort() isn't passed to lsof in any way, but instead is used in the code to find matching entries in the implementation of WaitForBind() and WaitForBindAtPort(). Are you sure that's the actual reason behind? http://gerrit.cloudera.org:8080/#/c/20514/2//COMMIT_MSG@25 PS2, Line 25: If for some : reason an other port is opened What might be the reason for opening some other port instead of the configured one? http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/ranger-kms/mini_ranger_kms.cc File src/kudu/ranger-kms/mini_ranger_kms.cc: http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/ranger-kms/mini_ranger_kms.cc@280 PS2, Line 280: RETURN_NOT_OK(WaitForTcpBind( : process_->pid(), : &port_, : adresses, : MonoDelta::FromSeconds(0))); If the process opens multiple (at least two) ports, how do we know that we are about to capture the API port here? http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/util/test_util.h File src/kudu/util/test_util.h: http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/util/test_util.h@188 PS2, Line 188: int pid = -1 style nit here and elsewhre: put the extra parameter into into its own line http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/util/test_util.h@194 PS2, Line 194: int pid = -1 nit: could it be std::optional<pid_t>, so no special values are introduced? -- To view, visit http://gerrit.cloudera.org:8080/20514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89c3409e9efd53a821a6d4244a35564e134323bb Gerrit-Change-Number: 20514 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Martonka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 11 Oct 2023 03:33:09 +0000 Gerrit-HasComments: Yes
