Henry Robinson has posted comments on this change. Change subject: IMPALA-4856: Port data stream service to KRPC ......................................................................
Patch Set 1: (10 comments) http://gerrit.cloudera.org:8080/#/c/7103/1/be/src/runtime/data-stream-mgr.cc File be/src/runtime/data-stream-mgr.cc: Line 61: bool unused = false; > Doesn't the contract for FindRecvr() state that we need to hold 'lock_' bef Done Line 105: EarlySendersList waiters; > Add brief comment: Done PS1, Line 123: for (int32_t sender_id: waiters.closing_senders) recvr->RemoveSender(sender_id); > According to the header comment in data-stream-mgr.h, a sender shouldn't be Done PS1, Line 300: early_senders_ > Assume the following case: The sender fragment instance would fail, and then the coordinator should cancel the receiver. I believe there's an outstanding issue where, if the coordinator fails to cancel a fragment instance, the fragment instance will not fail itself. I'm going to file a JIRA for that, but it's unrelated to KRPC. Line 321: // Wait for 10s > Add a brief comment stating that this is to check if the DataStreamMgr is b Done http://gerrit.cloudera.org:8080/#/c/7103/1/be/src/runtime/data-stream-mgr.h File be/src/runtime/data-stream-mgr.h: PS1, Line 81: will do one of three things > nit: would be nice to format them as bullet points. Done PS1, Line 83: if the buffer is full > "if the batch queues are full"? Done PS1, Line 87: the sender > "the sender along with its payload" ? Done Line 224: /// has not yet prepared 'payload' is queued until it arrives, or is timed out. If the > nit: been prepared, Done http://gerrit.cloudera.org:8080/#/c/7103/1/be/src/service/impala-server.h File be/src/service/impala-server.h: PS1, Line 255: void UpdateFilter > Leave a TODO stating that this should move to query-state.h/cc after IMPALA I'm going to leave that for now, since I don't want to make design decisions for IMPALA-3825 in this patch. -- To view, visit http://gerrit.cloudera.org:8080/7103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia66704be7a0a8162bb85556d07b583ec756c584b Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
