Skip to content

Commit

Permalink
osd/PrimaryLogPG: fix potential pg-log overtrimming
Browse files Browse the repository at this point in the history
In ceph#21580 I set a trap to catch some wired
and random segmentfaults and in a recent QA run I was able to observe it was
successfully triggered by one of the test case, see:

```
http://qa-proxy.ceph.com/teuthology/xxg-2018-07-30_05:25:06-rados-wip-hb-peers-distro-basic-smithi/2837916/teuthology.log
```

The root cause is that there might be holes on log versions, thus the
approx_size() method should (almost) always overestimate the actual number of log entries.
As a result, we might be at the risk of overtrimming log entries.

ceph#18338 reveals a probably easier way
to fix the above problem but unfortunately it also can cause big performance regression
and hence comes this pr..

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
(cherry picked from commit 3654d56)

Conflicts:
	src/osd/PrimaryLogPG.cc: trivial resolution
(cherry picked from commit 85a029a)

Resolves: rhbz#1608060
  • Loading branch information
xiexingguo authored and jdurgin committed Oct 24, 2018
1 parent de0b7f7 commit 4d158f6
Showing 1 changed file with 22 additions and 11 deletions.
33 changes: 22 additions & 11 deletions src/osd/PrimaryLogPG.cc
Expand Up @@ -1592,19 +1592,30 @@ void PrimaryLogPG::calc_trim_to()
cct->_conf->osd_pg_log_trim_max >= cct->_conf->osd_pg_log_trim_min) {
return;
}
list<pg_log_entry_t>::const_iterator it = pg_log.get_log().log.begin();
eversion_t new_trim_to;
for (size_t i = 0; i < num_to_trim; ++i) {
new_trim_to = it->version;
++it;
if (new_trim_to >= limit) {
new_trim_to = limit;
dout(10) << "calc_trim_to trimming to limit: " << limit << dendl;
break;
auto it = pg_log.get_log().log.begin(); // oldest log entry
auto rit = pg_log.get_log().log.rbegin();
eversion_t by_n_to_keep; // start from tail
eversion_t by_n_to_trim = eversion_t::max(); // start from head
for (size_t i = 0; it != pg_log.get_log().log.end(); ++it, ++rit) {
i++;
if (i > target && by_n_to_keep == eversion_t()) {
by_n_to_keep = rit->version;
}
if (i >= num_to_trim && by_n_to_trim == eversion_t::max()) {
by_n_to_trim = it->version;
}
if (by_n_to_keep != eversion_t() &&
by_n_to_trim != eversion_t::max()) {
break;
}
}
dout(10) << "calc_trim_to " << pg_trim_to << " -> " << new_trim_to << dendl;
pg_trim_to = new_trim_to;

if (by_n_to_keep == eversion_t()) {
return;
}

pg_trim_to = std::min({by_n_to_keep, by_n_to_trim, limit});
dout(10) << __func__ << " pg_trim_to now " << pg_trim_to << dendl;
assert(pg_trim_to <= pg_log.get_head());
}
}
Expand Down

0 comments on commit 4d158f6

Please sign in to comment.