Merge pull request #1135 from petermattis/pmattis/flush-empty-key

sstable: fix sstable.Writer key order checks for nil keys

flush_test.go

@@ -114,3 +114,17 @@ func TestFlushDelRangeEmptyKey(t *testing.T) {
  require.NoError(t, d.Flush())
  require.NoError(t, d.Close())
 }
+
+// TestFlushEmptyKey tests that flushing an empty key does not trigger that key
+// order invariant assertions.
+func TestFlushEmptyKey(t *testing.T) {
+ d, err := Open("", &Options{FS: vfs.NewMem()})
+ require.NoError(t, err)
+ require.NoError(t, d.Set(nil, []byte("hello"), nil))
+ require.NoError(t, d.Flush())
+ val, closer, err := d.Get(nil)
+ require.NoError(t, err)
+ require.Equal(t, val, []byte("hello"))
+ require.NoError(t, closer.Close())
+ require.NoError(t, d.Close())
+}

internal/base/internal.go

@@ -285,7 +285,7 @@ func (k InternalKey) Valid() bool {
 
 // Clone clones the storage for the UserKey component of the key.
 func (k InternalKey) Clone() InternalKey {
- if k.UserKey == nil {
+ if len(k.UserKey) == 0 {
  return k
  }
  return InternalKey{

sstable/writer.go

@@ -220,7 +220,7 @@ func (w *Writer) Add(key InternalKey, value []byte) error {
 }
 
 func (w *Writer) addPoint(key InternalKey, value []byte) error {
- if !w.disableKeyOrderChecks {
+ if !w.disableKeyOrderChecks && w.meta.LargestPoint.UserKey != nil {
  // TODO(peter): Manually inlined version of base.InternalCompare(). This is
  // 3.5% faster on BenchmarkWriter on go1.13. Remove if go1.14 or future
  // versions show this to not be a performance win.
@@ -246,12 +246,16 @@ func (w *Writer) addPoint(key InternalKey, value []byte) error {
  w.block.add(key, value)
 
  w.meta.updateSeqNum(key.SeqNum())
- if w.props.NumEntries == 0 {
- w.meta.SmallestPoint = key.Clone()
- }
  // block.curKey contains the most recently added key to the block.
  w.meta.LargestPoint.UserKey = w.block.curKey[:len(w.block.curKey)-8]
  w.meta.LargestPoint.Trailer = key.Trailer
+ if w.meta.SmallestPoint.UserKey == nil {
+ // NB: we clone w.meta.LargestPoint rather than "key", even though they are
+ // semantically identical, because we need to ensure that SmallestPoint.UserKey
+ // is not nil. This is required by WriterMetadata.Smallest in order to
+ // distinguish between an unset SmallestPoint and a zero-length one.
+ w.meta.SmallestPoint = w.meta.LargestPoint.Clone()
+ }
 
  w.props.NumEntries++
  switch key.Kind() {