From 6ebbf299ec82cca8d85b441894f8ee9b841f2f63 Mon Sep 17 00:00:00 2001 From: Neha Ojha Date: Mon, 30 Jul 2018 17:09:51 -0700 Subject: [PATCH] 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 (cherry picked from commit 283b0bde4a52128c1590afe8e5011b266a2e334b) --- src/osd/PG.cc | 30 ------------------------------ src/osd/PG.h | 1 - 2 files changed, 31 deletions(-) diff --git a/src/osd/PG.cc b/src/osd/PG.cc index c3fb6ef7c133a..61e23c858d5da 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -3568,33 +3568,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::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 @@ -7807,9 +7780,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, diff --git a/src/osd/PG.h b/src/osd/PG.h index 136368a199021..bcd884a435734 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -2888,7 +2888,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; -- 2.39.5