Repository navigation
feat: return ledger + accountsdb metrics - #624
Conversation
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
WalkthroughImplements a metrics ticker that collects ledger/accounts storage and counts; adds column-level compaction retention control; relaxes RocksDB property accessor types and adds column-count caching; exposes new metrics helpers and gauges. Changes
Sequence Diagram(s)sequenceDiagram
participant Ticker as Metrics Ticker Loop
participant LedgerCol as LedgerColumn / Cache
participant RocksDB as RocksDB
participant Metrics as Metrics Registry
loop Each tick (cancellable)
Ticker->>RocksDB: query ledger storage size
RocksDB-->>Metrics: set ledger storage gauge
Ticker->>RocksDB: query accounts storage size
RocksDB-->>Metrics: set accounts size gauge
Ticker->>LedgerCol: observe_columns_count_duration {measure}
LedgerCol->>LedgerCol: ensure cache (init_column_count_cache)
alt sequential CF
LedgerCol->>RocksDB: compute count via key arithmetic
else complex CF
LedgerCol->>RocksDB: get_int_property (CStrLike)
end
LedgerCol-->>Metrics: record duration & column count
Ticker->>RocksDB: query accounts count
RocksDB-->>Metrics: set accounts count gauge
end
sequenceDiagram
participant Compaction as Compaction Process
participant Filter as Compaction Filter
participant Column as Column Trait
Compaction->>Filter: evaluate key (slot_in_key, oldest_slot)
Filter->>Column: keep_all_on_compaction()
alt returns true
Filter->>Compaction: Keep (early exit)
else returns false
alt slot_in_key < oldest_slot
Filter->>Compaction: Remove
else
Filter->>Compaction: Keep
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-21T14:00:54.642ZApplied to files:
🧬 Code graph analysis (1)magicblock-metrics/src/metrics/mod.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
magicblock-api/src/tickers.rs(1 hunks)magicblock-ledger/src/database/columns.rs(2 hunks)magicblock-ledger/src/database/compaction_filter.rs(1 hunks)magicblock-ledger/src/database/ledger_column.rs(4 hunks)magicblock-ledger/src/database/rocks_db.rs(2 hunks)magicblock-metrics/src/metrics/mod.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-07T13:20:13.793Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.
Applied to files:
magicblock-ledger/src/database/columns.rs
🧬 Code graph analysis (5)
magicblock-metrics/src/metrics/mod.rs (1)
magicblock-api/src/tickers.rs (1)
set_accounts_count(152-159)
magicblock-ledger/src/database/rocks_db.rs (1)
magicblock-ledger/src/database/compaction_filter.rs (2)
name(68-70)name(103-105)
magicblock-api/src/tickers.rs (2)
magicblock-api/src/magic_validator.rs (1)
ledger(735-737)magicblock-metrics/src/metrics/mod.rs (4)
set_ledger_size(286-288)set_accounts_size(341-343)set_accounts_count(345-347)observe_columns_count_duration(334-339)
magicblock-ledger/src/database/compaction_filter.rs (1)
magicblock-ledger/src/database/columns.rs (10)
key(125-125)key(170-174)key(234-243)key(318-323)key(393-398)key(495-497)key(566-571)key(641-643)keep_all_on_compaction(132-134)keep_all_on_compaction(659-661)
magicblock-ledger/src/database/ledger_column.rs (1)
magicblock-ledger/src/database/columns.rs (9)
columns(105-117)slot(131-131)slot(181-183)slot(249-251)slot(329-331)slot(404-406)slot(503-505)slot(577-579)slot(649-651)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run_make_ci_lint
🔇 Additional comments (16)
magicblock-ledger/src/database/columns.rs (2)
132-134: LGTM: Clean trait method addition.The default implementation returning
falseis appropriate for most columns, allowing slot-based cleanup during compaction.
658-661: LGTM: Correct override for non-slot-based column.The implementation correctly preserves
AccountModDatasduring compaction since it's not keyed by slot. The inline comment clearly explains the rationale.magicblock-ledger/src/database/compaction_filter.rs (2)
84-84: LGTM: Correct unused parameter convention.Prefixing with underscore appropriately indicates the parameter is unused.
89-91: LGTM: Efficient early-exit for non-slot-based columns.The short-circuit correctly bypasses slot-based removal when
keep_all_on_compaction()is true, preserving columns likeAccountModDatasthat aren't slot-keyed.magicblock-ledger/src/database/rocks_db.rs (2)
11-11: LGTM: Import added for API flexibility.Adding
CStrLikesupports the more flexible parameter type inget_int_property_cf.
237-241: LGTM: More flexible property name parameter.Changing from
&'static std::ffi::CStrtoimpl CStrLikeimproves ergonomics while maintaining backward compatibility.magicblock-metrics/src/metrics/mod.rs (1)
334-347: LGTM: Clean metric helper functions.The helper functions are well-structured and provide clean interfaces for observing duration and updating gauge values.
magicblock-api/src/tickers.rs (3)
112-140: LGTM: Elegant macro-based metric collection.The macro pattern effectively reduces duplication across multiple ledger count metrics while maintaining clear error handling.
142-159: LGTM: Appropriate error handling and type conversion.The functions correctly handle storage size and count queries with appropriate logging and safe type conversion using
unwrap_or(i64::MAX)for overflow protection.
164-177: LGTM: Well-structured ticker loop.The loop correctly:
- Respects the cancellation token
- Collects all metrics in sequence
- Wraps column count collection with duration observation
- Uses async sleep for non-blocking operation
magicblock-ledger/src/database/ledger_column.rs (6)
10-12: LGTM: Import updates for API flexibility.The import changes support the more flexible
CStrLikeparameter type.
229-234: LGTM: Consistent API flexibility improvement.The signature change aligns with
rocks_db.rsand improves API ergonomics.
293-309: LGTM: Thread-safe cache initialization.The compare-exchange pattern correctly handles concurrent initialization attempts, ensuring the cache is set exactly once without blocking.
311-319: LGTM: Clean lazy initialization pattern.The refactored function efficiently returns cached values and initializes on first access.
565-584: Verify slot contiguity assumption.The calculation
last_slot - start_slot + 1assumes slots are contiguous without gaps. Ensure this holds true for all sequential columns, or document the assumption.If slots can have gaps (e.g., due to deletions or compaction), this count may be inaccurate.
586-595: LGTM: Appropriate use of RocksDB estimate.Using
rocksdb.estimate-num-keysis reasonable for complex columns. The property name is correctly spelled and the comment appropriately notes it's approximate.
| #[inline(always)] | ||
| fn is_sequential_cf<T: ColumnName>() -> bool { | ||
| matches!( | ||
| T::NAME, | ||
| crate::database::columns::Blocktime::NAME | ||
| | crate::database::columns::Blockhash::NAME | ||
| | crate::database::columns::PerfSamples::NAME | ||
| | crate::database::columns::AccountModDatas::NAME | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify hardcoded column classification remains accurate.
The function hardcodes specific column names to determine sequential behavior. Ensure this classification is maintained if columns are added, renamed, or removed in the future.
Consider documenting this classification in the Column trait or column definitions to ensure maintainability.
🤖 Prompt for AI Agents
In magicblock-ledger/src/database/ledger_column.rs around lines 280 to 289, the
is_sequential_cf function uses a hardcoded match of specific column NAMEs which
will break if columns are added/renamed/removed; change this by moving the
sequential classification into the ColumnName trait (e.g., add an associated
const or method like IS_SEQUENTIAL) and use that property here instead of
matching on names, update all column definitions to set the new flag, and add a
comment and a unit test to verify the classification remains correct when
columns change.
bmuddha
left a comment
bmuddha
left a comment
There was a problem hiding this comment.
LGTM, but please address the metric registration comment
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Performance** * Compaction now preserves account-modification data during cleanup to avoid losing important account-related entries. * Faster metadata access via column-count caching, reducing overhead during storage operations. * **Monitoring** * Continuous metrics for ledger storage size, accounts storage size, and account counts. * New duration histogram for column-count measurements and renamed/updated execution-time histogram for improved observability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary by CodeRabbit
Performance
Monitoring