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 save merging #13

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Fix save merging #13

merged 2 commits into from
Jul 11, 2023

Conversation

nezuo
Copy link
Owner

@nezuo nezuo commented Jul 11, 2023

Save merging is when you override a save that hasn't been processed yet with a new one.

-- This could result in only a single save call since they get merged.
document:save()
document:save()
document:save()

This fixes an edge case where we merge saves when it's not safe. For example, if you call save and then call close while the save is running UpdateAsync it would result in the document never being closed because the merge doesn't take affect since the UpdateAsync call has already been made.

Essentially it's always safe to merge saves unless UpdateAsync is running.

This PR fixes this edge case by keeping track of ongoingSaves and pendingSaves. ongoingSaves are saves that have been added to the throttle queue. pendingSaves are saves that are waiting for their respective ongoingSave to finish. We only merge pending saves now, we do not merge pending saves with ongoing saves. This means the merge algorithm is not completely optimal since it can be safe to merge saves even if they are in the throttle queue as long as they haven't called UpdateAsync. I plan to use the optimal algorithm eventually.

@nezuo nezuo merged commit 249f00b into master Jul 11, 2023
@nezuo nezuo deleted the fix-save-merging branch July 11, 2023 02:29
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