Repository navigation
fix(points): reconcile shared MySQL point placeholder publication #1739
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
107 changes: 107 additions & 0 deletions
107
VotingPlugin/src/main/java/com/bencodez/votingplugin/placeholders/PointPlaceholderFence.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| package com.bencodez.votingplugin.placeholders; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.UUID; | ||
|
|
||
| /** Coordinates point rendering with in-flight shared-storage mutations. */ | ||
| final class PointPlaceholderFence { | ||
| private static final class State { | ||
| long generation; | ||
| int pending; | ||
| int readers; | ||
| int refills; | ||
| boolean uncertain; | ||
| boolean snapshotUnavailable; | ||
| } | ||
|
|
||
| private final Map<UUID, State> states = new HashMap<>(); | ||
| private long generation; | ||
| private long lifecycle; | ||
| record Stamp(long lifecycle, long generation) { } | ||
|
|
||
| synchronized Stamp generation(UUID uuid) { | ||
| State state = states.computeIfAbsent(uuid, ignored -> { | ||
| State created = new State(); | ||
| created.generation = ++generation; | ||
| return created; | ||
| }); | ||
| state.readers++; | ||
| return new Stamp(lifecycle, state.generation); | ||
| } | ||
|
|
||
| synchronized boolean readable(UUID uuid, Stamp observed) { | ||
| if (lifecycle != observed.lifecycle()) return false; | ||
| State state = states.get(uuid); | ||
| return state != null && state.generation == observed.generation() | ||
| && state.pending == 0 && state.refills == 0 && !state.uncertain && !state.snapshotUnavailable; | ||
| } | ||
|
|
||
| synchronized void release(UUID uuid) { | ||
| State state = states.get(uuid); | ||
| if (state == null || state.readers == 0) throw new IllegalStateException("Unbalanced point reader"); | ||
| state.readers--; | ||
| retireIfSafe(uuid, state); | ||
| } | ||
|
|
||
| private void retireIfSafe(UUID uuid, State state) { | ||
| if (state.readers == 0 && state.pending == 0 && state.refills == 0 && !state.uncertain && !state.snapshotUnavailable) states.remove(uuid); | ||
| } | ||
|
|
||
| synchronized void invalidateLifecycle() { lifecycle++; } | ||
| synchronized long lifecycle() { return lifecycle; } | ||
| synchronized boolean recoverable(UUID uuid) { | ||
| State state = states.get(uuid); | ||
| return state != null && state.snapshotUnavailable && !state.uncertain && state.pending == 0 && state.refills == 0; | ||
| } | ||
|
|
||
| synchronized Stamp beginRefill(UUID uuid, Runnable invalidate) { | ||
| State state = states.computeIfAbsent(uuid, ignored -> new State()); | ||
| state.generation = ++generation; | ||
| state.snapshotUnavailable = true; | ||
| state.refills++; | ||
| state.readers++; | ||
| invalidate.run(); | ||
| return new Stamp(lifecycle, state.generation); | ||
| } | ||
|
|
||
| synchronized void begin(UUID uuid, Runnable invalidate) { | ||
| State state = states.computeIfAbsent(uuid, ignored -> new State()); | ||
| state.generation = ++generation; | ||
| state.pending++; | ||
| invalidate.run(); | ||
| } | ||
|
|
||
| synchronized void finish(UUID uuid, boolean knownOutcome, boolean trustworthy) { | ||
| State state = states.get(uuid); | ||
| if (state == null || state.pending == 0) throw new IllegalStateException("Unbalanced point mutation"); | ||
| state.uncertain |= !knownOutcome; | ||
| state.snapshotUnavailable = !trustworthy || state.refills != 0; | ||
| state.pending--; | ||
| state.generation = ++generation; | ||
| retireIfSafe(uuid, state); | ||
| } | ||
|
|
||
| synchronized void finishAdmission(UUID uuid, boolean workerSettled) { | ||
| State state = states.get(uuid); | ||
| if (state == null || state.pending == 0) throw new IllegalStateException("Unbalanced point admission"); | ||
| state.pending--; | ||
| state.snapshotUnavailable |= !workerSettled; | ||
| state.generation = ++generation; | ||
| retireIfSafe(uuid, state); | ||
| } | ||
|
|
||
| synchronized void finishRefill(UUID uuid, Stamp observed, boolean verified) { | ||
| State state = states.get(uuid); | ||
| if (state == null || state.refills == 0) throw new IllegalStateException("Unbalanced point refill"); | ||
| boolean current = lifecycle == observed.lifecycle() && state.generation == observed.generation(); | ||
| state.refills--; | ||
| state.snapshotUnavailable = !(verified && current && state.pending == 0 && state.refills == 0 && !state.uncertain); | ||
| // Even an obsolete read completion can replace storage data: retire every older render. | ||
| state.generation = ++generation; | ||
| } | ||
|
|
||
| synchronized void publish(UUID uuid, Stamp observed, Runnable publication) { | ||
| if (readable(uuid, observed)) publication.run(); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
Script executed:
cat VotingPlugin/src/main/java/com/bencodez/votingplugin/placeholders/PointPlaceholderFence.java rg -n 'uncertain' VotingPlugin/srcRepository: BenCodez/VotingPlugin
Length of output: 14930
Script executed:
Repository: BenCodez/VotingPlugin
Length of output: 41788
Script executed:
Repository: BenCodez/VotingPlugin
Length of output: 42063
Script executed:
Repository: BenCodez/VotingPlugin
Length of output: 43531
Script executed:
Repository: BenCodez/VotingPlugin
Length of output: 38016
Script executed:
Repository: BenCodez/VotingPlugin
Length of output: 22202
Add a terminal reconciliation path for
PointPlaceholderFence.uncertain.After
settleTransfercallsdrainCache(target), aRuntimeExceptionreaches thefinallyblock whileknownis stillfalse.finishPointPublicationtherefore callsfinishSharedPointDebit(..., false, ...)beforesettleWithConfirmationruns. This fences both affected users.finishpermanently preservesuncertain. A later successful mutation, verified refill, cache refresh, journal recovery, orPlaceHolders.reload()cannot clear it. The affected placeholders can therefore remain"..."for the lifetime of the currentPlaceHoldersinstance. A plugin instance replacement can create a new fence, so “until JVM restart” is too absolute.The reload fix is unsafe. Reload does not prove whether the original write committed. Clear
uncertainonly after journal recovery or another authoritative reconciliation establishes the terminal storage state. Then retainsnapshotUnavailableand trigger a verified refill before publishing the refreshed balance.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.
Keeping genuine transaction uncertainty fail closed. Current recovery results identify a player and column, but do not tie terminal evidence to the original uncertain operation; cache draining can also perform writes before journal settlement. A reload, unrelated successful mutation or snapshot read therefore cannot safely clear this latch. Safe reopening needs a separately scoped, operation-bound reconciliation path. The integration guide now explicitly documents the resulting availability limitation; no generic clear or automatic mutation retry is introduced.
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.
@BenCodez Understood. Keep genuine transaction uncertainty fail closed. A reload, unrelated mutation, or snapshot read does not establish the original operation’s outcome.
The availability limitation remains, but a generic clear or automatic mutation retry would be unsafe. Operation-bound reconciliation belongs in a separate follow-up. This is an accepted limitation, not a completed fix for this finding.
Would you like me to open a follow-up issue for that reconciliation path?
Learnings added
Learnings used
You are interacting with an AI system.