Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10440 )
Change subject: IMPALA-5384, part 2: Simplify Coordinator locking and clarify state ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/10440/1/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/10440/1/be/src/runtime/coordinator.h@351 PS1, Line 351: Cannot finalize execution until exec RPCs are no longer : /// being sent. > this comment is new We added a DCHECK, but this comment suggests that this method will ensure that finalization fails(like returns a non-ok status) if exec RPCs are pending. we should either move the wait() call to this method or move this comment to UpdateBackendExecStatus() and update the one for this to say that "It should not be called if exec RPCs are pending" http://gerrit.cloudera.org:8080/#/c/10440/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/10440/1/be/src/runtime/coordinator.cc@530 PS1, Line 530: // TODO: IMPALA-7011 is this needed? This will also happen on the "backend" side of : // cancel rpc. And in the case of EOS, sink already knows this. I think we can update this comment, since we figured out that we do need it. http://gerrit.cloudera.org:8080/#/c/10440/1/be/src/runtime/coordinator.cc@695 PS1, Line 695: exec_rpcs_complete_barrier_->Wait(); should we be concerned about adding a possible long wait() call on an RPC callback? -- To view, visit http://gerrit.cloudera.org:8080/10440 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c Gerrit-Change-Number: 10440 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 17 May 2018 21:48:26 +0000 Gerrit-HasComments: Yes
