Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8116 )
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 Reviewed-on: http://gerrit.cloudera.org:8080/8116 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <aser...@cloudera.com> Reviewed-by: Todd Lipcon <t...@apache.org> --- 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 4 files changed, 143 insertions(+), 38 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, but someone else must approve Todd Lipcon: Looks good to me, approved -- 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: merged Gerrit-Change-Id: I37bfd547db2a6c58d15d228979c88b9b871f72c5 Gerrit-Change-Number: 8116 Gerrit-PatchSet: 6 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>
