Tim Armstrong has posted comments on this change. Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue. ......................................................................
Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4350/3/be/src/util/blocking-queue.h 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 http://gerrit.cloudera.org:8080/4350 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Chen Huang <paulhuan...@utexas.edu> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
