Hello Alexey Serbin, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8116
to look at the new patch set (#2).
Change subject: periodic: prevent runaway callback loops
......................................................................
periodic: prevent runaway callback loops
One of the key semantics of PeriodicTimer is that Stop() doesn't wait for
an outstanding scheduled callback to finish. It returns immediately and
when the scheduled callback runs, it'll see that Stop() was called and exit.
However, if Start() is called after Stop() but before the callback runs,
both "callback loops" will remain alive. The existing locking will ensure
that their periods converge and that the functor is only invoked once per
period, but it's still an unnecessary resource drain.
This patch addresses this by assigning a "generation" to each callback loop.
When a callback runs, it'll exit if its generation is older than the timer's
current generation. That means an additional N scheduled callbacks for N
Stop/Start call pairs, but after one period all but one of the callbacks
will detect the generational change and exit.
All of this wouldn't be necessary if Stop() could cancel the underlying
callback, but Messenger::ScheduleOnReactor() doesn't support cancelation,
and it'd be far more work to add that than to employ this workaround.
While making this change I decided to replace the various atomic primitives
with basic ones protected by the existing spinlock; I found it easier to
grok the critical sections this way.
Change-Id: I37bfd547db2a6c58d15d228979c88b9b871f72c5
---
M src/kudu/rpc/periodic-test.cc
M src/kudu/rpc/periodic.cc
M src/kudu/rpc/periodic.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/reactor.h
5 files changed, 153 insertions(+), 36 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/8116/2
--
To view, visit http://gerrit.cloudera.org:8080/8116
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I37bfd547db2a6c58d15d228979c88b9b871f72c5
Gerrit-Change-Number: 8116
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
