You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
perf: replace table files and drop finished intents without a folder flush - #50
Two new DurableFiles methods drop the folder fsync where it buys nothing. They're used only for the table file save and for deleting a finished wager intent.
writeReplacing: the new file's data is still fsynced before the atomic rename. The folder fsync is skipped only when the rename replaces a file that already existed; a new file still gets it.
deleteUnflushed: deletes without a folder fsync.
Why
One blackjack chip on an auto-tray table did about 8 disk syncs on the main thread. coverAutoTray took 6.2 s and tryPlaceChip's table save 2.2 s in Main's busiest hour on 11-02.
Worst case
Event
What can happen
Server crash
Nothing lost.
OS crash or power loss, before ext4's next commit (≤5 s, or the next fsync on the filesystem)
The table file reverts to its previous complete version, or a finished intent reappears and the table freezes for staff review, which asks Denars for the posting status.
A file is never torn: the data is synced before the rename, and the temp file sits in the same folder.
Unchanged and still fully flushed: new intents (synced before money is posted), the review/conflict intent rewrites, the intent delete after a refused posting, estate publications and journals, migrations and the running marker.
DenarEconomy's companion change keeps every Games posting fully synced. Only coin pickups are relaxed there, so a wager's posting is still on disk before its intent goes.
Checked
Tests: they inject a sync counter to assert which folder flushes happen, and that content is correct.
Local run: with Windows' folder-fsync error suppressed for the run, 1,299 tests: the same 333 Windows-only failures as main with the same patch. Linux CI is the reference.
Review: an independent review approved it on its own; the combined money-flow concern is covered by the DenarEconomy design.
Dev (TFMCDev01, 11-03)
Running as games-DIRSYNC-c301d29-DEV.jar. Games enabled and its tables loaded, with no errors in the log.
Not exercised on Dev: the chip/wager path. It needs a guild-funded auto-tray blackjack table and a seated bot, which I couldn't set up quickly. The changed save path runs on any table change.
Draft: not merged, not released, nothing on Main or Tutorial.
…ry flush
Every chip on an auto-tray blackjack table did ~8 fsyncs on the main thread
(intent write 2, ledger 1, table file 2, intent delete 1, plus chip
placement's table save 2): ~8 s in Main's busiest hour.
Two hot paths now skip only the directory flush:
- TableOwnershipStore.save replaces an existing table file with
DurableFiles.writeReplacing: the new bytes are still flushed before the
atomic rename, so the file is always whole; a new table file is flushed in
full.
- WagerJournal deletes a finished wager intent with deleteUnflushed.
Until the filesystem's next journal commit (any fsync on it, or ext4's 5 s
commit) an OS crash or power loss can bring back the previous complete table
file or the finished intent; a returning intent freezes the table for review,
which asks Denars for the posting's status. A server crash loses nothing.
New intents, review/conflict intents, estate publications and journals,
migrations and the running marker keep the full flush.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This repository does not receive automatic reviews because it has fewer than 10 stars.
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 397705cb-f954-4883-bbbf-849a7d462cac
Summary
Summary by CodeRabbit
Improvements
Table saves now replace existing files atomically, while new files receive a full directory flush.
Removing completed wager records avoids an additional directory sync; if a record reappears after a crash or power loss, the table is held for review.
File deletion and save operations now distinguish between new and existing files to apply the appropriate durability behaviour.
Durable file operations now support replacement writes and unflushed deletion. Table ownership saves and wager intent cleanup use these operations. Tests cover flush behaviour and update their call and failure checks.
writeReplacing skips the parent-directory flush when replacing an existing regular file. New files retain the directory flush. deleteUnflushed removes a file without syncing its parent directory. Tests cover these behaviours and confirm that regular writes and deletes still flush.
Table ownership saves use writeReplacing. After wager posting and table saving succeed, intent cleanup uses deleteUnflushed. Related tests now verify the updated calls and failure paths.
TF-Minecraft/Games#47: Changes the DurableFiles flush contract that this pull request extends.
TF-Minecraft/Games#49: Changes the same table-save and wager-journal paths, including their durability and intent-handling behaviour.
Suggested reviewers:ryanbarlow97
Merge Risk: Moderate · previous review 8fcaa
The change removes directory syncs from table saves and intent cleanup to cut main-thread stalls. After a power loss, chip placements and finished wagers could then lose their table update without the table being frozen for staff review. Resolve these two durability gaps, or explicitly accept them, before merging.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
…nt delete through deleteUnflushed
CI on the PR: tests that stub or count DurableFiles.write for table files, and the delete of an applied wager intent, now follow the save paths this branch moved to writeReplacing and deleteUnflushed. Estate publication stubs keep write, since that path still flushes in full.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
CodeRabbit on #50: POSIX does not order two directory updates, so with the
table save's folder flush skipped and the intent deleted unflushed, a power
loss could keep the deletion while losing the table rename: the table back at
its previous version, the posting applied, and no intent left to freeze it for
review. WagerJournal.apply now flushes the tables folder (new
TableManager.flushTableSaves via DurableFiles.flushDirectory) after the table
save and before deleteUnflushed. That still saves one sync per wager compared
with main (table save 1 data + 1 folder, intent delete without a folder flush).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d a failing flush
Review of e27a717: TableManager.checkpoint (estate plans take the saved table as their before) now flushes the tables folder too; WagerJournalTest checks the intent is still on disk when the table save is flushed and that a failing flush keeps the intent, freezes the table and posts nothing twice; the Source.durable comment now matches writeReplacing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Findings handled: the tables folder is flushed before a finished intent is deleted (e27a717, plus estate checkpoints in 7e3706e); the unjournaled-movement finding declined with reasoning in its thread. CodeRabbit is rate-limited; the fixes were reviewed independently (APPROVE) and retested on Dev.
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
size:M30-99 effective changed lines (tests excluded in mixed PRs).
1 participant
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.
github-actions
Bot
added
the
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
Two new
DurableFilesmethods drop the folder fsync where it buys nothing. They're used only for the table file save and for deleting a finished wager intent.writeReplacing: the new file's data is still fsynced before the atomic rename. The folder fsync is skipped only when the rename replaces a file that already existed; a new file still gets it.deleteUnflushed: deletes without a folder fsync.Why
One blackjack chip on an auto-tray table did about 8 disk syncs on the main thread.
coverAutoTraytook 6.2 s andtryPlaceChip's table save 2.2 s in Main's busiest hour on 11-02.Worst case
Checked
mainwith the same patch. Linux CI is the reference.Dev (TFMCDev01, 11-03)
games-DIRSYNC-c301d29-DEV.jar. Games enabled and its tables loaded, with no errors in the log.Draft: not merged, not released, nothing on Main or Tutorial.
🤖 Generated with Claude Code