Skip to content

Commit

Permalink
Handle case of sequence number 0 files for compatibility with RocksDB.
Browse files Browse the repository at this point in the history
Fixes #316
  • Loading branch information
sumeerbhola committed Oct 7, 2019
1 parent 36425ed commit 9509253
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 16 deletions.
8 changes: 8 additions & 0 deletions internal/manifest/testdata/version_check_ordering
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ L0 flushed file 000003 has an ingested file coincident with smallest seqnum: 4-6
2:[a#5,1-d#5,1]
3:[a#4,1-d#6,1]

check-ordering
L0
a.SET.0-d.SET.0
a.SET.0-d.SET.0
a.SET.0-d.SET.3
----
OK

check-ordering
L1
a.SET.1-b.SET.2
Expand Down
15 changes: 15 additions & 0 deletions internal/manifest/testdata/version_edit_apply
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,21 @@ edit
----
pebble: internal error: L0 flushed file 000004 overlaps with the largest seqnum of a preceding flushed file: 3-5 vs 4

apply
L0
1:a.SET.1-c.SET.2
2:c.SET.3-d.SET.4
edit
add
L0
4:b.SET.0-d.SET.0
----
0:
4:[b#0,1-d#0,1]
1:[a#1,1-c#2,1]
2:[c#3,1-d#4,1]


apply
edit
add
Expand Down
29 changes: 22 additions & 7 deletions internal/manifest/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,14 +432,16 @@ func CheckOrdering(cmp Compare, format base.Formatter, level int, files []FileMe
// - Files with exactly one sequence number: these could be either ingested files
// or flushed files. We cannot tell the difference between them based on FileMetadata,
// so our consistency checking here uses the weaker checks assuming it is an ingested
// file.
// file (except for the 0 sequence number case below).
// - Files with multiple sequence numbers: these are necessarily flushed files.
//
// The only overlapping sequence number case is an ingested file contained in the sequence
// numbers of the flushed file -- it must be fully contained (not coincident with either
// end of the flushed file) since the memtable must have been at [a, b-1] (where b > a)
// when the ingested file was assigned sequence num b, and the memtable got a subsequent
// update that was given sequence num b+1, before being flushed.
// Two cases of overlapping sequence numbers:
// Case 1:
// An ingested file contained in the sequence numbers of the flushed file -- it must be
// fully contained (not coincident with either end of the flushed file) since the memtable
// must have been at [a, b-1] (where b > a) when the ingested file was assigned sequence
// num b, and the memtable got a subsequent update that was given sequence num b+1, before
// being flushed.
//
// So a sequence [1000, 1000] [1002, 1002] [1000, 2000] is invalid since the first and
// third file are inconsistent with each other. So comparing adjacent files is insufficient
Expand All @@ -449,6 +451,12 @@ func CheckOrdering(cmp Compare, format base.Formatter, level int, files []FileMe
// x------y x-----------yx-------------y (flushed files where x, y are the endpoints)
// y y y y (y's represent ingested files)
// And these are ordered in increasing order of y. Note that y's must be unique.
//
// Case 2:
// A flushed file that did not overlap in keys with any file in any level, but does overlap
// in the file key intervals. This file is placed in L0 since it overlaps in the file
// key intervals but since it has no overlapping data, it is assigned a sequence number
// of 0 in RocksDB. We handle this case for compatibility with RocksDB.

// The largest sequence number of a flushed file. Increasing.
var largestFlushedSeqNum uint64
Expand All @@ -466,13 +474,20 @@ func CheckOrdering(cmp Compare, format base.Formatter, level int, files []FileMe
if i > 0 {
// Validate that the sorting is sane.
prev := &files[i-1]
if !prev.lessSeqNum(f) {
if prev.LargestSeqNum == 0 && f.LargestSeqNum == prev.LargestSeqNum {
// Multiple files satisfying case 2 mentioned above.
} else if !prev.lessSeqNum(f) {
return fmt.Errorf("L0 files %06d and %06d are not properly ordered: %d-%d vs %d-%d",
prev.FileNum, f.FileNum,
prev.SmallestSeqNum, prev.LargestSeqNum,
f.SmallestSeqNum, f.LargestSeqNum)
}
}
if f.LargestSeqNum == 0 && f.LargestSeqNum == f.SmallestSeqNum {
// We don't add these to uncheckedIngestedSeqNums since there could be flushed
// files of the form [0, N] where N > 0.
continue
}
if i > 0 && largestSeqNum >= f.LargestSeqNum {
return fmt.Errorf("L0 file %06d does not have strictly increasing "+
"largest seqnum: %d-%d vs %d", f.FileNum, f.SmallestSeqNum, f.LargestSeqNum, largestSeqNum)
Expand Down
15 changes: 8 additions & 7 deletions internal/manifest/version_edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,18 +477,18 @@ func (b *BulkVersionEdit) Apply(
// be added and deleted). And we can delay checking consistency of it until we merge
// currFiles, addedFiles and deletedMap.
if level == 0 {
// - Note that any single sequence number (ssn) files contained inside a multi-sequence
// - Note that any ingested single sequence number (ssn) file contained inside a multi-sequence
// number (msn) file must have been added before the latter. So it is not possible for
// an ssn file to be in addedFiles and its corresponding msn file to be in currFiles,
// but the reverse is possible. So for consistency checking we may need to look back
// into currFiles for ssn files that overlap with an msn file in addedFiles. Also note
// that LargestSeqNums must be strictly increasing across the slice formed by
// concatenating addedFiles and currFiles and that this also is the desired sort order
// of the unioned set of files. See the CheckOrdering func in version.go for a more
// detailed explanation.
// into currFiles for ssn files that overlap with an msn file in addedFiles.
// - The previous bullet does not hold for sequence number 0 files that can be added
// later. See the CheckOrdering func in version.go for a detailed explanation.
// Due to these files, the LargestSeqNums may not be increasing across the slice formed by
// concatenating addedFiles and currFiles.
// - Instead of constructing a custom variant of the CheckOrdering logic, that is aware
// of the 2 slices, we observe that the number of L0 files is small so we can afford
// to repeat the full check on the concatenated slices (and CheckOrdering only does
// to repeat the full check on the combined slices (and CheckOrdering only does
// sequence num comparisons and not expensive key comparisons).
for _, ff := range [2][]FileMetadata{currFiles, addedFiles} {
for i := range ff {
Expand All @@ -503,6 +503,7 @@ func (b *BulkVersionEdit) Apply(
v.Files[level] = append(v.Files[level], *f)
}
}
SortBySeqNum(v.Files[level])
if err := CheckOrdering(cmp, format, 0, v.Files[level]); err != nil {
return nil, fmt.Errorf("pebble: internal error: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions tool/testdata/manifest_dump
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ MANIFEST-invalid
last-seq-num: 20
added: L0 2:0[#0,0-#0,0]
EOF
pebble: internal error: L0 files 000001 and 000002 are not properly ordered: 2-5 vs 1-4
pebble: internal error: L0 flushed file 000001 overlaps with the largest seqnum of a preceding flushed file: 2-5 vs 4

manifest check
./testdata/MANIFEST-invalid
----
MANIFEST-invalid: offset: 65 err: pebble: internal error: L0 files 000001 and 000002 are not properly ordered: 2-5 vs 1-4
MANIFEST-invalid: offset: 65 err: pebble: internal error: L0 flushed file 000001 overlaps with the largest seqnum of a preceding flushed file: 2-5 vs 4
Version state before failed Apply
--- L0 ---
1:0[#0,0-#0,0]
Expand Down

0 comments on commit 9509253

Please sign in to comment.