go/store/nbs: added sentinel error for read-only manifest, fixed true-up

go/store/nbs/journal.go

@@ -155,13 +155,18 @@ func trueUpBackingManifest(ctx context.Context, root hash.Hash, backing manifest
 	mc.root = root
 
 	mc, err = backing.Update(ctx, prev, mc, &Stats{}, nil)
-	if err != nil {
+	if errors.Is(err, errReadOnlyManifest) {
+		// set our in-memory root to match the journal
+		mc.root = root
+		return mc, nil
+	} else if err != nil {
 		return manifestContents{}, err
 	} else if mc.lock != next {
 		return manifestContents{}, errOptimisticLockFailedTables
 	} else if mc.root != root {
 		return manifestContents{}, errOptimisticLockFailedRoot
 	}
+	// true-up succeeded
 	return mc, nil
 }
 
@@ -425,9 +430,8 @@ func newJournalManifest(ctx context.Context, dir string) (m manifest, err error)
 		return m, nil
 	}
 	defer func() {
-		// keep first error
 		if cerr := f.Close(); err == nil {
-			err = cerr
+			err = cerr // keep first error
 		}
 	}()
 
@@ -452,11 +456,7 @@ func (jm journalManifest) Name() string {
 }
 
 // ParseIfExists implements manifest.
-func (jm journalManifest) ParseIfExists(
-	ctx context.Context,
-	stats *Stats,
-	readHook func() error,
-) (exists bool, contents manifestContents, err error) {
+func (jm journalManifest) ParseIfExists(ctx context.Context, stats *Stats, readHook func() error) (exists bool, contents manifestContents, err error) {
 	t1 := time.Now()
 	defer func() { stats.ReadManifestLatency.SampleTimeSince(t1) }()
 	return parseIfExists(ctx, jm.dir, readHook)
@@ -465,7 +465,12 @@ func (jm journalManifest) ParseIfExists(
 // Update implements manifest.
 func (jm journalManifest) Update(ctx context.Context, lastLock addr, newContents manifestContents, stats *Stats, writeHook func() error) (mc manifestContents, err error) {
 	if jm.readOnly {
-		return manifestContents{}, errors.New("cannot update manifest: database is read only")
+		_, mc, err = jm.ParseIfExists(ctx, stats, nil)
+		if err != nil {
+			return manifestContents{}, err
+		}
+		// return current contents and sentinel error
+		return mc, errReadOnlyManifest
 	}
 
 	t1 := time.Now()
@@ -482,7 +487,12 @@ func (jm journalManifest) Update(ctx context.Context, lastLock addr, newContents
 // UpdateGCGen implements manifest.
 func (jm journalManifest) UpdateGCGen(ctx context.Context, lastLock addr, newContents manifestContents, stats *Stats, writeHook func() error) (mc manifestContents, err error) {
 	if jm.readOnly {
-		return manifestContents{}, errors.New("cannot update manifest: database is read only")
+		_, mc, err = jm.ParseIfExists(ctx, stats, nil)
+		if err != nil {
+			return manifestContents{}, err
+		}
+		// return current contents and sentinel error
+		return mc, errReadOnlyManifest
 	}
 
 	t1 := time.Now()

go/store/nbs/store.go

@@ -1154,6 +1154,7 @@ var (
 	errLastRootMismatch           = fmt.Errorf("last does not match nbs.Root()")
 	errOptimisticLockFailedRoot   = fmt.Errorf("root moved")
 	errOptimisticLockFailedTables = fmt.Errorf("tables changed")
+	errReadOnlyManifest           = fmt.Errorf("cannot update manifest: database is read only")
 )
 
 // callers must acquire lock |nbs.mu|

integration-tests/bats/sql-server.bats

@@ -34,7 +34,6 @@ teardown() {
 
 @test "sql-server: can create savepoint when no database is selected" {
     skiponwindows "Missing dependencies"
-    skip "currently fails with: Error 1105: plan is not resolved because of node '*plan.CreateSavepoint' in server log"
 
     mkdir my-db
     cd my-db
@@ -179,7 +178,7 @@ SQL
 }
 
 
-@test "sql-server: test command line modification" {
+@test "sql-server: inspect sql-server using CLI" {
     skiponwindows "Missing dependencies"
 
     cd repo1
@@ -195,7 +194,7 @@ SQL
         c1 BIGINT,
         c2 BIGINT,
         PRIMARY KEY (pk))"
-    
+
     run dolt ls
     [ "$status" -eq 0 ]
     [[ "$output" =~ "one_pk" ]] || false
@@ -226,6 +225,7 @@ SQL
     start_sql_server repo1
 
     # No tables at the start
+    dolt ls
     run dolt ls
     [ "$status" -eq 0 ]
     [[ "$output" =~ "No tables in working set" ]] || false