src/osd/PG.cc: remove redundant call to trim_log()

This change is motived by the failure tracked in
https://tracker.ceph.com/issues/25198. The failure highlights a case, when a
call to trim_log() after the PG has recovered, races with the previous op,
on a replica OSD. Since the previous operation has not completed, the
last_complete value for that OSD is not valid, when we try to trim the
log. It is also worth noting that the race is due to MOSDPGTrim going through
the strict queue as a peering message vs regular ops going through the
non-strict queue.

During the investigation of this bug, we noticed that, with
https://tracker.ceph.com/issues/23979, we allow pg log trimming to
happen on the primary and replicas, whenever we cross the upper bound of
the pg log. This also ensures that pg log trimming happens while processing
any new op.

Therefore, the function trim_log(), which earlier served the purpose of
trimming logs on the primary and replicas, just before the PG went into
the Recovered state, is no more required. This acted like a last line of
defense to trim logs, when we did not need the logs any more. But, this call
seems redundant now, because, we are limiting the pg log length at all times.

Signed-off-by: Neha Ojha <nojha@redhat.com>
This commit is contained in:
Neha Ojha 2018-07-30 17:09:51 -07:00
parent a8b39ccc2c
commit 283b0bde4a
2 changed files with 0 additions and 31 deletions

View File

@ -3573,33 +3573,6 @@ void PG::write_if_dirty(ObjectStore::Transaction& t)
t.omap_setkeys(coll, pgmeta_oid, km);
}
void PG::trim_log()
{
assert(is_primary());
calc_trim_to();
dout(10) << __func__ << " to " << pg_trim_to << dendl;
if (pg_trim_to != eversion_t()) {
// inform peers to trim log
assert(!acting_recovery_backfill.empty());
for (set<pg_shard_t>::iterator i = acting_recovery_backfill.begin();
i != acting_recovery_backfill.end();
++i) {
if (*i == pg_whoami) continue;
osd->send_message_osd_cluster(
i->osd,
new MOSDPGTrim(
get_osdmap()->get_epoch(),
spg_t(info.pgid.pgid, i->shard),
pg_trim_to),
get_osdmap()->get_epoch());
}
// trim primary as well
pg_log.trim(pg_trim_to, info);
dirty_info = true;
}
}
void PG::add_log_entry(const pg_log_entry_t& e, bool applied)
{
// raise last_complete only if we were previously up to date
@ -7812,9 +7785,6 @@ PG::RecoveryState::Recovered::Recovered(my_context ctx)
pg->publish_stats_to_osd();
}
// trim pglog on recovered
pg->trim_log();
// adjust acting set? (e.g. because backfill completed...)
bool history_les_bound = false;
if (pg->acting != pg->up && !pg->choose_acting(auth_log_shard,

View File

@ -2887,7 +2887,6 @@ protected:
bool transaction_applied = true,
bool async = false);
bool check_log_for_corruption(ObjectStore *store);
void trim_log();
std::string get_corrupt_pg_log_name() const;