fix(sqlite): flush data items using bundle db writer PE-6872 #219
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #219 +/- ##
===========================================
+ Coverage 68.72% 68.81% +0.09%
===========================================
Files 32 32
Lines 7919 7942 +23
Branches 431 434 +3
===========================================
+ Hits 5442 5465 +23
Misses 2476 2476
Partials 1 1 ☔ View full report in Codecov by Sentry. |
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes updates to the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🔇 Additional comments (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/database/standalone-sqlite.ts (1)
949-951: Specify the return type forsaveBlockAndTxsThe
saveBlockAndTxsmethod currently lacks an explicit return type, which can lead to less clarity and potential type safety issues. Consider specifying the return type for better code maintainability.Apply this diff to specify the return type:
-saveBlockAndTxs( +saveBlockAndTxs( block: PartialJsonBlock, txs: PartialJsonTransaction[], missingTxIds: string[], +): { endHeight: number; maxStableBlockTimestamp: number } {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- package.json (1 hunks)
- src/database/standalone-sqlite.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (3)
package.json (1)
11-11: Verify compatibility with updated @ar.io/sdk versionThe @ar.io/sdk dependency has been updated from ^2.0.0 to ^2.2.5. This minor version update may introduce new features or optimizations that could be relevant to the database operation changes mentioned in the PR objectives.
While keeping dependencies up-to-date is good practice, please ensure:
- The new SDK version has been thoroughly tested with your codebase.
- Any new features or changes in the SDK from 2.0.0 to 2.2.5 have been reviewed for potential impacts on your implementation.
- The changelog or release notes for @ar.io/sdk versions 2.1.0 through 2.2.5 have been checked for any relevant updates or breaking changes.
To help verify the impact of this change, you can run the following script:
src/database/standalone-sqlite.ts (2)
2381-2381: Verify all usages ofWorkerMethodNameafter type changeThe
WorkerMethodNametype has been updated tokeyof StandaloneSqliteDatabaseWorker. Ensure that all references and implementations usingWorkerMethodNameare compatible with this change to prevent type errors.
3085-3096: Confirm the synchronous execution ofsaveBlockAndTxsin workerIn the worker message handler,
worker.saveBlockAndTxsis called synchronously, and the result is immediately posted back. Verify that this method is intended to be synchronous and does not perform long-running operations that could block the worker thread.
Prior to this commit, data items were flushed via the core DB writer. Under sufficiently heavy bunde import load this lead to contention with data item writes. This change adjusts the flushing logic to use the bundle DB writer. This avoids concurrent bundle DB write attempts and eliminates the contention.
djwhitt
force-pushed
the
PE-6872-data-item-flush-contention-fix
branch
from
46aa5ac
to
3d36c69
Compare
Prior to this commit, data items were flushed via the core DB writer. Under sufficiently heavy bunde import load this lead to contention with data item writes. This change adjusts the flushing logic to use the bundle DB writer. This avoids concurrent bundle DB write attempts and eliminates the contention.