Skip to content
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

fix: Exclude unverified checkpoint marker when listing all paths for marking ReadOnly #4429

Merged
merged 2 commits into from
Mar 24, 2025

Conversation

ShuoWangNSL
Copy link
Contributor

This PR fixes the test failure of can_state_sync_into_existing_checkpoint, where the main thread removes the unverified checkpoint marker but the state sync thread tries to mark it as readonly.

It excludes the marker from the list of paths, since another thread might also be validating the checkpoint and may have already deleted the marker. Marking the unverified marker as read-only is unnecessary for this function's purpose and may cause an error.

@github-actions github-actions bot added the fix label Mar 19, 2025
@ShuoWangNSL ShuoWangNSL changed the title fix: Remove unverified checkpoint marker when listing all paths fix: Exclude unverified checkpoint marker when listing all paths for marking ReadOnly Mar 19, 2025
@ShuoWangNSL ShuoWangNSL force-pushed the shuo/exclude_unverified_marker_when_listing branch 2 times, most recently from 1a882e0 to 4cd973e Compare March 19, 2025 07:57
@ShuoWangNSL ShuoWangNSL force-pushed the shuo/exclude_unverified_marker_when_listing branch from 4cd973e to 0258f12 Compare March 19, 2025 08:03
@ShuoWangNSL ShuoWangNSL marked this pull request as ready for review March 19, 2025 14:26
@ShuoWangNSL ShuoWangNSL requested a review from a team as a code owner March 19, 2025 14:26
@ShuoWangNSL
Copy link
Contributor Author

Run a production test of installing 100k canisters.
dir_list_recursive takes around 2s. excluding the marker from list takes around 40ms. marking files readonly in parallel takes around 100ms. syncfs takes around 5s.

The 40ms additional time is OK as it is not significant compared to others and the whole function happens asynchronously during checkpointing.

@ShuoWangNSL ShuoWangNSL added this pull request to the merge queue Mar 24, 2025
Merged via the queue into master with commit 27c9d2b Mar 24, 2025
24 checks passed
@ShuoWangNSL ShuoWangNSL deleted the shuo/exclude_unverified_marker_when_listing branch March 24, 2025 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants