fix(coordinator): fix the error in the incorrect call during database… - #1576
Conversation
Sorry, something went wrong.
WalkthroughThe pull request introduces a modification in the recoverActiveAttempts method of the BatchProverTask struct within the batch prover task logic. The change involves switching the ORM method used for decreasing active attempts from chunkOrm.DecreaseActiveAttemptsByHash to batchOrm.DecreaseActiveAttemptsByHash. This alteration suggests a realignment in how active attempts are tracked and managed for batch tasks, potentially reflecting an underlying data model or organizational change. Changes
Suggested Reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord. 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? ❤️ Share 🪧 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 (.coderabbit.yaml)
Documentation and Community
|
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)coordinator/internal/logic/provertask/batch_prover_task.go (1)📜 Review details235-237: Consider adding tests for attempt recovery
Since this fixes a bug in attempt management, it would be valuable to add tests verifying:
- Correct decrease of active attempts for batch tasks
- Error handling behavior
Would you like me to help create test cases for this functionality?
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Reviewing files that changed from the base of the PR and between 33b1b3c and 4ae4645.
📒 Files selected for processing (1)- coordinator/internal/logic/provertask/batch_prover_task.go (1 hunks)
235-237: LGTM! Correct ORM usage for batch operations
The change from chunkOrm to batchOrm is correct as we're operating on batch-level attempts. This fixes a logical error where chunk-level operations were being used for batch-level state management.
235-237: Verify consistent attempt management across codebase
Let's verify there are no other inconsistent usages of attempt management between chunk and batch operations.
✅ Verification successfulBased on the search results, I can see a consistent pattern in how attempt management is handled across the codebase:
- All three types (chunk, batch, bundle) implement DecreaseActiveAttemptsByHash with similar behavior
- The attempt management is used consistently in error paths:
- In proof submission (submitproof/proof_receiver.go)
- In proof collection (controller/cron/collect_proof.go)
- In prover task handling (provertask/*.go)
- The error handling and logging patterns are consistent across all implementations
The code under review follows the same pattern as other similar operations in the codebase. The attempt management is properly synchronized with the task lifecycle and error handling paths.
Attempt management implementation is consistent
The implementation follows the established patterns in the codebase and properly handles attempt management for batch operations, consistent with how it's done for chunks and bundles.
🏁 Scripts executedThe following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for other potential mismatches in attempt management
# Look for attempt-related operations on chunks and batches
rg -A 3 "DecreaseActiveAttempts|UpdateAttempts|GetAssigned.*Attempts" --type go
# Look for potential error recovery paths that might need similar fixes
ast-grep --pattern 'if err := $$$; err != nil {
$$$
return $$$, err
}'
Length of output: 34577
Sorry, something went wrong.
scroll-tech#1576) Co-authored-by: xiaoranlu <xiaoranlu@tencent.com>
Purpose or design rationale of this PR
BatchProverTask.recoverActiveAttempts() should call batchOrm
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has tag in common/version.go been updated or have you added bump-version label to this PR?
Breaking change label
Does this PR have the breaking-change label?
Summary by CodeRabbit
Bug Fixes
Refactor