diff --git a/sstable/block.go b/sstable/block.go index 4ed82aa439..0b360cd222 100644 --- a/sstable/block.go +++ b/sstable/block.go @@ -432,10 +432,23 @@ type blockIter struct { // blockB: a@1,b@2,c@1 with syntheticSuffix=3 // // To ensure this, Suffix replacement will not change the ordering of keys in - // the block because the iter assumes the following about the block: + // the block because the iter assumes that no two keys in the block share the + // same prefix. Furthermore, during SeekGE and SeekLT operations, the block + // iterator handles "off by one" errors (explained in more detail in those + // functions) when, for a given key, originalSuffix < searchSuffix < + // replacementSuffix, with integer comparison. To handle these cases, the + // iterator assumes: + // + // pebble.Compare(keyPrefix{replacementSuffix},keyPrefix{originalSuffix}) < 0 + // for keys with a suffix. + // + // NB: it is possible for a block iter to add a synthetic suffix on a key + // without a suffix, which implies + // pebble.Compare(keyPrefix{replacementSuffix},keyPrefix{noSuffix}) > 0 , + // however, the iterator would never need to handle an off by one error in + // this case since originalSuffix (empty) > searchSuffix (non empty), with + // integer comparison. // - // (1) no two keys in the block share the same prefix. - // (2) pebble.Compare(keyPrefix{replacementSuffix},keyPrefix{originalSuffix}) < 0 // // In addition, we also assume that any block with rangekeys will not contain // a synthetic suffix. diff --git a/sstable/block_test.go b/sstable/block_test.go index 282ce094d7..7ced2526e4 100644 --- a/sstable/block_test.go +++ b/sstable/block_test.go @@ -391,7 +391,6 @@ func (c *checker) check( } func TestBlockSyntheticSuffix(t *testing.T) { - // TODO(msbutler): add test where replacement suffix has fewer bytes than previous suffix expectedSuffix := "15" synthSuffix := []byte("@" + expectedSuffix) @@ -405,7 +404,7 @@ func TestBlockSyntheticSuffix(t *testing.T) { suffixWriter, expectedSuffixWriter := &blockWriter{restartInterval: restarts}, &blockWriter{restartInterval: restarts} keys := []string{ - "apple@2", "apricot@2", "banana@13", + "apple@2", "apricot@2", "banana@13", "cantaloupe", "grape@2", "orange@14", "peach@4", "pear@1", "persimmon@4", } @@ -476,6 +475,25 @@ func TestBlockSyntheticSuffix(t *testing.T) { // Ensure off by one handling works at end of iterator c.check(expect.Last())(got.Last()) c.check(expect.SeekLT([]byte("apple@10"), base.SeekLTFlagsNone))(got.SeekLT([]byte("apple@10"), base.SeekLTFlagsNone)) + + // Try searching on the suffixless key + c.check(expect.First())(got.First()) + c.check(expect.SeekGE([]byte("cantaloupe"), base.SeekGEFlagsNone))(got.SeekGE([]byte("cantaloupe"), base.SeekGEFlagsNone)) + + c.check(expect.First())(got.First()) + c.check(expect.SeekGE([]byte("cantaloupe@16"), base.SeekGEFlagsNone))(got.SeekGE([]byte("cantaloupe@16"), base.SeekGEFlagsNone)) + + c.check(expect.First())(got.First()) + c.check(expect.SeekGE([]byte("cantaloupe@14"), base.SeekGEFlagsNone))(got.SeekGE([]byte("cantaloupe@14"), base.SeekGEFlagsNone)) + + c.check(expect.Last())(got.Last()) + c.check(expect.SeekLT([]byte("cantaloupe"), base.SeekLTFlagsNone))(got.SeekLT([]byte("cantaloupe"), base.SeekLTFlagsNone)) + + c.check(expect.Last())(got.Last()) + c.check(expect.SeekLT([]byte("cantaloupe10"), base.SeekLTFlagsNone))(got.SeekLT([]byte("cantaloupe10"), base.SeekLTFlagsNone)) + + c.check(expect.Last())(got.Last()) + c.check(expect.SeekLT([]byte("cantaloupe16"), base.SeekLTFlagsNone))(got.SeekLT([]byte("cantaloupe16"), base.SeekLTFlagsNone)) }) } }