mon/AuthMonitor: provide auth_lock-safe _assign_global_id()

Break assign_global_id() into two parts:

- _assign_global_id(), called under auth_lock, that does nothing but
  assign an id (or fail).  This makes use of the num_mon and num_rank
  values, which means it only works while we are in quorum.
- increase_max_global_id(), if it looks like we should.

Also, adjust _should_increase_max_global_id() to also work under auth_lock
since we are looking at last_allocated_id.  All last_allocated_id users
are not protected by auth_lock.

Fixes: http://tracker.ceph.com/issues/38372
Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index b44b62825be9557d1dde753a551887434d1c3641..6cbe420e4e0df6571e7ea667850dc74bfd9422d4 100644 (file)
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -73,7 +73,12 @@ void AuthMonitor::tick()
 
   // increase global_id?
   bool propose = false;
-  if (should_increase_max_global_id()) {
+  bool increase;
+  {
+    std::lock_guard l(mon->auth_lock);
+    increase = _should_increase_max_global_id();
+  }
+  if (increase) {
     if (mon->is_leader()) {
       increase_max_global_id();
       propose = true;
@@ -107,7 +112,12 @@ void AuthMonitor::on_active()
     return;
   mon->key_server.start_server();
 
-  if (is_writeable() && should_increase_max_global_id()) {
+  bool increase;
+  {
+    std::lock_guard l(mon->auth_lock);
+    increase = _should_increase_max_global_id();
+  }
+  if (is_writeable() && increase) {
     increase_max_global_id();
     propose_pending();
   }
@@ -307,17 +317,23 @@ void AuthMonitor::update_from_paxos(bool *need_bootstrap)
     }
   }
 
-  if (last_allocated_id == 0)
-    last_allocated_id = max_global_id;
+  {
+    std::lock_guard l(mon->auth_lock);
+    if (last_allocated_id == 0) {
+      last_allocated_id = max_global_id;
+      dout(10) << __func__ << " last_allocated_id initialized to "
+              << max_global_id << dendl;
+    }
+  }
 
-  dout(10) << "update_from_paxos() last_allocated_id=" << last_allocated_id
-          << " max_global_id=" << max_global_id
+  dout(10) << __func__ << " max_global_id=" << max_global_id
           << " format_version " << format_version
           << dendl;
 }
 
-bool AuthMonitor::should_increase_max_global_id()
+bool AuthMonitor::_should_increase_max_global_id()
 {
+  ceph_assert(ceph_mutex_is_locked(mon->auth_lock));
   auto num_prealloc = g_conf()->mon_globalid_prealloc;
   if (max_global_id < num_prealloc ||
       (last_allocated_id + 1) >= max_global_id - num_prealloc / 2) {
@@ -330,10 +346,10 @@ void AuthMonitor::increase_max_global_id()
 {
   ceph_assert(mon->is_leader());
 
-  dout(10) << "increasing max_global_id to " << max_global_id << dendl;
   Incremental inc;
   inc.inc_type = GLOBAL_ID;
   inc.max_global_id = max_global_id + g_conf()->mon_globalid_prealloc;
+  dout(10) << "increasing max_global_id to " << inc.max_global_id << dendl;
   pending_auth.push_back(inc);
 }
 
@@ -492,6 +508,7 @@ bool AuthMonitor::prepare_update(MonOpRequestRef op)
     return false;
   }
 }
+
 void AuthMonitor::_set_mon_num_rank(int num, int rank)
 {
   dout(10) << __func__ << " num " << num << " rank " << rank << dendl;
@@ -503,9 +520,13 @@ void AuthMonitor::_set_mon_num_rank(int num, int rank)
 uint64_t AuthMonitor::_assign_global_id()
 {
   ceph_assert(ceph_mutex_is_locked(mon->auth_lock));
-  if (num_mon < 1 || rank < 0) {
-    dout(10) << __func__ << " inactive (num_mon " << num_mon
-            << " rank " << rank << ")" << dendl;
+  if (mon_num < 1 || mon_rank < 0) {
+    dout(10) << __func__ << " inactive (num_mon " << mon_num
+            << " rank " << mon_rank << ")" << dendl;
+    return 0;
+  }
+  if (!last_allocated_id) {
+    dout(10) << __func__ << " last_allocated_id == 0" << dendl;
     return 0;
   }
 
@@ -529,14 +550,19 @@ uint64_t AuthMonitor::_assign_global_id()
 
 uint64_t AuthMonitor::assign_global_id(bool should_increase_max)
 {
+  uint64_t id;
+  {
+    std::lock_guard l(mon->auth_lock);
+    id =_assign_global_id();
+    if (should_increase_max) {
+      should_increase_max = _should_increase_max_global_id();
+    }
+  }
   if (mon->is_leader() &&
-      should_increase_max &&
-      should_increase_max_global_id()) {
+      should_increase_max) {
     increase_max_global_id();
   }
-
-  std::lock_guard l(mon->auth_lock);
-  return _assign_global_id();
+  return id;
 }
 
 bool AuthMonitor::prep_auth(MonOpRequestRef op, bool paxos_writable)

diff --git a/src/mon/AuthMonitor.h b/src/mon/AuthMonitor.h
index c5bf22fee27d909e1cf80ba1b5ac67856f1a031f..ca884fbd1ed04a7f3ca17b5e25447fe170392ce9 100644 (file)
--- a/src/mon/AuthMonitor.h
+++ b/src/mon/AuthMonitor.h
@@ -145,10 +145,11 @@ private:
   void update_from_paxos(bool *need_bootstrap) override;
   void create_pending() override;  // prepare a new pending
   bool prepare_global_id(MonOpRequestRef op);
-  bool should_increase_max_global_id();
+  bool _should_increase_max_global_id(); ///< called under mon->auth_lock
   void increase_max_global_id();
   uint64_t assign_global_id(bool should_increase_max);
 public:
+  uint64_t _assign_global_id(); ///< called under mon->auth_lock
   void _set_mon_num_rank(int num, int rank); ///< called under mon->auth_lock
 
 private: