Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15324 )
Change subject: util: deadline for BlockingQueue::Blocking{Put,Get}
......................................................................
Patch Set 4:
(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: Status SubprocessServer::QueueCall(const
shared_ptr<SubprocessCall>& call) {
It's only called in one place; maybe just inline it?
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: ASSERT_OK(test_queue.BlockingPut(3, MonoTime::Now() +
kShortTimeout));
If this fires, join() is never called. Should use SCOPED_CLEANUP to avoid.
Below too.
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: if (deadline.Initialized() && MonoTime::Now() > deadline) {
: return Status::TimedOut("");
: }
The other operations don't check the timeout up front. Should they? Why the
discrepancy in semantics?
--
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: 4
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:10:44 +0000
Gerrit-HasComments: Yes
