add a space after while

can we simplify this loop like

auto trim_to = pg_log.get_log().log.cbegin();
eversion_t trim_to_ver;
for (unsigned n_to_trim = 0, n_to_reserve = pg_log.get_log().log.size();
     n_to_trim < osd_pg_log_trim_max && n_to_reserve >= target;
     ++n_to_trim, --n_to_reserve) {
  if (trim_to == pg_log.get_log().log.cend()) {
    break;
  }
  trim_to_ver = trim_to->version;
  if (trim_to_ver >= limit) {
    break;
  }
  ++trim_to;
}

so we can stop the loop earlier once the trim_to_ver reaches the limit.

also, i think we can update pg_log.get_log().approx_size() to take the advantage of O(1) implementation of list::size(). as we've moved to C++17, and GCC-7 is required.

if approx_size() is accurate, probably we don't need to fuss over this anymore.

@tchaikov This change needs to be backported to mimc

the same also applies to mimic.

in master:

in mimic:

sorry, i forgot that the libstdc++ shipped along with the GCC-7 packaged by SCLs is still using the old ABI. so it has O(n) list::size().

could rename candidate to by_n_to_trim . as it just serves as another criterion for choosing the upper bound of trimming. so, we could improve the code like:

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()) {
      break;
    }
  }
}

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

pg_trim_to = std::min({by_n_to_keep, by_n_to_trim, limit});

please note the parameter of s of PGLog::IndexedLog::trim() is an inclusive bound. so we need to use >= when comparing i with target.

num_to_trim here is the upper bound of the number of entries to trim. now, the only concern is that the actual num to trim could be less than osd_pg_log_trim_min. in that case, by_n_to_trim is eversion_t::max().

not sure how often it happens though, or if it really happens in real world. if we care about this, and want to avoid it. we need to note down i when by_n_to_keep is set, and the total number of pg_log.get_log().log to calculate the number to trim in case by_n_to_trim is not set.

I don't understand. Wouldn't pg_trim_to also be hard-caped at by_n_to_keep or head?

it has nothing to do with by_n_to_keep , but is related the number to trim, there is chance that we never reach the num_to_trim when iterating thru the log. in that case, the number to trim could be less than osd_pg_log_trim_min.

your PR tries to avoid "overtrimming". my concern is about the too small a batch size of trimming.

I see... I guess we can leave it as it is? Because there is no O(1) way to get the log size and that is why this (complicated) fix is needed...