internal/keyspan: document InterleavingIter error invariants

In #3021 (33a77e1) we fixed the InterleavingIter's error handling to propagate
all errors encountered by child iterators. This commit documents the invariants
more thoroughly.

Additionally the behavior of nextPos|prevPos is adjusted to set posExhausted if
on i.err != nil on entry. This fixes a remaining latent bug when changing
directions, resulting in two steps. Previously, if both steps were on the same
iterator and the first step errored but the second did not, the error could go
unnoticed.

Future work will unit test these cases explicitly using new error-injection
testing facilities.

internal/keyspan/interleaving_iter.go

@@ -113,7 +113,22 @@ type InterleavingIter struct {
 	nextPrefixBuf []byte
 	pointKey      *base.InternalKey
 	pointVal      base.LazyValue
-	err           error
+	// err holds an iterator error from either pointIter or keyspanIter. It's
+	// reset to nil on seeks. An overview of error-handling mechanics:
+	//
+	// Whenever either pointIter or keyspanIter is respositioned and a nil
+	// key/span is returned, the code performing the positioning is responsible
+	// for checking the iterator's Error() value. This happens in savePoint and
+	// saveSpan[Forward,Backward].
+	//
+	// Once i.err is non-nil, the computation of i.pos must set i.pos =
+	// posExhausted. This happens in compute[Smallest|Largest]Pos and
+	// [next|prev]Pos. Setting i.pos to posExhausted ensures we'll yield nil to
+	// the caller, which they'll interpret as a signal they must check Error().
+	//
+	// INVARIANTS:
+	// i.err != nil => i.pos = posExhausted
+	err error
 	// prefix records the iterator's current prefix if the iterator is in prefix
 	// mode. During prefix mode, Pebble will truncate spans to the next prefix.
 	// If the iterator subsequently leaves prefix mode, the existing span cached
@@ -601,6 +616,23 @@ func (i *InterleavingIter) computeLargestPos() {
 
 // nextPos advances the iterator one position in the forward direction.
 func (i *InterleavingIter) nextPos() {
+	if invariants.Enabled {
+		defer func() {
+			if i.err != nil && i.pos != posExhausted {
+				panic(errors.AssertionFailedf("iterator has accumulated error but i.pos = %d", i.pos))
+			}
+		}()
+	}
+	// NB: If i.err != nil or any of the positioning methods performed in this
+	// function result in i.err != nil, we must set i.pos = posExhausted. We
+	// perform this check explicitly here, but if any of the branches below
+	// advance either iterator, they must also check i.err and set posExhausted
+	// if necessary.
+	if i.err != nil {
+		i.pos = posExhausted
+		return
+	}
+
 	switch i.pos {
 	case posExhausted:
 		i.savePoint(i.pointIter.Next())
@@ -609,6 +641,10 @@ func (i *InterleavingIter) nextPos() {
 		i.computeSmallestPos()
 	case posPointKey:
 		i.savePoint(i.pointIter.Next())
+		if i.err != nil {
+			i.pos = posExhausted
+			return
+		}
 		// If we're not currently within the span, we want to chose the
 		// MIN(pointKey,span.Start), which is exactly the calculation performed
 		// by computeSmallestPos.
@@ -620,8 +656,6 @@ func (i *InterleavingIter) nextPos() {
 		// Since we previously were within the span, we want to choose the
 		// MIN(pointKey,span.End).
 		switch {
-		case i.err != nil:
-			i.pos = posExhausted
 		case i.span == nil:
 			panic("i.withinSpan=true and i.span=nil")
 		case i.pointKey == nil:
@@ -654,6 +688,23 @@ func (i *InterleavingIter) nextPos() {
 
 // prevPos advances the iterator one position in the reverse direction.
 func (i *InterleavingIter) prevPos() {
+	if invariants.Enabled {
+		defer func() {
+			if i.err != nil && i.pos != posExhausted {
+				panic(errors.AssertionFailedf("iterator has accumulated error but i.pos = %d", i.pos))
+			}
+		}()
+	}
+	// NB: If i.err != nil or any of the positioning methods performed in this
+	// function result in i.err != nil, we must set i.pos = posExhausted. We
+	// perform this check explicitly here, but if any of the branches below
+	// advance either iterator, they must also check i.err and set posExhausted
+	// if necessary.
+	if i.err != nil {
+		i.pos = posExhausted
+		return
+	}
+
 	switch i.pos {
 	case posExhausted:
 		i.savePoint(i.pointIter.Prev())
@@ -662,6 +713,10 @@ func (i *InterleavingIter) prevPos() {
 		i.computeLargestPos()
 	case posPointKey:
 		i.savePoint(i.pointIter.Prev())
+		if i.err != nil {
+			i.pos = posExhausted
+			return
+		}
 		// If we're not currently covered by the span, we want to chose the
 		// MAX(pointKey,span.End), which is exactly the calculation performed
 		// by computeLargestPos.
@@ -670,8 +725,6 @@ func (i *InterleavingIter) prevPos() {
 			return
 		}
 		switch {
-		case i.err != nil:
-			i.pos = posExhausted
 		case i.span == nil:
 			panic("withinSpan=true, but i.span == nil")
 		case i.pointKey == nil:
@@ -972,6 +1025,12 @@ func (i *InterleavingIter) verify(
 			panic("pebble: invariant violation: key < lower bound")
 		case k != nil && i.upper != nil && i.cmp(k.UserKey, i.upper) >= 0:
 			panic("pebble: invariant violation: key ≥ upper bound")
+		case i.err != nil && k != nil:
+			panic("pebble: invariant violation: accumulated error swallowed")
+		case i.err == nil && i.pointIter.Error() != nil:
+			panic("pebble: invariant violation: pointIter swallowed")
+		case i.err == nil && i.keyspanIter.Error() != nil:
+			panic("pebble: invariant violation: keyspanIter error swallowed")
 		}
 	}
 	return k, v