fix: database cannot resize on jmt write#7244
fix: database cannot resize on jmt write#7244SWvheerden merged 1 commit intotari-project:developmentfrom
Conversation
fix error handling to allow the jmt to resize the database
WalkthroughThe change updates the internal error handling logic in the Changes
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs (1)
2045-2067: LGTM! The fix correctly handles JMT-originated resize requests.This change effectively addresses the PR objective by ensuring that database resize requests originating from Jellyfish Merkle Tree operations are properly handled. The downcast pattern and error handling logic are sound.
However, consider refactoring the duplicated resize logic into a helper function to improve maintainability:
+ fn perform_database_resize( + &self, + size_that_could_not_be_written: usize, + resize_count: usize, + ) -> Result<(), ChainStorageError> { + info!( + target: LOG_TARGET, + "Database resize required (resized {} time(s) in this transaction)", + resize_count + 1 + ); + unsafe { + LMDBStore::resize(&self.env, &self.env_config, size_that_could_not_be_written) + } + } // Then replace both resize blocks with: - info!( - target: LOG_TARGET, - "Database resize required (resized {} time(s) in this transaction)", - i + 1 - ); - unsafe { - LMDBStore::resize(&self.env, &self.env_config, size_that_could_not_be_written)?; - } + self.perform_database_resize(size_that_could_not_be_written, i)?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: Cucumber tests / FFI
- GitHub Check: ci
- GitHub Check: cargo check with stable
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: test (mainnet, stagenet)
Test Results (CI) 3 files 126 suites 41m 20s ⏱️ For more details on these failures, see this check. Results for commit cf92310. |
Test Results (Integration tests) 2 files + 2 1 errors 1 suites +1 15m 18s ⏱️ + 15m 18s For more details on these parsing errors and failures, see this check. Results for commit cf92310. ± Comparison against base commit 255bb1b. |
* development: fix: remove long timeout in interactive_tx till tx is persisted into db (tari-project#7252) chore: new release v4.6.1-pre.0 (tari-project#7248) fix: minotari_merge_mining_proxy returns Tari block hash even if submit_to_origin is disabled (tari-project#7242) chore(ci): windows binary audit fix and more code-signing verification (tari-project#7245) fix: database cannot resize on jmt write (tari-project#7244)
Description
fix error handling to allow the jmt to resize the database
Summary by CodeRabbit