Fix RBF checkpoint high water mark

Previously, the `checkpoint()` function determined the segments to drop
based on the current active transactions' WAL ID references. However, if
no transactions are active then the checkpoint would drop segments too
aggressively.

This changes the determination by using the highest WAL ID that is
actually checkpointed to disk to determine the high water mark. If no
page are checkpointed then no segments can be dropped.

rbf/db.go

@@ -187,6 +187,7 @@ func (db *DB) checkpoint() error {
 	// Loop over each transaction
 	walID++
 	pageMap := immutable.NewMap(&uint32Hasher{})
+	var maxCheckpointedWALID int64
 	for {
 		// Determine last page of transaction.
 		metaWALID, metaFlags, err := db.findNextWALMetaPage(walID)
@@ -240,20 +241,27 @@ func (db *DB) checkpoint() error {
 			if err := db.writePage(pgno, page); err != nil {
 				return err
 			}
+
+			// Track highest WALID that has been checkpointed back to disk.
+			if IsMetaPage(page) {
+				maxCheckpointedWALID = walID
+			}
 		}
 	}
 
 	// Remove WAL segments that have been checkpointed.
-	for len(db.segments) > 1 {
-		segment := db.segments[0]
-		if minActiveWALID != 0 && segment.MaxWALID() >= minActiveWALID {
-			break
-		}
+	if maxCheckpointedWALID != 0 {
+		for len(db.segments) > 1 {
+			segment := db.segments[0]
+			if segment.MaxWALID() >= maxCheckpointedWALID {
+				break
+			}
 
-		if err := segment.Close(); err != nil {
-			return err
+			if err := segment.Close(); err != nil {
+				return err
+			}
+			db.segments, db.segments[0] = db.segments[1:], nil
 		}
-		db.segments, db.segments[0] = db.segments[1:], nil
 	}
 
 	db.pageMap = pageMap
@@ -590,8 +598,10 @@ func (db *DB) removeTx(tx *Tx) error {
 
 	// Write pages from WAL to DB.
 	// TODO(bbj): Move this to an async goroutine.
-	if err := db.checkpoint(); err != nil {
-		return err
+	if tx.writable {
+		if err := db.checkpoint(); err != nil {
+			return err
+		}
 	}
 
 	delete(tx.db.txs, tx)

rbf/tx.go

@@ -106,18 +106,8 @@ func (tx *Tx) Rollback() {
 		}
 	}
 
-	// turn on these error checks! we see
-	// panic: cannot find segment containing WAL page: 1
-	// when running go test -v
-	// TestCursor_FirstNext_Quick/6
-	//
-	//panicOn(tx.db.checkpoint())
-	//panicOn(tx.db.removeTx(tx))
-
-	_ = tx.db.checkpoint()
-
 	// Disconnect transaction from DB.
-	_ = tx.db.removeTx(tx)
+	panicOn(tx.db.removeTx(tx))
 }
 
 // Root returns the root page number for a bitmap. Returns 0 if the bitmap does not exist.