Adar Dembo has posted comments on this change.

Change subject: log: shut down appender thread when idle
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

Line 256:       &worker_state_, WORKER_STOPPED, WORKER_ACTIVE);
> hard to repro in a test without injecting various sleeps. I did manually in
I looked at mt-log-test and I don't think it's geared to trigger this, because 
all of the writers are running all at once, and then not at all. That is, 
there's no staggering, so there's no window of time where the queue will be 
empty of entries for an entire second.

Maybe mt-log-test could reliably trigger this if you made kIdleThreshold 
configurable and turned it way down (say 1ms) for the test? That would probably 
be a good stress test in and of itself, since it'd trigger many active->stopped 
and stopped->active transitions.

Broadly speaking I don't find MAYBE... style fault injection to be polluting, 
but I don't want to ask to you spend hours building a test that, in the best of 
times, rarely triggers the race. So, if it's easily reproducible by tweaking 
the timeout (or injecting some latency), I'd support it, but not if it 
necessitates a new test with hundreds of lines of code.


-- 
To view, visit http://gerrit.cloudera.org:8080/6856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to