db: test iterator error while switching to forward direction

This commit adds datadriven test cases for an iterator error encountered while
switching from backward to forward iteration. It also fixes a case within
NextPrefix in which an `invariants` build would panic, improperly assuming
iterKey is non-nil (the only case in which it may be nil is if an error
occurred).

data_test.go

@@ -1368,6 +1368,16 @@ func parseDBOptionsArgs(opts *Options, args []datadriven.CmdArg) error {
 			for i := range opts.Levels {
 				opts.Levels[i].BlockSize = v
 			}
+		case "cache-size":
+			if opts.Cache != nil {
+				opts.Cache.Unref()
+				opts.Cache = nil
+			}
+			size, err := strconv.ParseInt(cmdArg.Vals[0], 10, 64)
+			if err != nil {
+				return err
+			}
+			opts.Cache = NewCache(size)
 		case "index-block-size":
 			v, err := strconv.Atoi(cmdArg.Vals[0])
 			if err != nil {

iterator.go

@@ -1772,6 +1772,16 @@ func (i *Iterator) nextPrefix() IterValidityState {
 			// key with the user key less than i.key, so we're guaranteed to
 			// land on the correct key with a single Next.
 			i.iterKey, i.iterValue = i.iter.Next()
+			if i.iterKey == nil {
+				// This should only be possible if i.iter.Next() encountered an
+				// error.
+				if i.iter.Error() == nil {
+					i.opts.logger.Fatalf("pebble: invariant violation: Nexting internal iterator from iterPosPrev found nothing")
+				}
+				// NB: Iterator.Error() will return i.iter.Error().
+				i.iterValidityState = IterExhausted
+				return i.iterValidityState
+			}
 			if invariants.Enabled && !i.equal(i.iterKey.UserKey, i.key) {
 				i.opts.logger.Fatalf("pebble: invariant violation: Nexting internal iterator from iterPosPrev landed on %q, not %q",
 					i.iterKey.UserKey, i.key)

iterator_histories_test.go

@@ -32,6 +32,7 @@ func TestIterHistories(t *testing.T) {
 		}
 
 		var d *DB
+		var refedCache bool
 		var buf bytes.Buffer
 		iters := map[string]*Iterator{}
 		batches := map[string]*Batch{}
@@ -54,9 +55,13 @@ func TestIterHistories(t *testing.T) {
 			opts.DisableAutomaticCompactions = true
 			opts.EnsureDefaults()
 			opts.WithFSDefaults()
+			originalCache := opts.Cache
 			if err := parseDBOptionsArgs(opts, td.CmdArgs); err != nil {
 				return nil, err
 			}
+			// If the test replaced the cache, we'll need to unref the
+			// new cache later.
+			refedCache = opts.Cache != originalCache
 			return opts, nil
 		}
 		cleanup := func() (err error) {
@@ -85,6 +90,12 @@ func TestIterHistories(t *testing.T) {
 				err = firstError(err, d.Close())
 				d = nil
 			}
+
+			if refedCache {
+				opts.Cache.Unref()
+				opts.Cache = nil
+				refedCache = false
+			}
 			return err
 		}
 		defer cleanup()
@@ -111,9 +122,13 @@ func TestIterHistories(t *testing.T) {
 				if err := cleanup(); err != nil {
 					return err.Error()
 				}
+				originalCache := opts.Cache
 				if err := parseDBOptionsArgs(opts, td.CmdArgs); err != nil {
 					return err.Error()
 				}
+				// If the test replaced the cache, we'll need to unref the
+				// new cache later.
+				refedCache = originalCache != opts.Cache
 				d, err = Open("", opts)
 				require.NoError(t, err)
 				return ""

testdata/iter_histories/errors

@@ -144,3 +144,81 @@ prev
 e: (e9, .)
 d: (d9, .)
 err=injected error
+
+reset
+----
+
+# Test the case of switching from reverse to forward iteration. During reverse
+# iteration, the underlying iterator may be positioned at an internal key with a
+# user key less than the current Key() [i.pos = iterPosPrev]. When switching
+# back to the forward direction, the iterator must be stepped forward to a user
+# key representing the current Key().
+#
+# Previously, if this step errored with the invariants tag enabled, a nil
+# pointer panic was possible as an invariant assertion assumed that i.iterKey
+# became non-nil (based on the knowledge that there must exist internal keys
+# that represent the current Key()). This test exercises this case for both
+# Next and NextPrefix.
+#
+# We set the cache-size to 1 so that no unused blocks are held in the cache, and
+# we know the Next of the internal iterator will need to read from disk.
+
+define auto-compactions=off cache-size=1 block-size=1
+L1
+  a.SET.1:a1
+  b.SET.1:b1
+  c.SET.1:c1
+  d.SET.1:d1
+  e.SET.1:e1
+----
+1:
+  000004:[a#1,SET-e#1,SET]
+
+layout filename=000004.sst
+----
+         0  data (22)
+        27  data (22)
+        54  data (22)
+        81  data (22)
+       108  data (22)
+       135  index (84)
+       224  properties (567)
+       796  meta-index (33)
+       834  footer (53)
+       887  EOF
+
+# NB: Block offset to key contained:
+#  0 -> a, 27 -> b, 54 -> c, 81 -> d, 108 -> e
+#
+# After the last prev in the below cases, the top-level pebble.Iterator is
+# positioned at c, but the underlying internal iterator is positioned at b.
+# We set up error injection to error when loading the block corresponding to 'c'
+# but only on the second tim the block is loaded.
+
+reopen cache-size=1 auto-compactions=off enable-table-stats=false inject-errors=((ErrInjected (And (PathMatch "000004.sst") (OpFileReadAt 54) (OnIndex 1))))
+----
+
+combined-iter
+last
+prev
+prev
+next-prefix
+----
+e: (e1, .)
+d: (d1, .)
+c: (c1, .)
+err=injected error
+
+reopen cache-size=1 auto-compactions=off enable-table-stats=false inject-errors=((ErrInjected (And (PathMatch "000004.sst") (OpFileReadAt 54) (OnIndex 1))))
+----
+
+combined-iter
+last
+prev
+prev
+next
+----
+e: (e1, .)
+d: (d1, .)
+c: (c1, .)
+err=injected error