Fix shutdown crash when flushing actions for unregistered players - #390
Open
luozaixuan wants to merge 1 commit into
Open
Fix shutdown crash when flushing actions for unregistered players#390luozaixuan wants to merge 1 commit into
luozaixuan wants to merge 1 commit into
Conversation
Ledger (1.3.18, 1.20.1 Fabric) crashed 100% of the time on server stop when the action queue had a backlog (tens of thousands to hundreds of thousands of entries): flushing inserted into the players table and hit SQLITE_CONSTRAINT_NOTNULL (players.player_name is NOT NULL with no default), the error was mistaken for a transient failure and retried 3 times with backoff, stalling shutdown for ~5 minutes before timing out and losing the whole backlog. Root cause: insertActions called getOrCreatePlayerId(it.id); that helper reused getOrCreateObjectId, which is designed for single-key lookup tables (Sources/Worlds/ObjectResourceLocations) and, on a miss, inserts only the unique key column. The Players table has a second mandatory column, player_name, so the generated INSERT omitted it and violated the constraint. Player rows are normally created asynchronously by onJoin's logPlayer, so any action produced before that write landed (or by entities/Create fake players with no registered join) hit a missing player row when shutdown force-drained the queue. Fix: getOrCreatePlayerId now takes the full GameProfile and, on a cache/DB miss, inserts a complete row including player_name (falling back to an unknown_ + uuid-prefix name within the 16-char limit when the profile has no name), keeping both playerKeys and playernameKeys caches in sync. No schema or migration changes are needed: the NOT NULL constraint guarantees no dirty rows exist in existing databases.
Author
|
this fix only for 1.20.1-backport,because Ledger with Create will have this error |
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
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.
Ledger (1.3.18, 1.20.1 Fabric) crashed 100% of the time on server stop when the action queue had a backlog (tens of thousands to hundreds of thousands of entries): flushing inserted into the players table and hit SQLITE_CONSTRAINT_NOTNULL (players.player_name is NOT NULL with no default), the error was mistaken for a transient failure and retried 3 times with backoff, stalling shutdown for ~5 minutes before timing out and losing the whole backlog.
Root cause: insertActions called getOrCreatePlayerId(it.id); that helper reused getOrCreateObjectId, which is designed for single-key lookup tables (Sources/Worlds/ObjectResourceLocations) and, on a miss, inserts only the unique key column. The Players table has a second mandatory column, player_name, so the generated INSERT omitted it and violated the constraint. Player rows are normally created asynchronously by onJoin's logPlayer, so any action produced before that write landed (or by entities/Create fake players with no registered join) hit a missing player row when shutdown force-drained the queue.
Fix: getOrCreatePlayerId now takes the full GameProfile and, on a cache/DB miss, inserts a complete row including player_name (falling back to an unknown_ + uuid-prefix name within the 16-char limit when the profile has no name), keeping both playerKeys and playernameKeys caches in sync.
No schema or migration changes are needed: the NOT NULL constraint guarantees no dirty rows exist in existing databases.