* rationale for a PGLog::merge_old_entry case
@ 2013-06-02 11:09 Loic Dachary
2013-06-03 20:28 ` Samuel Just
0 siblings, 1 reply; 3+ messages in thread
From: Loic Dachary @ 2013-06-02 11:09 UTC (permalink / raw)
To: Samuel Just; +Cc: Ceph Development
[-- 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 --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: rationale for a PGLog::merge_old_entry case 2013-06-02 11:09 rationale for a PGLog::merge_old_entry case Loic Dachary @ 2013-06-03 20:28 ` Samuel Just 2013-06-04 12:14 ` Loic Dachary 0 siblings, 1 reply; 3+ messages in thread From: Samuel Just @ 2013-06-03 20:28 UTC (permalink / raw) To: Loic Dachary; +Cc: Ceph Development In all three cases, we know the authoritative log does not contain an entry for oe.soid, therefore: If oe.prior_version > log.tail, we must already have processed an earlier entry for that object resulting in the object being correctly marked missing (or not) (specifically, the entry for oe.prior_version). If log.tail >= oe.prior_version > eversion_t(), the missing entry should have need set at oe.prior_version (revise_need). oe.prior_version cannot be divergent because all divergent entries must fall within the log (otherwise, we would have backfilled). If oe.prior_version == eversion_t(), the object no longer exists, and the object should be removed from the missing set. Hope that helps. -Sam On Sun, Jun 2, 2013 at 4:09 AM, Loic Dachary <loic@dachary.org> wrote: > 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. > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: rationale for a PGLog::merge_old_entry case 2013-06-03 20:28 ` Samuel Just @ 2013-06-04 12:14 ` Loic Dachary 0 siblings, 0 replies; 3+ messages in thread From: Loic Dachary @ 2013-06-04 12:14 UTC (permalink / raw) To: Samuel Just; +Cc: Ceph Development [-- Attachment #1: Type: text/plain, Size: 2889 bytes --] Hi Sam, Thanks for the explanation. I misread the third case, my description was incorrect. I amended https://github.com/ceph/ceph/pull/340 accordingly. Cheers On 06/03/2013 10:28 PM, Samuel Just wrote: > In all three cases, we know the authoritative log does not contain an > entry for oe.soid, therefore: > > If oe.prior_version > log.tail, we must already have processed an > earlier entry for that object resulting in the object being correctly > marked missing (or not) (specifically, the entry for > oe.prior_version). > > If log.tail >= oe.prior_version > eversion_t(), the missing entry > should have need set at oe.prior_version (revise_need). > oe.prior_version cannot be divergent because all divergent entries > must fall within the log (otherwise, we would have backfilled). > > If oe.prior_version == eversion_t(), the object no longer exists, and > the object should be removed from the missing set. > > Hope that helps. > -Sam > > On Sun, Jun 2, 2013 at 4:09 AM, Loic Dachary <loic@dachary.org> wrote: >> 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. >> -- 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 --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-06-04 12:14 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-02 11:09 rationale for a PGLog::merge_old_entry case Loic Dachary 2013-06-03 20:28 ` Samuel Just 2013-06-04 12:14 ` Loic Dachary
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.