-
Notifications
You must be signed in to change notification settings - Fork 86
Feature/improve publish success rate #4075
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… Now we are waiting for the nodes that did an ack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
src/modules/triple-store/implementation/ot-blazegraph/ot-blazegraph.js
Outdated
Show resolved
Hide resolved
| null, | ||
| this.completedStatuses, | ||
| ); | ||
| await this.repositoryModuleManager.updateMinAcksReached(operationId, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Race condition allows duplicate operation completion
The check record?.minAcksReached followed by markOperationAsCompleted and updateMinAcksReached has a race condition. Multiple concurrent responses can each read minAcksReached as false, then all proceed to call markOperationAsCompleted before any of them updates minAcksReached to true. This can cause the same operation to be marked as completed multiple times. The check and update are not atomic and occur outside the operationMutex used by getResponsesStatuses.
Description
processing once minAcksReached is set, only failing when all contacted nodes have responded unsuccessfully
(src/service/publish-service.js).
cache-miss scenarios to avoid noisy errors while still emitting state changes (src/commands/protocols/publish/
publish-finalization-command.js).
to aid debugging (src/modules/triple-store/implementation/ot-blazegraph/ot-blazegraph.js).
Note
Publish now completes once minimum acknowledgements are met (and only fails after all responses if insufficient), with clearer cache logs and enhanced Blazegraph update error reporting.
src/service/publish-service.js):minAckResponsesis reached; skip further processing viaminAcksReachedguard.minAckResponses.src/commands/protocols/publish/publish-finalization-command.js):ualearlier for logging/context.datasetPathonce before retry loop.src/modules/triple-store/implementation/ot-blazegraph/ot-blazegraph.js):queryVoidin try/catch; log HTTP status and response data snippet on failure; rethrow error.Written by Cursor Bugbot for commit 17016fa. This will update automatically on new commits. Configure here.