Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/12672 )
Change subject: IMPALA-8143: Enhance DoRpcWithRetry(). ...................................................................... Patch Set 12: (5 comments) Thanks Michael, I believe I addressed the nits. All tests pass. http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.h File be/src/rpc/impala-service-pool.h: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.h@72 PS11, Line 72: char* > char* Done http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.cc File be/src/rpc/impala-service-pool.cc: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.cc@192 PS11, Line 192: UNLIKELY(!debug_st > UNLIKELY(!debug_status.ok()) Done http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.h File be/src/rpc/rpc-mgr-test.h: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.h@281 PS11, Line 281: GetNumberOfCalls() > nit: number_of_calls() or GetNumberOfCalls() Done http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.cc File be/src/rpc/rpc-mgr-test.cc: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.cc@331 PS11, Line 331: // DoRpcWithRetry will fail with probability (1/2)^num_rpc_retry_calls. : ASSERT_TRUE(status.ok()); : } : // There will be no overflows (i.e. service too busy) with probability : // (1/2)^num_retries. > :-). :-) Thanks for thinking about this! http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr.h File be/src/rpc/rpc-mgr.h: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr.h@183 PS11, Line 183: /// Pass 'debug_action' to DebugAction() to potentially inject errors. > Please add a TODO: Done -- To view, visit http://gerrit.cloudera.org:8080/12672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390 Gerrit-Change-Number: 12672 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Sherman <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Fri, 07 Jun 2019 16:49:04 +0000 Gerrit-HasComments: Yes
