Fix issue with drizzle state persistence #164
Conversation
📝 WalkthroughWalkthroughThis pull request introduces new JSON configuration files for several packages specifying prerelease versions with patch changes. Modifications have been made to enhance state persistence and message generation by introducing optional properties and refining the handling of the Changes
Sequence Diagram(s)sequenceDiagram
participant Indexer
participant MessageGenerator
participant Tester
Tester->>MessageGenerator: Call generateMockMessages with options
MessageGenerator->>MessageGenerator: Calculate currentBlockNumber using baseBlockNumber
MessageGenerator->>MessageGenerator: Compute uniqueKey via uniqueKeyFromOrderKey if enabled
MessageGenerator-->>Tester: Return messages with updated orderKey and uniqueKey
sequenceDiagram
participant Plugin
participant PersistState
participant Database
Plugin->>PersistState: Invoke persistState with endCursor
alt endCursor.uniqueKey is truthy
PersistState->>Database: Store uniqueKey value
else
PersistState->>Database: Store uniqueKey as null (indicating deletion)
end
Database-->>PersistState: Acknowledge update
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (3)
packages/indexer/src/internal/testing.ts (3)
54-55: Fix minor typo in variable naming.Consider correcting “fianlizedToBlock” to “finalizedToBlock” to maintain clarity:
- const fianlizedToBlock = + const finalizedToBlock =
72-73: Duplicate field definitions.These lines appear to mirror lines 22-23. If unintentional duplication, consider refactoring or confirming they’re required in two locations. Otherwise, it’s consistent with the prior addition.
87-89: Clarify prefix for generated unique key.Appending
0xff00is effective, but ensure there’s no confusion about its purpose or potential collisions. Could revise or document the prefix if used more widely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
change/@apibara-indexer-795d3289-02e0-4d36-bfca-345341984941.json(1 hunks)change/@apibara-plugin-drizzle-13ed35ed-0dd1-478f-9722-6501d99b9f31.json(1 hunks)change/@apibara-plugin-mongo-85e9b5c6-310b-478d-b775-9335d4b7a04d.json(1 hunks)packages/indexer/src/internal/testing.ts(3 hunks)packages/plugin-drizzle/src/persistence.ts(1 hunks)packages/plugin-drizzle/tests/persistence.test.ts(1 hunks)packages/plugin-drizzle/tests/storage.test.ts(1 hunks)packages/plugin-mongo/src/persistence.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/indexer/src/internal/testing.ts (2)
packages/protocol/src/stream.ts (4) (4)
Invalidate(88-93)Invalidate(95-95)Finalize(97-102)Finalize(104-104)packages/protocol/src/proto/stream.ts (4) (4)
Invalidate(188-199)Invalidate(753-823)Finalize(202-209)Finalize(829-882)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (17)
change/@apibara-indexer-795d3289-02e0-4d36-bfca-345341984941.json (1)
1-7: Configuration change for indexer packageThis new JSON configuration file correctly identifies the patch release needed for fixing state persistence issues when switching back to backfill. The change is related to the problem described in the PR objectives.
change/@apibara-plugin-mongo-85e9b5c6-310b-478d-b775-9335d4b7a04d.json (1)
1-7: Configuration change for MongoDB pluginThis configuration file correctly describes the purpose of the change: explicitly setting
uniqueKeyto null when it's undefined. This aligns with the implementation change in the persistence.ts file and addresses the issue described in the PR objectives.change/@apibara-plugin-drizzle-13ed35ed-0dd1-478f-9722-6501d99b9f31.json (1)
1-7: Configuration change for Drizzle pluginThis configuration file properly documents the prerelease patch for fixing state persistence when switching back to backfill in the Drizzle plugin. This is consistent with the overall fix described in the PR objectives.
packages/plugin-mongo/src/persistence.ts (1)
47-47: Fix for undefined uniqueKey handlingThis change properly implements the solution described in the PR objectives. By explicitly setting
uniqueKeyto null whenendCursor.uniqueKeyis falsy, it prevents the persistence layer from ignoring undefined values. This addresses the issue that occurs when an indexer processes a live block with a unique key, disconnects, and then reconnects to a finalized block without a unique key.packages/plugin-drizzle/src/persistence.ts (1)
177-179: Ensure proper handling ofuniqueKeyupdates.Setting
uniqueKeytonullwhen the incoming value is falsy correctly signals that it should be cleared. This is consistent with your inline comment and solves the issue where Drizzle otherwise won't update the column. It's advisable to confirm that all queries or downstream logic properly differentiate between anullkey and a truly absent key.packages/indexer/src/internal/testing.ts (4)
22-23: New optional fields look good.Introducing
uniqueKey?: booleanandbaseBlockNumber?: bigintis a straightforward approach to customize block numbering and key generation in tests. No immediate concerns.
34-35: Good use of fallback forbaseBlockNumber.Using
options?.baseBlockNumber ?? BigInt(5_000_000)provides a sensible default, enhancing flexibility for test scenarios.
37-38: Efficient unique key derivation.Generating a unique key from
currentBlockNumbercentralizes logic and keeps test code clear. No further issues noted.
40-41: No issues withinvalidateToBlock.This calculation is consistent and clearly offsets from
baseBlockNumber. The approach is straightforward.packages/plugin-drizzle/tests/storage.test.ts (1)
17-17: Renaming the describe block.Renaming the top-level describe to "Drizzle storage" is clear, matching the file’s purpose. No concerns.
packages/plugin-drizzle/tests/persistence.test.ts (7)
19-69: Test “should persist state” gracefully handles errors.
- The test ensures the DB records remain consistent up to the failure point (line 37).
- Throwing an error to confirm partial persistence is a solid approach to verifying rollback vs. commit logic.
Looks robust for validating partial updates and error handling.
71-122: Test “should override the persisted uniqueKey.”
- Verifies that new runs can clear or update the
uniqueKeyfield in the checkpoint table.- Adequately checks end state with two separate mock clients.
Well-structured coverage for unique key override behavior.
124-308: Test “should persist the filters and latest block number (factory mode).”
- Exercises dynamic filter creation via a factory function.
- Tracks transitions from block 100 to 108 and ensures filters table updates accordingly.
Comprehensive test for multi-filter state management.
310-484: Test “should invalidate state (factory mode).”
- Checks partial invalidation using
_tag: "invalidate", reverting to the correct block.- Ensures filters table is updated to reflect the truncated range.
Covers reorg/invalidate logic thoroughly.
486-660: Test “should finalize state (factory mode).”
- Validates finality at block 107, ensuring upstream blocks remain unaffected.
- Confirmed updates to
filtersandcheckpointsalign with finality logic.Solid coverage of finalize scenarios.
662-701: Test “persistence schema version check.”
- Ensures
schemaVersionis set upon initialization.- Straightforward confirmation of expected version (0).
No issues; demonstrates appropriate schema setup.
702-845: Test “should not persist state for pending blocks.”
- Correctly only commits data for accepted blocks; pending blocks remain ephemeral.
- The final checkpoint is the last accepted block, highlighting partial acceptance logic.
This is a key scenario ensuring correctness of final vs. pending states.
fracek
force-pushed
the
fix-drizzle-persistence
branch
from
02dca40 to
d31120c
Compare
2 participants
The DNA server does not include the
endCursor.uniqueKeyfor backfilled blocks since they cannot be part of a chain reorganization.This means the value is
undefinedfor these blocks.In some circumstances this caused an issue with indexers that persist state to Drizzle.
starting cursor Y/X not found. canonical: YYYY, reorged: noneThis PR makes sure we overwrite the unique key to null if it's undefined.