impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue.
Date Thu, 15 Sep 2016 20:34:36 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue.

Patch Set 3:

(1 comment)
File be/src/util/blocking-queue.h:

Line 99:   bool BlockingPut(const T& val) {
> Yes, I have thought about it before. I have tested various locations for no
We discussed this a little offline and I feel like I understand why option (3) works in practice
now - when N producers are outpacing consumers, this switches to a mode where the steady state
is a nearly empty queue with k producers sitting blocked and N - k producers working. Essentially
there are some additional elements in the queue because the blocked producers also have some
row batches ready to add to the queue immediately.

I'm ok moving forward with it so long as we document the expected behaviour. Perhaps it's
adequate to document the expect behaviour with a faster consumer, matched producer/consumer
and fast producers.

Have you benchmarked this with concurrent queries or workloads with many concurrent scans?
One concern with the empty queue is that if the queue is empty and a producer isn't scheduled
immediately, the consumer may end up waiting on the condition variable for every batch and
potentially getting swapped it. We could perhaps work around that if the producer added its
item to the queue *before* blocking, so that the the consumer can get the item without requiring
a context switch.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Chen Huang <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Mostafa Mokhtar <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message