From 462b9163da763d81aecb51fb82827d1c51f58cd2 Mon Sep 17 00:00:00 2001 From: Greg Farnum Date: Mon, 29 Apr 2019 15:39:59 -0700 Subject: [PATCH] mon: paxos: introduce new reset_pending_committing_finishers for safety There are asserts about the state of the system and pending_finishers which can be triggered by running arbitrary commands through again. They are correct when not restarting, but when we do restart we need to take care to preserve the same invariants as appropriate. Use this function to be careful about the order of committing_finishers v pending_finishers and to make sure they're both empty before any Contexts actually get called. We also reorder a call to finish_contexts on the waiting_for_writeable list for similar reasons. Fixes: http://tracker.ceph.com/issues/39484 Signed-off-by: Greg Farnum (cherry picked from commit b17caec586ba2801593db61f91d66719d40b905e) --- src/mon/Paxos.cc | 14 ++++++++------ src/mon/Paxos.h | 9 +++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc index 008642bfa3076..5d863e19f05cc 100644 --- a/src/mon/Paxos.cc +++ b/src/mon/Paxos.cc @@ -1343,8 +1343,7 @@ void Paxos::leader_init() // discard pending transaction pending_proposal.reset(); - finish_contexts(g_ceph_context, pending_finishers, -EAGAIN); - finish_contexts(g_ceph_context, committing_finishers, -EAGAIN); + reset_pending_committing_finishers(); logger->inc(l_paxos_start_leader); @@ -1375,9 +1374,8 @@ void Paxos::peon_init() pending_proposal.reset(); // no chance to write now! + reset_pending_committing_finishers(); finish_contexts(g_ceph_context, waiting_for_writeable, -EAGAIN); - finish_contexts(g_ceph_context, pending_finishers, -EAGAIN); - finish_contexts(g_ceph_context, committing_finishers, -EAGAIN); logger->inc(l_paxos_start_peon); } @@ -1400,13 +1398,17 @@ void Paxos::restart() // discard pending transaction pending_proposal.reset(); - finish_contexts(g_ceph_context, committing_finishers, -EAGAIN); - finish_contexts(g_ceph_context, pending_finishers, -EAGAIN); + reset_pending_committing_finishers(); finish_contexts(g_ceph_context, waiting_for_active, -EAGAIN); logger->inc(l_paxos_restart); } +void Paxos::reset_pending_committing_finishers() +{ + committing_finishers.splice(committing_finishers.end(), pending_finishers); + finish_contexts(g_ceph_context, committing_finishers, -EAGAIN); +} void Paxos::dispatch(MonOpRequestRef op) { diff --git a/src/mon/Paxos.h b/src/mon/Paxos.h index 9015f45bb072a..517df5fa50811 100644 --- a/src/mon/Paxos.h +++ b/src/mon/Paxos.h @@ -597,6 +597,15 @@ private: * this list. When it commits, these finishers are notified. */ list committing_finishers; + /** + * This function re-triggers pending_ and committing_finishers + * safely, so as to maintain existing system invariants. In particular + * we maintain ordering by triggering committing before pending, and + * we clear out pending_finishers prior to any triggers so that + * we don't trigger asserts on them being empty. You should + * use it instead of sending -EAGAIN to them with finish_contexts. + */ + void reset_pending_committing_finishers(); /** * @defgroup Paxos_h_sync_warns Synchronization warnings -- 2.39.5