impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC
Date Mon, 12 Jun 2017 23:46:16 GMT
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4856: Port data stream service to KRPC

Patch Set 1:

File be/src/runtime/

Line 61:           bool unused = false;
> Doesn't the contract for FindRecvr() state that we need to hold 'lock_' bef

Line 105:   EarlySendersList waiters;
> Add brief comment:

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

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
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.

PS1, Line 83: if the buffer is full
> "if the batch queues are full"?

PS1, Line 87: the sender
> "the sender along with its payload" ?

Line 224:   /// has not yet prepared 'payload' is queued until it arrives, or is timed out.
If the
> nit: been prepared,
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66704be7a0a8162bb85556d07b583ec756c584b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-HasComments: Yes

View raw message