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

Detach fields in-order after consensus match #843

Open
wants to merge 4 commits into
base: branch-24.03
Choose a base branch
from

Conversation

manopapad
Copy link
Contributor

No description provided.

@manopapad manopapad added the category:bug-fix PR is a bug fix and will be classified as such in release notes label Sep 27, 2023
@manopapad manopapad marked this pull request as ready for review September 28, 2023 00:19
@lightsighter
Copy link

So if I'm understanding this correctly, this version effectively does a consensus match to see which fields to recycle, and when it does that, then it see which fields can be recycled across all the shards, and it does the ordered detach at that point right before it recycles the IDs in the same way across all the shards. If that's accurate, it definitely seems better than doing the unordered detach operations.

One obvious idea to play around with here is whether we can pipeline this process. For example, if we shrunk the number of allocations between consensus match calls down to 16 (from the default 32?), but then instead of blocking to wait for the result, we just saved the result for the next 16 allocation calls to be done, and then look at the result of the previous consensus match to see if we can recycle any of those fields? Seems like that might be better than exposing the latency of the consensus match. We could also probably make the pipeline three stages deep:

  1. Start consensus match
  2. Check consensus match and issue ordered deletions for any fields
  3. Check futures that are done and finalize actually remove references to their external resources

@marcinz marcinz changed the base branch from branch-23.11 to branch-24.01 November 9, 2023 16:51
@marcinz marcinz changed the base branch from branch-24.01 to branch-24.03 February 22, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bug-fix PR is a bug fix and will be classified as such in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants