Skip to content

Commit

Permalink
osd: PeeringState: fix stretch peering so PGs can go peered but not a…
Browse files Browse the repository at this point in the history
…ctive

I misunderstood and there was a pretty serious error here: to prevent
accidents, choose_acting() was preventing PGs from *finishing* peering
if they didn't satisfy the stretch cluster rules. What we actually want
to do is to finish peering, but not go active.

Happily, this is easy to fix -- we just add a call to stretch_set_can_peer()
alongside existing min_size checks when we choose whether to go PG_STATE_ACTIVE
or PG_STATE_PEERED!

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
  • Loading branch information
gregsfortytwo committed Mar 12, 2021
1 parent 439aaad commit 38045b4
Showing 1 changed file with 10 additions and 14 deletions.
24 changes: 10 additions & 14 deletions src/osd/PeeringState.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2440,16 +2440,6 @@ bool PeeringState::choose_acting(pg_shard_t &auth_log_shard_id,
}
return false;
}
// make sure we respect the stretch cluster rules -- and
// didn't break them with earlier choices!
const pg_pool_t& pg_pool = pool.info;
if (pg_pool.is_stretch_pool()) {
stringstream ss;
if (!pg_pool.stretch_set_can_peer(want, get_osdmap().get(), &ss)) {
psdout(5) << "peering blocked by stretch_can_peer: " << ss.str() << dendl;
return false;
}
}

if (request_pg_temp_change_only)
return true;
Expand Down Expand Up @@ -2660,7 +2650,8 @@ 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 ((acting.size() >= pool.info.min_size) &&
pool.info.stretch_set_can_peer(acting, get_osdmap().get(), NULL)) {
ceph_assert(cct->_conf->osd_find_best_info_ignore_history_les ||
info.last_epoch_started <= activation_epoch);
info.last_epoch_started = activation_epoch;
Expand Down Expand Up @@ -2961,7 +2952,8 @@ void PeeringState::activate(
state_set(PG_STATE_ACTIVATING);
pl->on_activate(std::move(to_trim));
}
if (acting.size() >= pool.info.min_size) {
if ((acting.size() >= pool.info.min_size) &&
pool.info.stretch_set_can_peer(acting, get_osdmap().get(), NULL)) {
PGLog::LogEntryHandlerRef rollbacker{pl->get_log_handler(t)};
pg_log.roll_forward(rollbacker.get());
}
Expand Down Expand Up @@ -6226,7 +6218,9 @@ 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->acting.size() < ps->pool.info.min_size) ||
!ps->pool.info.stretch_set_can_peer(ps->acting,
ps->get_osdmap().get(), NULL)) {
ps->state_set(PG_STATE_PEERED);
} else {
ps->state_set(PG_STATE_ACTIVE);
Expand Down Expand Up @@ -6384,7 +6378,9 @@ boost::statechart::result PeeringState::ReplicaActive::react(
{}, /* lease */
ps->get_lease_ack());

if (ps->acting.size() >= ps->pool.info.min_size) {
if ((ps->acting.size() >= ps->pool.info.min_size) &&
ps->pool.info.stretch_set_can_peer(ps->acting,
ps->get_osdmap().get(), NULL)) {
ps->state_set(PG_STATE_ACTIVE);
} else {
ps->state_set(PG_STATE_PEERED);
Expand Down

0 comments on commit 38045b4

Please sign in to comment.