manifest: improve VersionEdit tests

In this commit we add a `ParseVersionEditDebug` which can create a `VersionEdit` from a `DebugString` output (similar to `ParseFileMetadataDebug` and `ParseVersionDebug`). We improve `TestVersionEditApply` to use the `DebugString` formats instead of a one-off format.
cockroachdb · Mar 11, 2024 · 4bf09d5 · 4bf09d5
1 parent a034560
commit 4bf09d5
Show file tree

Hide file tree

Showing 5 changed files with 296 additions and 237 deletions.
diff --git a/internal/manifest/testdata/version_edit_apply b/internal/manifest/testdata/version_edit_apply
@@ -1,180 +1,171 @@
-apply
- L0
-  1:[a#1,SET-b#2,SET]
-  2:[c#3,SET-d#4,SET]
-edit
- delete
-  L0
-   1
- add
-  L2
-  1:[a#1,SET-b#2,SET]
-  4:[c#3,SET-d#4,SET]
+define v1
+L0:
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4]
+  000001:[a#1,SET-b#2,SET] seqnums:[1-2]
 ----
 L0.0:
-  000002:[c#3,SET-d#4,SET]
+  000001:[a#1,SET-b#2,SET] seqnums:[1-2] points:[a#1,SET-b#2,SET]
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
+
+# Empty edit.
+apply v1
+----
+L0.0:
+  000001:[a#1,SET-b#2,SET] seqnums:[1-2] points:[a#1,SET-b#2,SET]
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
+
+apply v1
+  deleted: L0 000001
+  added:   L2 000001:[a#1,SET-b#2,SET] seqnums:[1-2]
+  added:   L2 000004:[c#3,SET-d#4,SET] seqnums:[3-4]
+----
+L0.0:
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
 L2:
-  000001:[a#1,SET-b#2,SET]
-  000004:[c#3,SET-d#4,SET]
+  000001:[a#1,SET-b#2,SET] seqnums:[1-2] points:[a#1,SET-b#2,SET]
+  000004:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
 
-apply
- L0
-  1:[a#1,SET-b#2,SET]
-  2:[c#3,SET-d#4,SET]
-edit
- delete
-  L1
-   1
+apply v1
+  deleted: L1 000001
 ----
 pebble: internal error: No current or added files but have deleted files: 1
 
-apply
- L0
-  1:[a#1,SET-c#2,SET]
-  2:[c#3,SET-d#4,SET]
-edit
- delete
-  L0
-   1
- add
-  L2
-   1:[a#1,SET-c#2,SET]
-   4:[b#3,SET-d#4,SET]
-----
-pebble: internal error: L2 files 000001 and 000004 have overlapping ranges: [a#1,SET-c#2,SET] vs [b#3,SET-d#4,SET]
-
-apply
- L0
-  1:[a#1,SET-c#2,SET]
-  2:[c#3,SET-d#4,SET]
-edit
- add
-  L0
-   4:[b#3,SET-d#5,SET]
+apply v1
+  deleted: L0 000001
+  added:   L2 000001:[a#1,SET-b#2,SET] seqnums:[1-2]
+  added:   L2 000004:[b#3,SET-d#4,SET] seqnums:[3-4]
+----
+pebble: internal error: L2 files 000001 and 000004 have overlapping ranges: [a#1,SET-b#2,SET] vs [b#3,SET-d#4,SET]
+
+define v2
+L0:
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4]
+  000001:[a#1,SET-c#2,SET] seqnums:[1-2]
+----
+L0.1:
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
+L0.0:
+  000001:[a#1,SET-c#2,SET] seqnums:[1-2] points:[a#1,SET-c#2,SET]
+
+apply v2
+  added:   L0 000004:[b#3,SET-d#5,SET] seqnums:[3-5]
 ----
 L0.2:
-  000004:[b#3,SET-d#5,SET]
+  000004:[b#3,SET-d#5,SET] seqnums:[3-5] points:[b#3,SET-d#5,SET]
 L0.1:
-  000002:[c#3,SET-d#4,SET]
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
 L0.0:
-  000001:[a#1,SET-c#2,SET]
+  000001:[a#1,SET-c#2,SET] seqnums:[1-2] points:[a#1,SET-c#2,SET]
 
-apply
- L0
-   1:[a#1,SET-c#2,SET]
-   2:[c#3,SET-d#4,SET]
-edit
- add
-  L0
-   4:[b#0,SET-d#0,SET]
+apply v2
+  added:   L0 000004:[b#0,SET-d#0,SET] seqnums:[0-0]
 ----
 L0.2:
-  000002:[c#3,SET-d#4,SET]
+  000002:[c#3,SET-d#4,SET] seqnums:[3-4] points:[c#3,SET-d#4,SET]
 L0.1:
-  000001:[a#1,SET-c#2,SET]
+  000001:[a#1,SET-c#2,SET] seqnums:[1-2] points:[a#1,SET-c#2,SET]
 L0.0:
-  000004:[b#0,SET-d#0,SET]
+  000004:[b#0,SET-d#0,SET] seqnums:[0-0] points:[b#0,SET-d#0,SET]
+
 
+define empty
+----
 
-apply
-edit
- add
-  L0
-   1:[a#1,SET-c#2,SET]
-   4:[b#3,SET-d#5,SET]
+apply empty
+  added:   L0 000001:[a#1,SET-c#2,SET] seqnums:[1-2]
+  added:   L0 000004:[b#3,SET-d#5,SET] seqnums:[3-5]
 ----
 L0.1:
-  000004:[b#3,SET-d#5,SET]
+  000004:[b#3,SET-d#5,SET] seqnums:[3-5] points:[b#3,SET-d#5,SET]
 L0.0:
-  000001:[a#1,SET-c#2,SET]
+  000001:[a#1,SET-c#2,SET] seqnums:[1-2] points:[a#1,SET-c#2,SET]
+
+apply v2
+  added:   L0 000001:[a#1,SET-c#2,SET] seqnums:[1-2]
+----
+pebble: files 000001 and 000001 collided on sort keys
 
-apply
- L0
-  1:[a#1,SET-c#2,SET]
+apply empty
+  added:   L0 000001:[a#1,SET-c#2,SET] seqnums:[1-2]
 ----
 L0.0:
-  000001:[a#1,SET-c#2,SET]
-
-apply
- L2
-  3:[b#1,SET-c#2,SET]
-  4:[d#3,SET-f#4,SET]
-  5:[h#3,SET-h#2,SET]
-  2:[n#5,SET-q#3,SET]
-  1:[r#2,SET-t#1,SET]
-edit
- delete
-  L2
-   4
-   1
- add
-  L2
-   6:[a#10,SET-a#7,SET]
-   7:[e#1,SET-g#2,SET]
-   10:[j#3,SET-m#2,SET]
+  000001:[a#1,SET-c#2,SET] seqnums:[1-2] points:[a#1,SET-c#2,SET]
+
+apply empty
+  added:   L2 000010:[j#3,SET-m#2,SET]
+  added:   L2 000006:[a#10,SET-a#7,SET]
 ----
 L2:
-  000006:[a#10,SET-a#7,SET]
+  000006:[a#10,SET-a#7,SET] seqnums:[0-0] points:[a#10,SET-a#7,SET]
+  000010:[j#3,SET-m#2,SET] seqnums:[0-0] points:[j#3,SET-m#2,SET]
+
+define v3
+L2:
   000003:[b#1,SET-c#2,SET]
-  000007:[e#1,SET-g#2,SET]
+  000004:[d#3,SET-f#4,SET]
   000005:[h#3,SET-h#2,SET]
-  000010:[j#3,SET-m#2,SET]
   000002:[n#5,SET-q#3,SET]
+  000001:[r#2,SET-t#1,SET]
+----
+L2:
+  000003:[b#1,SET-c#2,SET] seqnums:[0-0] points:[b#1,SET-c#2,SET]
+  000004:[d#3,SET-f#4,SET] seqnums:[0-0] points:[d#3,SET-f#4,SET]
+  000005:[h#3,SET-h#2,SET] seqnums:[0-0] points:[h#3,SET-h#2,SET]
+  000002:[n#5,SET-q#3,SET] seqnums:[0-0] points:[n#5,SET-q#3,SET]
+  000001:[r#2,SET-t#1,SET] seqnums:[0-0] points:[r#2,SET-t#1,SET]
 
-apply
-edit
- add
-  L2
-   10:[j#3,SET-m#2,SET]
-   6:[a#10,SET-a#7,SET]
+apply v3
+  deleted:  L2 000004
+  deleted:  L2 000001
+  added:    L2 000006:[a#10,SET-a#7,SET]
+  added:    L2 000007:[e#1,SET-g#2,SET]
+  added:    L2 000010:[j#3,SET-m#2,SET]
 ----
 L2:
-  000006:[a#10,SET-a#7,SET]
-  000010:[j#3,SET-m#2,SET]
-
-apply
- L0
-  1:[a#1,SET-b#2,SET]
- L1
-  2:[c#3,SET-d#2,SET]
-edit
- delete
-  L0
-   1
-  L1
-   2
+  000006:[a#10,SET-a#7,SET] seqnums:[0-0] points:[a#10,SET-a#7,SET]
+  000003:[b#1,SET-c#2,SET] seqnums:[0-0] points:[b#1,SET-c#2,SET]
+  000007:[e#1,SET-g#2,SET] seqnums:[0-0] points:[e#1,SET-g#2,SET]
+  000005:[h#3,SET-h#2,SET] seqnums:[0-0] points:[h#3,SET-h#2,SET]
+  000010:[j#3,SET-m#2,SET] seqnums:[0-0] points:[j#3,SET-m#2,SET]
+  000002:[n#5,SET-q#3,SET] seqnums:[0-0] points:[n#5,SET-q#3,SET]
+
+define v4
+L0:
+  000001:[a#1,SET-b#2,SET]
+L1:
+  000002:[c#3,SET-d#2,SET]
+----
+L0.0:
+  000001:[a#1,SET-b#2,SET] seqnums:[0-0] points:[a#1,SET-b#2,SET]
+L1:
+  000002:[c#3,SET-d#2,SET] seqnums:[0-0] points:[c#3,SET-d#2,SET]
+
+apply v4
+  deleted: L0 000001
+  deleted: L1 000002
 ----
 
 # Deletion of a non-existent table results in an error.
+apply v4
+  deleted: L0 000004
+----
+error during Accumulate: pebble: file deleted L0.000004 before it was inserted
+
+define v5
+L0:
+  000001:[a#1,SET-b#2,SET]
+----
+L0.0:
+  000001:[a#1,SET-b#2,SET] seqnums:[0-0] points:[a#1,SET-b#2,SET]
 
-apply
- L0
-  1:[a#1,SET-b#2,SET]
-edit
- delete
-  L0
-   2
-----
-pebble: file deleted L0.000002 before it was inserted
-
-apply
- L0
-  1:[a#1,SET-b#2,SET]
-edit
- delete
-  L0
-   1
- add
-  L2
-  1:[a#1,SET-b#2,SET]
-  4:[c#3,SET-d#4,SET]
-  5:[s#3,SET-z#4,SET]
-edit
-  delete
-    L2
-     1
-    L2
-     4
+apply v5
+  deleted: L0 1
+  added: L2 000001:[a#1,SET-b#2,SET]
+  added: L2 000004:[c#3,SET-d#4,SET]
+  added: L2 000005:[s#3,SET-z#4,SET]
+new version edit
+  deleted: L2 000001
+  deleted: L2 000004
 ----
 L2:
-  000005:[s#3,SET-z#4,SET]
+  000005:[s#3,SET-z#4,SET] seqnums:[0-0] points:[s#3,SET-z#4,SET]
diff --git a/internal/manifest/testutils.go b/internal/manifest/testutils.go
@@ -78,6 +78,13 @@ func (p *debugParser) Next() string {
 	return res
 }
 
+// Remaining returns all the remaining tokens, separated by spaces.
+func (p *debugParser) Remaining() string {
+	res := strings.Join(p.tokens, " ")
+	p.tokens = nil
+	return res
+}
+
 // Expect consumes the next tokens, verifying that they exactly match the
 // arguments.
 func (p *debugParser) Expect(tokens ...string) {
@@ -142,3 +149,15 @@ func (p *debugParser) Errf(format string, args ...any) {
 	msg := fmt.Sprintf(format, args...)
 	panic(errors.Errorf("error parsing %q at token %q: %s", p.original, p.lastToken, msg))
 }
+
+// maybeRecover can be used in a defer to convert panics into errors.
+func maybeRecover() error {
+	if r := recover(); r != nil {
+		err, ok := r.(error)
+		if !ok {
+			err = errors.Errorf("%v", r)
+		}
+		return err
+	}
+	return nil
+}
diff --git a/internal/manifest/version.go b/internal/manifest/version.go
@@ -814,13 +814,7 @@ func (m *FileMetadata) DebugString(format base.FormatKey, verbose bool) string {
 // representation.
 func ParseFileMetadataDebug(s string) (_ *FileMetadata, err error) {
 	defer func() {
-		if r := recover(); r != nil {
-			var ok bool
-			err, ok = r.(error)
-			if !ok {
-				err = errors.Errorf("%v", r)
-			}
-		}
+		err = errors.CombineErrors(err, maybeRecover())
 	}()
 
 	// Input format:
@@ -1256,6 +1250,9 @@ func ParseVersionDebug(comparer *base.Comparer, flushSplitBytes int64, s string)
 	var files [NumLevels][]*FileMetadata
 	level := -1
 	for _, l := range strings.Split(s, "\n") {
+		if l == "" {
+			continue
+		}
 		p := makeDebugParser(l)
 		if l, ok := p.TryLevel(); ok {
 			level = l