impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC
Date Tue, 26 Sep 2017 18:11:25 GMT
Sailesh Mukil has posted comments on this change. ( )

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

Patch Set 1:

File be/src/runtime/
PS1, Line 198: AddData
Isn't the point of the deserialize pool to deserialize the payload early?
Here, we're just calling AddData() on the payloads for early senders after the corresponding
receiver has been created.
File be/src/runtime/
PS1, Line 165: Either we'll consume a row batch from batch_queue_, or it's empty
Shouldn't there always be something in the batch_queue_ if there's something in the blocked_senders_
list? Since we fill the blocked_senders_ only if the queue is at its limit.

And we also logically service the batches from batch_queue_ first before servicing the batches
from the blocked_senders_ list.
PS1, Line 166: There is a window
Just to make things clearer, could you specify what there's a window for?
PS1, Line 225: 
There is a problem here. When we release lock_here, an arbitrary number of senders could call
AddBatch(), and all their batches would get enqueued even though the ExceedsLimit() would
be true. This breaks the guarantee of the queue not being over committed more than a single
PS1, Line 284:   for (const auto& queue_entry: batch_queue_) delete queue_entry.second;
batch_queue_.clear() ?

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0b8c1e50678da66ab1547d16530f88b323ed8c1
Gerrit-Change-Number: 8023
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Comment-Date: Tue, 26 Sep 2017 18:11:25 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message