]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
osd/PeeringState: fix acting_set_writeable min_size check 41609/head
authorSamuel Just <sjust@redhat.com>
Fri, 2 Apr 2021 22:30:54 +0000 (22:30 +0000)
committerDan van der Ster <daniel.vanderster@cern.ch>
Tue, 1 Jun 2021 08:44:36 +0000 (10:44 +0200)
acting.size() >= pool.info.min_size is meant to check min_size against
acting set participants, but acting is a vector with placeholders.
actingset is the representation with placeholders removed.

The upshot of this bug is that the activation process will basically
ignore min_size for an ec pool allowing writes in cases where it
shouldn't.  PastIntervals::check_new_interval, however, performs
the check correctly, and will therefore discount intervals in which
we really did serve writes as not writeable.  This can trigger many
different problem conditions including but not limited to:
  - Unfound objects due to accepting a last_update with insufficient
    osds
  - Lost writes
  - Crashes due to peering rules being violated

This bug was originally introduced with recovery below min_size in
e5a96fd, and then preserved through refactors in 749a13d and 95bec9.

7cb818a exposed it with with expansion of recovery below min_size
to include ec pools (acting.size() is sufficient for replicated
pools).

Fixes: https://tracker.ceph.com/issues/48613
Fixes: https://tracker.ceph.com/issues/48417
Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit 642a1c165499bcbd4cfdf907af313ac7ffe44ff4)

Conflicts:
src/osd/PeeringState.h

Fixes the callers rather than also backporting 95bec9873.

src/osd/PeeringState.cc

index 598679029b035134e7e1739a266949a9b2e808ba..9e893079ebebd18e846067f7f7a549848091b009 100644 (file)
@@ -2328,7 +2328,7 @@ void PeeringState::activate(
 
   if (is_primary()) {
     // only update primary last_epoch_started if we will go active
-    if (acting.size() >= pool.info.min_size) {
+    if (actingset.size() >= pool.info.min_size) {
       ceph_assert(cct->_conf->osd_find_best_info_ignore_history_les ||
             info.last_epoch_started <= activation_epoch);
       info.last_epoch_started = activation_epoch;
@@ -2634,7 +2634,7 @@ void PeeringState::activate(
     state_set(PG_STATE_ACTIVATING);
     pl->on_activate(std::move(to_trim));
   }
-  if (acting.size() >= pool.info.min_size) {
+  if (actingset.size() >= pool.info.min_size) {
     PGLog::LogEntryHandlerRef rollbacker{pl->get_log_handler(t)};
     pg_log.roll_forward(rollbacker.get());
   }
@@ -5864,7 +5864,7 @@ boost::statechart::result PeeringState::Active::react(const AllReplicasActivated
        pl->set_not_ready_to_merge_source(pgid);
       }
     }
-  } else if (ps->acting.size() < ps->pool.info.min_size) {
+  } else if (ps->actingset.size() < ps->pool.info.min_size) {
     ps->state_set(PG_STATE_PEERED);
   } else {
     ps->state_set(PG_STATE_ACTIVE);
@@ -6023,7 +6023,7 @@ boost::statechart::result PeeringState::ReplicaActive::react(
     {}, /* lease */
     ps->get_lease_ack());
 
-  if (ps->acting.size() >= ps->pool.info.min_size) {
+  if (ps->actingset.size() >= ps->pool.info.min_size) {
     ps->state_set(PG_STATE_ACTIVE);
   } else {
     ps->state_set(PG_STATE_PEERED);