Jiyoung Yoo has posted comments on this change. ( http://gerrit.cloudera.org:8080/24786 )
Change subject: IMPALA-15257: Add option to use keepalive for internal cluster Thrift connections ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/24786/4/be/src/rpc/thrift-util.h File be/src/rpc/thrift-util.h: http://gerrit.cloudera.org:8080/#/c/24786/4/be/src/rpc/thrift-util.h@37 PS4, Line 37: DECLARE_int32(internal_keepalive_probe_period_s); > I think it's a little unusual for us to add declarations to a header file. I moved them all to the .cc files. http://gerrit.cloudera.org:8080/#/c/24786/4/tests/custom_cluster/test_internal_keepalive.py File tests/custom_cluster/test_internal_keepalive.py: http://gerrit.cloudera.org:8080/#/c/24786/4/tests/custom_cluster/test_internal_keepalive.py@34 PS4, Line 34: SS_POLL_ATTEMPTS = 20 > Might be worth a run with asan to make sure this is long enough for slow te I verified that all tests pass with the ASAN build. http://gerrit.cloudera.org:8080/#/c/24786/4/tests/custom_cluster/test_internal_keepalive.py@41 PS4, Line 41: Thrift connections. The impalad<->statestored connection (port24000) is a > A space between port24000 might be nice. Done http://gerrit.cloudera.org:8080/#/c/24786/4/tests/custom_cluster/test_internal_keepalive.py@73 PS4, Line 73: # The disabled case must be quite across the whole window. > quite -> quiet? Done -- To view, visit http://gerrit.cloudera.org:8080/24786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9fec7a02b2c5ef92e8b68de9e3993bcee7bc898 Gerrit-Change-Number: 24786 Gerrit-PatchSet: 4 Gerrit-Owner: Jiyoung Yoo <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jiyoung Yoo <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 08 Sep 2026 17:15:33 +0000 Gerrit-HasComments: Yes
