Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10327 )
Change subject: IMPALA-6907: Close stale connections to removed cluster members ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/10327/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10327/3//COMMIT_MSG@25 PS3, Line 25: > Can you also add a comment stating the changes to impala_cluster.py? Done http://gerrit.cloudera.org:8080/#/c/10327/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/10327/3/be/src/service/impala-server.cc@1541 PS3, Line 1541: exec_env_->impalad_client_cache()->CloseConnections(entry->second.address); > nit: Add comment about closing connections. Done http://gerrit.cloudera.org:8080/#/c/10327/3/tests/custom_cluster/test_restart_services.py File tests/custom_cluster/test_restart_services.py: http://gerrit.cloudera.org:8080/#/c/10327/3/tests/custom_cluster/test_restart_services.py@57 PS3, Line 57: def test_restart_impala(self): > There's no part in this test that's actually validating that the connection Yes, the way the client cache code is structured makes it hard to discern whether the stale connections get flushed because our client cache code will pattern-match against the case when read() returns EOF. This test is just adding coverage for restarting Impalad. I manually confirmed the code is being exercised with the extra log statement. Please feel free to suggest any other ideas which can help differentiate whether the stale connections get flushed. http://gerrit.cloudera.org:8080/#/c/10327/3/tests/custom_cluster/test_restart_services.py@72 PS3, Line 72: # Sleep for a bit for the statestore change in membership to propagate > nit: Could you add the lower bound time it would take the statestore to pro 100ms for prioritized updates but I am putting 3 seconds just in case things are really slow on ASAN builds. -- To view, visit http://gerrit.cloudera.org:8080/10327 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I41b7297cf665bf291b09b23524d19b1d10ab281d Gerrit-Change-Number: 10327 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Comment-Date: Wed, 09 May 2018 01:57:42 +0000 Gerrit-HasComments: Yes
