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

[Improvement] Improve logic about dropping old version of KV data #1276

Closed
yuqi1129 opened this issue Dec 28, 2023 · 0 comments · Fixed by #2918
Closed

[Improvement] Improve logic about dropping old version of KV data #1276

yuqi1129 opened this issue Dec 28, 2023 · 0 comments · Fixed by #2918
Assignees
Labels
improvement Improvements on everything

Comments

@yuqi1129
Copy link
Contributor

What would you like to be improved?

Currently, the logic for dropping the old version of KV Paris is:

  • Calculate the version to be deleted: any data that version lower than the calculated version can be dropped.
  • Scan all transaction marks that are lower than the calculated version.
  • Get all KV pairs for each transaction mark, and then check whether they can be dropped. Pairs should be recycled if
    • It has been marked deleted.
    • It has a new version of data.

The procedures outlined have a drawback: we need to scan all transaction marks each time if wants to collect all data out of version exactly, it would be very time-consuming and not very elegant.

How should we improve?

No response

@yuqi1129 yuqi1129 added the improvement Improvements on everything label Dec 28, 2023
@jerryshao jerryshao added this to the Gravitino 0.5.0 milestone Feb 4, 2024
@yuqi1129 yuqi1129 self-assigned this Apr 12, 2024
jerryshao pushed a commit that referenced this issue Apr 16, 2024
…f data in KvGcCollector (#2918)

### What changes were proposed in this pull request?

Introduce a variable to mark the last transaction ID and perform the GC
from the last transaction ID next time to fulfill `incremental GC`.

### Why are the changes needed?

Full GC for the old version of the data takes a lot of time, we'd better
not use this method.

Fix: #1276 

### Does this PR introduce _any_ user-facing change?

N/A.

### How was this patch tested?

Existing tests and test locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvements on everything
Projects
None yet
2 participants