Skip to content

fix(clients): push bulk client changes to nodes only after the commit lands - #6181

Open
n0ctal wants to merge 1 commit into
MHSanaei:mainfrom
n0ctal:upstream-node-push-after-commit
Open

fix(clients): push bulk client changes to nodes only after the commit lands#6181
n0ctal wants to merge 1 commit into
MHSanaei:mainfrom
n0ctal:upstream-node-push-after-commit

Conversation

@n0ctal

@n0ctal n0ctal commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Move the node-side client push in bulkAdjustInboundClients and bulkDelInboundClients so it runs after the database transaction commits, and let the existing dirty-node reconcile converge instead of pushing eagerly.

Why

Both bulk paths pushed the mutated client set to the remote node before runSerializedTx wrote the inbound. If that transaction then failed — a serialization retry giving up, a constraint violation, the traffic poll holding the lock — the node had already applied changes the panel never recorded.

From that point the two disagree, and they disagree silently: the node serves expiry and quota values that exist nowhere in the database, while the panel believes the operation did not happen. Nothing marks the divergence, because each half individually reported success. It surfaces later as a client whose limits do not match the panel, with no event to trace it back to.

This is an ordering bug rather than a race — the push is unconditionally sequenced before the write, so no amount of retrying changes the outcome.

Scope

  • bulkAdjustInboundClients: drop the pre-commit push block and push after a successful commit, keeping the existing flowChanged and nodeBulkPushThreshold conditions unchanged.
  • bulkDelInboundClients: same reordering. The deletion path now runs through runSerializedTx with MarkNodeDirtyTx, so a push that fails converges through reconcile instead of leaving the node ahead of the panel.
  • Remove the prevSettings / advancePushedInbound bookkeeping that existed only to compensate for the early push.

Validation

  • go build ./internal/... clean.
  • go test ./internal/web/service/ green on main at ece16559 with this applied.
  • Cherry-picks onto current main without conflict.

Risk

Low. A failed push now leaves the node marked dirty and is picked up by the existing reconcile pass rather than retried inline — the same mechanism that already covers batches above nodeBulkPushThreshold and flow changes. No schema, API, or configuration change.

@n0ctal
n0ctal force-pushed the upstream-node-push-after-commit branch from ab5b2ab to 0503192 Compare August 8, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant