-
Notifications
You must be signed in to change notification settings - Fork 886
Implement new Giga GarbageCollector interface #3868
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.
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
e71f50b
6935ce7
42a667d
c9f3de9
2a10f31
f43b8b3
47a6dfd
45b80e0
29e7d7d
7ccdf29
9b96d16
c07fa75
a682ec3
9c0e746
5104f64
c15c7f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,6 +206,12 @@ func TestManifestNamesEveryField(t *testing.T) { | |
| // sitting in a config struct that configuration cannot address is exactly the kind of | ||
| // thing a replacement manager would otherwise try to map a key onto. | ||
| "KeepRecent", | ||
| // ExternalPruning is tagged mapstructure:"-" for a sharper reason than KeepRecent: it is | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This comment states as fact that |
||
| // only correct when this store is registered with a running StorageGarbageCollector, which | ||
| // is a property of how the process was wired and not something an operator can assert from | ||
| // app.toml. Exposing a key for it would let a node stand its pruner down with nothing to | ||
| // replace it, and the resulting unbounded growth is silent. | ||
| "ExternalPruning", | ||
| ) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| DBDirectory = string("") | ||
| Backend = string("pebbledb") | ||
| AsyncWriteBuffer = int(100) | ||
| KeepRecent = int(0) | ||
| KeepRecent = int(10000) | ||
| PruneIntervalSeconds = int(600) | ||
| ExternalPruning = bool(false) | ||
| EnableReadWriteMetrics = bool(false) | ||
| LogFilterParallelism = int(16) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,46 +5,79 @@ import ( | |
| "time" | ||
|
|
||
| littdb "github.com/sei-protocol/sei-chain/sei-db/db_engine/litt" | ||
| "github.com/sei-protocol/sei-chain/sei-db/management/gc" | ||
| ) | ||
|
|
||
| // LittBlockConfig configures a LittDB-backed types.BlockDB. | ||
| type LittBlockConfig struct { | ||
| // BlockDBConfig configures a LittDB-backed types.BlockDB. | ||
| type BlockDBConfig struct { | ||
| // Litt is the underlying LittDB configuration, including the data directory | ||
| // paths. The block store builds its two tables (blocks, qcs) on top of this | ||
| // DB. Required; use DefaultConfig to obtain one with sane defaults, then | ||
| // override fields as needed (e.g. Litt.Fsync, Litt.GCPeriod). | ||
| // paths. The block store builds its single table (see tableName, which holds | ||
| // blocks and QCs both) on top of this DB. Required; use DefaultConfig to obtain | ||
| // one with sane defaults, then override fields as needed (e.g. Litt.Fsync, | ||
| // Litt.GCPeriod). | ||
| Litt *littdb.Config | ||
|
|
||
| // Retention is the failsafe minimum age before any pruned record may be | ||
| // RetentionTime is the failsafe minimum age before any pruned record may be | ||
| // reclaimed. Reclamation requires BOTH this age to elapse AND the prune | ||
| // watermark to advance past the record, so even an over-eager watermark | ||
| // cannot delete data younger than Retention. Must be positive. | ||
| Retention time.Duration | ||
| // cannot delete data younger than RetentionTime. Must be positive. | ||
| // | ||
| // It is an age floor, not a retention policy: how much history this store | ||
| // keeps is RetentionWindow below. Raising this only delays reclaiming what | ||
| // the watermark has already released, which costs disk and buys nothing the | ||
| // window does not already express. | ||
| RetentionTime time.Duration | ||
|
|
||
| // RetentionWindow is how much history this store keeps beyond the shared rollback | ||
| // window of the StorageGarbageCollector that manages it, in blocks. It is what | ||
| // gc.PrunableStore.GetRetentionWindow answers: | ||
| // | ||
| // > 0 → that many blocks of history beyond the rollback window | ||
| // 0 → keep history to serve rollback window only | ||
| // -1 → never prune this store (gc.InfiniteRetentionWindow) | ||
| // | ||
| // Zero does NOT mean "keep everything" here, unlike the KeepRecent fields on | ||
| // StateStoreConfig and ReceiptStoreConfig, where 0 disables pruning. It is the most | ||
| // aggressive setting this field has; "keep everything" is -1. Assigning a KeepRecent | ||
| // value to this field inverts the retention it asks for. | ||
| // | ||
| // This is an input to a minimum shared across every managed store, not a policy applied | ||
| // to this store alone: a deep window here also holds back receiptDB and the SC/SS | ||
| // snapshots. Must be >= gc.InfiniteRetentionWindow. | ||
| // | ||
| // Independent of Retention, which is a wall-clock TTL failsafe underneath the watermark. | ||
| // Both must permit reclamation before any record is dropped. | ||
| RetentionWindow int64 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be useful to call out the following invariants:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest we paste this block of invariants at each RetentionWindow or RollbackWindow config (with wording adjusted a little, the above is specific to block storage). |
||
| } | ||
|
|
||
| // DefaultConfig returns a LittBlockConfig preloaded with all defaults, rooted at | ||
| // DefaultConfig returns a BlockDBConfig preloaded with all defaults, rooted at | ||
| // dir. Override fields as needed, then pass it to NewBlockDB (which validates). | ||
| func DefaultConfig(dir string) (*LittBlockConfig, error) { | ||
| func DefaultConfig(dir string) (*BlockDBConfig, error) { | ||
| littConfig, err := littdb.DefaultConfig(dir) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to build litt config: %w", err) | ||
| } | ||
| return &LittBlockConfig{ | ||
| Litt: littConfig, | ||
| Retention: 24 * time.Hour, | ||
| return &BlockDBConfig{ | ||
| Litt: littConfig, | ||
| RetentionTime: time.Hour, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This silently drops the TTL failsafe from 24h to 1h (the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Worth calling out that this is a live default change, not just a rename: Separately: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] The BlockDB age failsafe drops 24h → 1h here (the receipt side goes the other way, from |
||
| RetentionWindow: 10000, | ||
|
yzang2019 marked this conversation as resolved.
Outdated
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This default is a fleet-wide policy in a per-store field. As the |
||
| }, nil | ||
| } | ||
|
|
||
| // Validate performs a sanity check on the configuration. | ||
| func (c *LittBlockConfig) Validate() error { | ||
| func (c *BlockDBConfig) Validate() error { | ||
| if c == nil { | ||
| return fmt.Errorf("config is required") | ||
| } | ||
| if c.Litt == nil { | ||
| return fmt.Errorf("config.Litt is required") | ||
| } | ||
| if c.Retention <= 0 { | ||
| return fmt.Errorf("config.Retention must be positive (got %s)", c.Retention) | ||
| if c.RetentionTime <= 0 { | ||
| return fmt.Errorf("config.RetentionTime must be positive (got %s)", c.RetentionTime) | ||
| } | ||
| if c.RetentionWindow < gc.InfiniteRetentionWindow { | ||
| return fmt.Errorf("config.RetentionWindow must be >= %d (got %d)", | ||
| gc.InfiniteRetentionWindow, c.RetentionWindow) | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ package littblock | |
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "sync" | ||
| "sync/atomic" | ||
|
|
||
|
|
@@ -12,18 +14,27 @@ import ( | |
| "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" | ||
| ) | ||
|
|
||
| // ledgerTableName is the single table holding both blocks and QCs. They share | ||
| // one table so a crash leaves a contiguous write-order prefix spanning both | ||
| // record kinds (see NewBlockDB), which is what guarantees a persisted block is | ||
| // always covered by a persisted QC. | ||
| const ledgerTableName = "ledger" | ||
| // tableName is the single table holding blocks and QCs both, despite the name. They share one | ||
| // table so a crash leaves a contiguous write-order prefix spanning both record kinds (see | ||
| // NewBlockDB), which is what guarantees a persisted block is always covered by a persisted QC. | ||
| // | ||
| // This value is persisted layout, not just an identifier: littdb puts a table's data at | ||
| // <root>/<tableName>/segments, so changing it makes NewBlockDB open a fresh empty table while the | ||
| // old data sits untouched under the previous name — neither served nor reclaimed. refuseLegacyTable | ||
| // turns that into a startup error rather than a store that looks healthy and empty. | ||
| const tableName = "blocks" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] The comment you added here states the hazard precisely — a rename makes Since the check is cheap and the comment already argues for it:
yzang2019 marked this conversation as resolved.
|
||
|
|
||
| // legacyTableName is what tableName was called before the rename. Nothing opens it; it exists only | ||
| // so refuseLegacyTable can recognize a directory written before the rename. | ||
| const legacyTableName = "ledger" | ||
|
|
||
| var _ types.BlockDB = (*blockDB)(nil) | ||
|
|
||
| // blockDB is a durable types.BlockDB backed by LittDB | ||
| type blockDB struct { | ||
| db littdb.DB | ||
| table littdb.Table | ||
| db littdb.DB | ||
| table littdb.Table | ||
| config *BlockDBConfig | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This field is assigned once ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] Struct fields aren't caught by the |
||
|
|
||
| // watermark is a retention floor, always a QC boundary (a GlobalRange().First): | ||
| // PruneBefore rounds a requested prune point down to the start of the cohort | ||
|
|
@@ -74,13 +85,18 @@ type blockDB struct { | |
| } | ||
|
|
||
| // NewBlockDB opens (or creates) a LittDB-backed types.BlockDB from config. The | ||
| // underlying LittDB is built from config.Litt, and the two tables apply | ||
| // config.Retention as a TTL failsafe (pruning never reclaims data younger than | ||
| // that even once the watermark has advanced past it). | ||
| func NewBlockDB(config *LittBlockConfig) (types.BlockDB, error) { | ||
| // underlying LittDB is built from config.Litt, and the table applies | ||
| // config.RetentionTime as a TTL failsafe (pruning never reclaims data younger | ||
| // than that even once the watermark has advanced past it). | ||
| func NewBlockDB(config *BlockDBConfig) (types.BlockDB, error) { | ||
| if err := config.Validate(); err != nil { | ||
| return nil, fmt.Errorf("invalid block db config: %w", err) | ||
| } | ||
| // Before littbuilder.NewDB, so a refused open leaves the directory exactly as it found it | ||
| // rather than adding an empty table beside the one it is complaining about. | ||
| if err := refuseLegacyTable(config.Litt.Paths); err != nil { | ||
| return nil, err | ||
| } | ||
| db, err := littbuilder.NewDB(config.Litt) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to open litt db: %w", err) | ||
|
|
@@ -96,8 +112,8 @@ func NewBlockDB(config *LittBlockConfig) (types.BlockDB, error) { | |
| // guarantees a persisted block is always covered by a persisted QC. It also | ||
| // backs the write-order cursors and contiguous-QC recovery. ShardingFactor | ||
| // > 1, or splitting blocks and QCs across two tables, would void this. | ||
| tableConfig := littdb.DefaultTableConfig(ledgerTableName) | ||
| tableConfig.TTL = config.Retention | ||
| tableConfig := littdb.DefaultTableConfig(tableName) | ||
| tableConfig.TTL = config.RetentionTime | ||
| tableConfig.GCFilter = s.gcFilter | ||
| tableConfig.ShardingFactor = 1 // DO NOT CHANGE!! | ||
| table, err := db.BuildTable(tableConfig) | ||
|
|
@@ -107,6 +123,7 @@ func NewBlockDB(config *LittBlockConfig) (types.BlockDB, error) { | |
| } | ||
|
|
||
| s.table = table | ||
| s.config = config | ||
|
|
||
| if err := s.recoverCursors(); err != nil { | ||
| _ = db.Close() | ||
|
|
@@ -119,6 +136,37 @@ func NewBlockDB(config *LittBlockConfig) (types.BlockDB, error) { | |
| return s, nil | ||
| } | ||
|
|
||
| // refuseLegacyTable fails the open when any root path holds a table directory under | ||
| // legacyTableName. Such a directory is blocks and QCs that this process cannot reach: littdb | ||
| // resolves a table to <root>/<tableName>/segments, so the data is neither served nor reclaimed, and | ||
| // the store would otherwise present itself as healthy and empty. An empty store is indistinguishable | ||
| // from a correct one until something asks for history that is no longer there, which makes it the | ||
| // worst shape this failure could take. | ||
| // | ||
| // No deployment can reach this — nothing has ever run littblock with the old name persisted — so it | ||
| // exists for dev, CI, and devnet homes written before the rename. The operator action is to delete | ||
| // the directory (or move it aside); this check can be deleted once no such directory remains. | ||
| // | ||
| // A root that cannot be stat'd is refused for the same reason it is refused when the directory is | ||
| // present: an unreadable root cannot rule out data hiding under the old name. | ||
| func refuseLegacyTable(paths []string) error { | ||
| for _, root := range paths { | ||
| legacy := filepath.Join(root, legacyTableName) | ||
| switch _, err := os.Stat(legacy); { | ||
| case err == nil: | ||
| return fmt.Errorf( | ||
| "block db: found a pre-rename %q table at %s; the table is now named %q, so those "+ | ||
| "blocks and QCs would be neither served nor reclaimed. Delete or move the "+ | ||
| "directory aside to start from an empty store", | ||
| legacyTableName, legacy, tableName) | ||
| case !os.IsNotExist(err): | ||
| return fmt.Errorf("block db: check for a pre-rename %q table at %s: %w", | ||
| legacyTableName, legacy, err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // recoverCursors reloads the write-order cursors (lastBlockNumber, lastQCNext, | ||
| // and their presence flags) from on-disk state. Without this, a reopened DB | ||
| // would treat itself as empty and let WriteBlock/WriteQC silently accept | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] I confirmed the safety claim for seid:
readReceiptStoreConfig(app/receipt_store_config.go:27) overwritesKeepRecentunconditionally frommin-retain-blocks, andKeepRecentismapstructure:"-"so the app.toml template is unaffected. So no node changes behavior here.The one caller worth flagging is
DefaultGigaStorageConfig(sei-db/config/giga_config.go:58), which now hands a receipt storeKeepRecent = 10000instead of "keep everything". It has no production caller today, but it is the Giga wiring path this PR is building toward — a node constructed from it would serveeth_getTransactionReceiptfor only ~10k blocks, and nothing in that path re-derivesKeepRecentfrommin-retain-blocks. Consider setting it explicitly inDefaultGigaStorageConfig(or pinning it ingiga_config_test.go) so the wiring PR can't inherit this default by accident.