rationale for a PGLog::merge_old_entry case

[Loic Dachary <loic@dachary.org>]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

Hi Sam,

TL;DR:

When there no new entry, what is the rationale for merge_old_entry to remove the object from missing only if the tail is eversion_t() and the object prior_version is also eversion_t() ?
https://github.com/dachary/ceph/blob/f58299db098d5f18c817b516fa6ffaa76245e57d/src/osd/PGLog.cc#L330

Long version:

The conditions are created with:

    info.log_tail = eversion_t();
    oe.soid.hash = 1;
    oe.op = pg_log_entry_t::DELETE;
    oe.prior_version = eversion_t();
    
    missing.add(oe.soid, eversion_t(1,1), eversion_t());

as shown in
https://github.com/dachary/ceph/blob/f58299db098d5f18c817b516fa6ffaa76245e57d/src/test/osd/TestPGLog.cc#L467
I double checked with gdb and when called with

EXPECT_FALSE(merge_old_entry(t, oe, info, remove_snap, dirty_log));
https://github.com/dachary/ceph/blob/f58299db098d5f18c817b516fa6ffaa76245e57d/src/test/osd/TestPGLog.cc#L481

it reaches

missing.rm(oe.soid, missing.missing[oe.soid].need);
https://github.com/dachary/ceph/blob/f58299db098d5f18c817b516fa6ffaa76245e57d/src/osd/PGLog.cc#L330

and the expected side effects are observed:

    EXPECT_FALSE(dirty_log);
    EXPECT_TRUE(remove_snap.empty());
    EXPECT_TRUE(t.empty());
    EXPECT_FALSE(missing.have_missing());
    EXPECT_TRUE(log.empty());
    EXPECT_EQ(0U, ondisklog.length());
https://github.com/dachary/ceph/blob/f58299db098d5f18c817b516fa6ffaa76245e57d/src/test/osd/TestPGLog.cc#L483

Cheers

-- 
Loïc Dachary, Artisan Logiciel Libre
All that is necessary for the triumph of evil is that good people do nothing.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]