Skip to content

Commit 6bc367f

Browse files
author
Ibrahim Jarif
committed
Remove the 'this entry should've caught' log from value.go (#1170)
Fixes - #1031 (There wasn't a bug to fix. The log statement shouldn't have been there) This PR removes the warning message `WARNING: This entry should have been caught.`. The warning message assumed that we will always find the **newest key if two keys have the same version** This assumption is valid in case of a normal key but it's **NOT TRUE** in case of **move keys**. Here's how we can end up fetching the older version of a move key if two move keys have the same version. ``` It might be possible that the entry read from LSM Tree points to an older vlog file. This can happen in the following situation. Assume DB is opened with numberOfVersionsToKeep=1 Now, if we have ONLY one key in the system "FOO" which has been updated 3 times and the same key has been garbage collected 3 times, we'll have 3 versions of the movekey for the same key "FOO". NOTE: moveKeyi is the moveKey with version i Assume we have 3 move keys in L0. - moveKey1 (points to vlog file 10), - moveKey2 (points to vlog file 14) and - moveKey3 (points to vlog file 15). Also, assume there is another move key "moveKey1" (points to vlog file 6) (this is also a move Key for key "FOO" ) on upper levels (let's say level 3). The move key "moveKey1" on level 0 was inserted because vlog file 6 was GCed. Here's what the arrangement looks like L0 => (moveKey1 => vlog10), (moveKey2 => vlog14), (moveKey3 => vlog15) L1 => .... L2 => .... L3 => (moveKey1 => vlog6) When L0 compaction runs, it keeps only moveKey3 because the number of versions to keep is set to 1. (we've dropped moveKey1's latest version) The new arrangement of keys is L0 => .... L1 => (moveKey3 => vlog15) L2 => .... L3 => (moveKey1 => vlog6) Now if we try to GC vlog file 10, the entry read from vlog file will point to vlog10 but the entry read from LSM Tree will point to vlog6. The move key read from LSM tree will point to vlog6 because we've asked for version 1 of the move key. This might seem like an issue but it's not really an issue because the user has set the number of versions to keep to 1 and the latest version of moveKey points to the correct vlog file and offset. The stale move key on L3 will be eventually dropped by compaction because there is a newer version in the upper levels. ``` (cherry picked from commit 2a90c66)
1 parent 6ee3f53 commit 6bc367f

File tree

1 file changed

+47
-1
lines changed

1 file changed

+47
-1
lines changed

value.go

+47-1
Original file line numberDiff line numberDiff line change
@@ -523,12 +523,19 @@ func (vlog *valueLog) rewrite(f *logFile, tr trace.Trace) error {
523523
var vp valuePointer
524524
vp.Decode(vs.Value)
525525

526+
// If the entry found from the LSM Tree points to a newer vlog file, don't do anything.
526527
if vp.Fid > f.fid {
527528
return nil
528529
}
530+
// If the entry found from the LSM Tree points to an offset greater than the one
531+
// read from vlog, don't do anything.
529532
if vp.Offset > e.offset {
530533
return nil
531534
}
535+
// If the entry read from LSM Tree and vlog file point to the same vlog file and offset,
536+
// insert them back into the DB.
537+
// NOTE: It might be possible that the entry read from the LSM Tree points to
538+
// an older vlog file. See the comments in the else part.
532539
if vp.Fid == f.fid && vp.Offset == e.offset {
533540
moved++
534541
// This new entry only contains the key, and a pointer to the value.
@@ -564,7 +571,46 @@ func (vlog *valueLog) rewrite(f *logFile, tr trace.Trace) error {
564571
wb = append(wb, ne)
565572
size += es
566573
} else {
567-
vlog.db.opt.Warningf("This entry should have been caught. %+v\n", e)
574+
// It might be possible that the entry read from LSM Tree points to an older vlog file.
575+
// This can happen in the following situation. Assume DB is opened with
576+
// numberOfVersionsToKeep=1
577+
//
578+
// Now, if we have ONLY one key in the system "FOO" which has been updated 3 times and
579+
// the same key has been garbage collected 3 times, we'll have 3 versions of the movekey
580+
// for the same key "FOO".
581+
// NOTE: moveKeyi is the moveKey with version i
582+
// Assume we have 3 move keys in L0.
583+
// - moveKey1 (points to vlog file 10),
584+
// - moveKey2 (points to vlog file 14) and
585+
// - moveKey3 (points to vlog file 15).
586+
587+
// Also, assume there is another move key "moveKey1" (points to vlog file 6) (this is
588+
// also a move Key for key "FOO" ) on upper levels (let's say 3). The move key
589+
// "moveKey1" on level 0 was inserted because vlog file 6 was GCed.
590+
//
591+
// Here's what the arrangement looks like
592+
// L0 => (moveKey1 => vlog10), (moveKey2 => vlog14), (moveKey3 => vlog15)
593+
// L1 => ....
594+
// L2 => ....
595+
// L3 => (moveKey1 => vlog6)
596+
//
597+
// When L0 compaction runs, it keeps only moveKey3 because the number of versions
598+
// to keep is set to 1. (we've dropped moveKey1's latest version)
599+
//
600+
// The new arrangement of keys is
601+
// L0 => ....
602+
// L1 => (moveKey3 => vlog15)
603+
// L2 => ....
604+
// L3 => (moveKey1 => vlog6)
605+
//
606+
// Now if we try to GC vlog file 10, the entry read from vlog file will point to vlog10
607+
// but the entry read from LSM Tree will point to vlog6. The move key read from LSM tree
608+
// will point to vlog6 because we've asked for version 1 of the move key.
609+
//
610+
// This might seem like an issue but it's not really an issue because the user has set
611+
// the number of versions to keep to 1 and the latest version of moveKey points to the
612+
// correct vlog file and offset. The stale move key on L3 will be eventually dropped by
613+
// compaction because there is a newer versions in the upper levels.
568614
}
569615
return nil
570616
}

0 commit comments

Comments
 (0)