Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osd/PG: restrict want_acting to up+acting on recovery completion #13420

Merged
merged 1 commit into from Feb 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 37 additions & 12 deletions src/osd/PG.cc
Expand Up @@ -1005,7 +1005,9 @@ PG::Scrubber::~Scrubber() {}
* 3) Prefer current primary
*/
map<pg_shard_t, pg_info_t>::const_iterator PG::find_best_info(
const map<pg_shard_t, pg_info_t> &infos, bool *history_les_bound) const
const map<pg_shard_t, pg_info_t> &infos,
bool restrict_to_up_acting,
bool *history_les_bound) const
{
assert(history_les_bound);
/* See doc/dev/osd_internals/last_epoch_started.rst before attempting
Expand Down Expand Up @@ -1045,6 +1047,9 @@ map<pg_shard_t, pg_info_t>::const_iterator PG::find_best_info(
for (map<pg_shard_t, pg_info_t>::const_iterator p = infos.begin();
p != infos.end();
++p) {
if (restrict_to_up_acting && !is_up(p->first) &&
!is_acting(p->first))
continue;
// Only consider peers with last_update >= min_last_update_acceptable
if (p->second.last_update < min_last_update_acceptable)
continue;
Expand Down Expand Up @@ -1103,17 +1108,19 @@ void PG::calc_ec_acting(
pg_shard_t up_primary,
const map<pg_shard_t, pg_info_t> &all_info,
bool compat_mode,
bool restrict_to_up_acting,
vector<int> *_want,
set<pg_shard_t> *backfill,
set<pg_shard_t> *acting_backfill,
pg_shard_t *want_primary,
ostream &ss) {
ostream &ss)
{
vector<int> want(size, CRUSH_ITEM_NONE);
map<shard_id_t, set<pg_shard_t> > all_info_by_shard;
unsigned usable = 0;
for(map<pg_shard_t, pg_info_t>::const_iterator i = all_info.begin();
i != all_info.end();
++i) {
for (map<pg_shard_t, pg_info_t>::const_iterator i = all_info.begin();
i != all_info.end();
++i) {
all_info_by_shard[i->first.shard].insert(i->first);
}
for (uint8_t i = 0; i < want.size(); ++i) {
Expand All @@ -1140,7 +1147,7 @@ void PG::calc_ec_acting(
ss << " selecting acting[i]: " << pg_shard_t(acting[i], shard_id_t(i)) << std::endl;
want[i] = acting[i];
++usable;
} else {
} else if (!restrict_to_up_acting) {
for (set<pg_shard_t>::iterator j = all_info_by_shard[shard_id_t(i)].begin();
j != all_info_by_shard[shard_id_t(i)].end();
++j) {
Expand Down Expand Up @@ -1189,14 +1196,16 @@ void PG::calc_replicated_acting(
pg_shard_t up_primary,
const map<pg_shard_t, pg_info_t> &all_info,
bool compat_mode,
bool restrict_to_up_acting,
vector<int> *want,
set<pg_shard_t> *backfill,
set<pg_shard_t> *acting_backfill,
pg_shard_t *want_primary,
ostream &ss)
{
ss << "calc_acting newest update on osd." << auth_log_shard->first
<< " with " << auth_log_shard->second << std::endl;
<< " with " << auth_log_shard->second
<< (restrict_to_up_acting ? " restrict_to_up_acting" : "") << std::endl;
pg_shard_t auth_log_shard_id = auth_log_shard->first;

// select primary
Expand Down Expand Up @@ -1287,6 +1296,9 @@ void PG::calc_replicated_acting(
}
}

if (restrict_to_up_acting) {
return;
}
for (map<pg_shard_t,pg_info_t>::const_iterator i = all_info.begin();
i != all_info.end();
++i) {
Expand Down Expand Up @@ -1323,8 +1335,19 @@ void PG::calc_replicated_acting(
*
* calculate the desired acting, and request a change with the monitor
* if it differs from the current acting.
*
* if restrict_to_up_acting=true, we filter out anything that's not in
* up/acting. in order to lift this restriction, we need to
* 1) check whether it's worth switching the acting set any time we get
* a new pg info (not just here, when recovery finishes)
* 2) check whether anything in want_acting went down on each new map
* (and, if so, calculate a new want_acting)
* 3) remove the assertion in PG::RecoveryState::Active::react(const AdvMap)
* TODO!
*/
bool PG::choose_acting(pg_shard_t &auth_log_shard_id, bool *history_les_bound)
bool PG::choose_acting(pg_shard_t &auth_log_shard_id,
bool restrict_to_up_acting,
bool *history_les_bound)
{
map<pg_shard_t, pg_info_t> all_info(peer_info.begin(), peer_info.end());
all_info[pg_whoami] = info;
Expand All @@ -1336,7 +1359,7 @@ bool PG::choose_acting(pg_shard_t &auth_log_shard_id, bool *history_les_bound)
}

map<pg_shard_t, pg_info_t>::const_iterator auth_log_shard =
find_best_info(all_info, history_les_bound);
find_best_info(all_info, restrict_to_up_acting, history_les_bound);

if (auth_log_shard == all_info.end()) {
if (up != acting) {
Expand Down Expand Up @@ -1390,6 +1413,7 @@ bool PG::choose_acting(pg_shard_t &auth_log_shard_id, bool *history_les_bound)
up_primary,
all_info,
compat_mode,
restrict_to_up_acting,
&want,
&want_backfill,
&want_acting_backfill,
Expand All @@ -1405,6 +1429,7 @@ bool PG::choose_acting(pg_shard_t &auth_log_shard_id, bool *history_les_bound)
up_primary,
all_info,
compat_mode,
restrict_to_up_acting,
&want,
&want_backfill,
&want_acting_backfill,
Expand Down Expand Up @@ -6773,7 +6798,7 @@ PG::RecoveryState::Recovered::Recovered(my_context ctx)
// adjust acting set? (e.g. because backfill completed...)
bool history_les_bound = false;
if (pg->acting != pg->up && !pg->choose_acting(auth_log_shard,
&history_les_bound))
true, &history_les_bound))
assert(pg->want_acting.size());

if (context< Active >().all_replicas_activated)
Expand Down Expand Up @@ -7564,8 +7589,8 @@ PG::RecoveryState::GetLog::GetLog(my_context ctx)
PG *pg = context< RecoveryMachine >().pg;

// adjust acting?
if (!pg->choose_acting(auth_log_shard,
&context< Peering >().history_les_bound)) {
if (!pg->choose_acting(auth_log_shard, false,
&context< Peering >().history_les_bound)) {
if (!pg->want_acting.empty()) {
post_event(NeedActingChange());
} else {
Expand Down
19 changes: 11 additions & 8 deletions src/osd/PG.h
Expand Up @@ -870,17 +870,16 @@ class PG : protected DoutPrefixProvider {
return actingbackfill.count(osd);
}
bool is_acting(pg_shard_t osd) const {
if (pool.info.ec_pool()) {
return acting.size() > (unsigned)osd.shard && acting[osd.shard] == osd.osd;
} else {
return std::find(acting.begin(), acting.end(), osd.osd) != acting.end();
}
return has_shard(pool.info.ec_pool(), acting, osd);
}
bool is_up(pg_shard_t osd) const {
if (pool.info.ec_pool()) {
return up.size() > (unsigned)osd.shard && up[osd.shard] == osd.osd;
return has_shard(pool.info.ec_pool(), up, osd);
}
static bool has_shard(bool ec, const vector<int>& v, pg_shard_t osd) {
if (ec) {
return v.size() > (unsigned)osd.shard && v[osd.shard] == osd.osd;
} else {
return std::find(up.begin(), up.end(), osd.osd) != up.end();
return std::find(v.begin(), v.end(), osd.osd) != v.end();
}
}

Expand Down Expand Up @@ -983,6 +982,7 @@ class PG : protected DoutPrefixProvider {

map<pg_shard_t, pg_info_t>::const_iterator find_best_info(
const map<pg_shard_t, pg_info_t> &infos,
bool restrict_to_up_acting,
bool *history_les_bound) const;
static void calc_ec_acting(
map<pg_shard_t, pg_info_t>::const_iterator auth_log_shard,
Expand All @@ -993,6 +993,7 @@ class PG : protected DoutPrefixProvider {
pg_shard_t up_primary,
const map<pg_shard_t, pg_info_t> &all_info,
bool compat_mode,
bool restrict_to_up_acting,
vector<int> *want,
set<pg_shard_t> *backfill,
set<pg_shard_t> *acting_backfill,
Expand All @@ -1007,12 +1008,14 @@ class PG : protected DoutPrefixProvider {
pg_shard_t up_primary,
const map<pg_shard_t, pg_info_t> &all_info,
bool compat_mode,
bool restrict_to_up_acting,
vector<int> *want,
set<pg_shard_t> *backfill,
set<pg_shard_t> *acting_backfill,
pg_shard_t *want_primary,
ostream &ss);
bool choose_acting(pg_shard_t &auth_log_shard,
bool restrict_to_up_acting,
bool *history_les_bound);
void build_might_have_unfound();
void activate(
Expand Down