Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15324 )
Change subject: util: deadline for BlockingQueue::Blocking{Put,Get}
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15324/4/src/kudu/subprocess/server.cc
File src/kudu/subprocess/server.cc:
http://gerrit.cloudera.org:8080/#/c/15324/4/src/kudu/subprocess/server.cc@292
PS4, Line 292:
> It's only called in one place; maybe just inline it?
Done
http://gerrit.cloudera.org:8080/#/c/15324/4/src/kudu/util/blocking_queue-test.cc
File src/kudu/util/blocking_queue-test.cc:
http://gerrit.cloudera.org:8080/#/c/15324/4/src/kudu/util/blocking_queue-test.cc@108
PS4, Line 108: });
> If this fires, join() is never called. Should use SCOPED_CLEANUP to avoid.
Done
http://gerrit.cloudera.org:8080/#/c/15324/4/src/kudu/util/blocking_queue.h
File src/kudu/util/blocking_queue.h:
http://gerrit.cloudera.org:8080/#/c/15324/4/src/kudu/util/blocking_queue.h@187
PS4, Line 187: // - OK if successful
: // - TimedOut if the deadline passed
: //
> The other operations don't check the timeout up front. Should they? Why the
I copied the exiting behavior of BlockingDrainTo() w.r.t. BlockingGet(), by
which elements are returned as long as there is stuff in the queue, which seems
reasonable and important (e.g. callers may need to process all elements that
are in the queue regardless of deadline). That seems less important for
BlockingPut().
I left a comment here.
--
To view, visit http://gerrit.cloudera.org:8080/15324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If32bad5667195d72eeb03e4942751824d4dadb7a
Gerrit-Change-Number: 15324
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 03 Mar 2020 05:44:23 +0000
Gerrit-HasComments: Yes
