Optionally disable multichain rewind lock#3028
Open
Wizdave97 wants to merge 4 commits intosubquery:mainfrom
Open
Optionally disable multichain rewind lock#3028Wizdave97 wants to merge 4 commits intosubquery:mainfrom
Wizdave97 wants to merge 4 commits intosubquery:mainfrom
Conversation
📝 WalkthroughWalkthroughAdded a new boolean config flag Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant NodeConfig
participant FetchService
participant MultiChainRewindService
participant Store
CLI->>NodeConfig: parse --disable-multichain-rewind-lock
NodeConfig-->>FetchService: config.disableMultichainRewindLock
NodeConfig-->>MultiChainRewindService: config.disableMultichainRewindLock
alt disableMultichainRewindLock == false
FetchService->>MultiChainRewindService: query rewind status
MultiChainRewindService->>Store: acquireGlobalRewindLock()
Store-->>MultiChainRewindService: lock result
MultiChainRewindService-->>FetchService: may block until complete
else disableMultichainRewindLock == true
FetchService->>MultiChainRewindService: skip rewind-wait gate
MultiChainRewindService-->>FetchService: immediate allow (acquire returns true)
end
FetchService->>FetchService: continue fillNextBlockBuffer / processing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/node-core/src/indexer/fetch.service.spec.ts`:
- Around line 830-838: The test is flaky because fetchService.init(10) spins off
an async loop and the negative assertion may run before that loop checks the
rewind lock; make it deterministic by ensuring the loop iteration that would
perform the rewind-check runs (or is advanced) before asserting. Specifically,
in this test modify the flow around fetchService.init, nodeConfig.merge, and
(multichainRewindService as any).status: either use Jest fake timers and call
jest.useFakeTimers() then advance timers (e.g., jest.runOnlyPendingTimers() /
jest.advanceTimersByTime()) to force the loop iteration, or await a known
promise/observable the loop awaits (or spy on the specific method that performs
the rewind-check and wait for it to be called/settled) before asserting that
logger.info was not called with the rewind message; keep the negative assertion
only after the loop has been deterministically progressed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e62ef57f-37e9-450b-b7f5-5e33fcee78ee
📒 Files selected for processing (6)
packages/node-core/src/configure/NodeConfig.spec.tspackages/node-core/src/configure/NodeConfig.tspackages/node-core/src/indexer/fetch.service.spec.tspackages/node-core/src/indexer/fetch.service.tspackages/node-core/src/indexer/multiChainRewind.service.tspackages/node-core/src/yargs.ts
Author
|
@ianhe8x Please take a look |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/node-core/src/indexer/multiChainRewind.service.ts (1)
216-218: Consider adding a debug/info log for observability.When the lock is disabled, the early return bypasses the "Rewind success" log at line 223. For consistent debugging and operational visibility, consider adding a log statement here.
♻️ Optional: Add logging for consistency
if (this.disableRewindLock) { + logger.info(`Rewind lock disabled, skipping chain lock release for chainId: ${this.chainId}`); return 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node-core/src/indexer/multiChainRewind.service.ts` around lines 216 - 218, Add an informational/debug log right before the early return when this.disableRewindLock is true so that skipping the rewind is visible in logs; use the same logger used for the "Rewind success" message (e.g., this.logger.info or whatever logger instance the service uses) and emit a message like "Rewind lock disabled, skipping rewind" including any available context (chain id or job id) before returning 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/node-core/src/indexer/multiChainRewind.service.ts`:
- Around line 216-218: Add an informational/debug log right before the early
return when this.disableRewindLock is true so that skipping the rewind is
visible in logs; use the same logger used for the "Rewind success" message
(e.g., this.logger.info or whatever logger instance the service uses) and emit a
message like "Rewind lock disabled, skipping rewind" including any available
context (chain id or job id) before returning 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bc477e5-8035-45d8-9c5d-1bad8dde4d06
📒 Files selected for processing (1)
packages/node-core/src/indexer/multiChainRewind.service.ts
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist
Summary by CodeRabbit
New Features
Behavior Changes
Tests