Skip to content

Fix pruning issue in SS causing huge disk spike #4321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alrevuelta merged 2 commits into from
Sep 24, 2026
Merged

Fix pruning issue in SS causing huge disk spike #4321

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
184297f
fix prunnng open iterator
alrevuelta Sep 24, 2026
e615a10
Merge branch 'main' into prunning-issue
alrevuelta Sep 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions sei-db/db_engine/pebbledb/mvcc/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (

dbm "github.com/tendermint/tm-db"

"github.com/sei-protocol/seilog"

errorutils "github.com/sei-protocol/sei-chain/sei-db/common/errors"
seidbmetrics "github.com/sei-protocol/sei-chain/sei-db/common/metrics"
"github.com/sei-protocol/sei-chain/sei-db/common/utils"
Expand All @@ -33,6 +35,8 @@ import (

var _ types.ContextIteratorStore = (*Database)(nil)

var logger = seilog.NewLogger("db", "db-engine", "pebbledb", "mvcc")

const (
VersionSize = 8

Expand Down Expand Up @@ -953,7 +957,22 @@ func (db *Database) pruneDescending(version int64) (_err error) {
if err != nil {
return err
}
defer func() { _ = itr.Close() }()
// itr pins Pebble's readState for as long as it's open, which blocks Pebble
// from deleting any sstable that readState might still need to serve reads
// from — including ones a concurrent or subsequent compaction (see
// compactPrunedRange below) has already superseded. itrOpen guards against
// closing it twice: the scan closes it explicitly as soon as it's done
// reading, right before compaction, and this defer only still applies if an
// error returned from inside the loop below.
itrOpen := true

@seidroid seidroid Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] AGENTS.md ('Structural corrections') asks that this kind of fix be made by naming a step, not by adding an itrOpen flag, a closeItr closure, and two long inline comments. Consider moving the scan into its own function, e.g. scanAndDeletePruned(...) (first, last []byte, err error), that opens the iterator and closes it with a plain defer. pruneDescending then reads as scan → commit → compact, and the iterator is closed before compaction by construction. The note about readState pinning belongs in that function's doc comment. pruneAscending has the same bug and could use the same shape.

closeItr := func() error {
if !itrOpen {
return nil
}
itrOpen = false
return itr.Close()
}
defer func() { _ = closeItr() }()

batch := db.storage.NewBatch()
defer func() { _ = batch.Close() }()
Expand Down Expand Up @@ -1055,6 +1074,15 @@ func (db *Database) pruneDescending(version int64) (_err error) {
itr.Next()
}

// Close the scan iterator now rather than leaving it to the deferred close
// at function return: compactPrunedRange below runs a compaction that can
// take a long time on a large deleted span, and every obsolete sstable it
// produces stays undeletable for as long as this iterator's readState is
// still pinning them.
if err := closeItr(); err != nil {
return err
}

// Commit any leftover delete ops in batch
if counter > 0 {
writeCount := int64(batch.Count())
Expand All @@ -1066,7 +1094,10 @@ func (db *Database) pruneDescending(version int64) (_err error) {
}
db.operationMetrics.AddRead(scanReads)

return db.compactPrunedRange(firstDeletedKey, lastDeletedKey)
compactStart := time.Now()
err = db.compactPrunedRange(firstDeletedKey, lastDeletedKey)
logger.Info("pruneDescending: compacted pruned range", "version", version, "elapsed", time.Since(compactStart), "err", err)

@seidroid seidroid Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] This Info line runs on every prune pass, even when nothing was deleted and compactPrunedRange returned immediately (first == nil). It also logs failures at Info as err=<error>. Consider logging only when a compaction actually ran (firstDeletedKey != nil), and moving failures to Error/Warn or leaving them to the caller, which already receives the returned error.

return err

@cursor cursor Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ascending prune still pins iterator

Medium Severity

The early iterator close before compactPrunedRange is only applied in pruneDescending. pruneAscending still defers itr.Close() until after compaction, so the scan iterator keeps Pebble's readState pinned and obsolete sstables stay undeletable. Legacy and archive nodes on the ascending path still hit the same disk spike this change targets.

Fix in Cursor Fix in Web

Triggered by learned rule: sei-db pebbledb: ascending and descending MVCC iterators must stay aligned

Reviewed by Cursor Bugbot for commit 184297f. Configure here.

}

func (db *Database) iteratorDescending(
Expand Down
Loading