impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue
Date Wed, 28 Sep 2016 16:06:52 GMT
Dan Hecht has posted comments on this change.

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

Patch Set 9:


Is there a new patchset that addresses Juan's change?
File be/src/util/blocking-queue.h:

PS7, Line 171: 
> Yes, this can be split into SizeLocked() which requires "put_lock_" be held
Right, I understand that, but that doesn't mean Size() should return any random value.  The
assumption that mpalaServer::MembershipCallback(), for example, seems valid.  Holding the
write lock does solve this though since it protects against the swap which is when the double
counting (or zero counting) can occur.
File be/src/util/blocking-queue.h:

PS9, Line 188: read racily
that's not true -- the atomic means it's not a "racy read".  I think what you mean is the
get and put sizes are not read atomically together?

PS9, Line 201: mutable
why do we need these mutable?  it wouldn't make sense to declare a 'const BlockingQueue' (and
it wouldn't work since a lot of these other fields would need to be mutable as well).

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 9
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: Juan Yu <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Mostafa Mokhtar <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message