-
Notifications
You must be signed in to change notification settings - Fork 2k
[ENH]: add operator to compute versions to garbage collect from version graph #4464
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
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
3185a2c to
2d8c1b7
Compare
fcefe15 to
042312d
Compare
042312d to
8e4042e
Compare
| if *version == 0 { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this implies we never delete the first version of the new collection after fork, and thus we can never gc the data of the source collection at the forking version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct
I did this to preserve the existing behavior of GC but I'm fine changing it to allow deleting the first version
2d8c1b7 to
fafe998
Compare
8e4042e to
34ed231
Compare
|
Add Operator for Computing Versions to Delete from Version Graph This PR introduces a new operator, Key Changes: Affected Areas: This summary was automatically generated by @propel-code-bot |
rust/garbage_collector/src/operators/compute_versions_to_delete_from_graph.rs
Show resolved
Hide resolved
rust/garbage_collector/src/operators/compute_versions_to_delete_from_graph.rs
Show resolved
Hide resolved
rust/garbage_collector/src/operators/compute_versions_to_delete_from_graph.rs
Show resolved
Hide resolved
rust/garbage_collector/src/operators/compute_versions_to_delete_from_graph.rs
Show resolved
Hide resolved
fafe998 to
ef973c9
Compare
34ed231 to
9ca2cd5
Compare
ef973c9 to
bdc98f5
Compare
9ca2cd5 to
0725564
Compare
bdc98f5 to
8137f4d
Compare
0725564 to
dd8eba1
Compare
8137f4d to
14eb5e7
Compare
dd8eba1 to
d7546c4
Compare
14eb5e7 to
721349a
Compare
d7546c4 to
8242dc4
Compare
721349a to
8f8a50b
Compare
8242dc4 to
5f592dc
Compare
af861f5 to
8fe5f2f
Compare
cb3e44a to
1242760
Compare
8fe5f2f to
afda731
Compare
1242760 to
1a374a3
Compare
afda731 to
1ea19ad
Compare
1a374a3 to
f4a6ed4
Compare
1ea19ad to
e949621
Compare
f4a6ed4 to
cdff65d
Compare
e949621 to
91a3c81
Compare
cdff65d to
3ec6e51
Compare
91a3c81 to
f71445b
Compare
797d3f7 to
7591c20
Compare
f71445b to
c2e5444
Compare
7591c20 to
38742ab
Compare
c2e5444 to
56724a9
Compare
56724a9 to
0197f05
Compare
Merge activity
|
…on graph (chroma-core#4464) ## Description of changes Adds an operator to compute which versions to garbage collect from a version graph. Intended to replace `rust/garbage_collector/src/operators/compute_versions_to_delete.rs`. ## Test plan _How are these changes tested?_ - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust Added new tests for operator. ## Documentation Changes _Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs section](https://github.com/chroma-core/chroma/tree/main/docs/docs.trychroma.com)?_ n/a

Description of changes
Adds an operator to compute which versions to garbage collect from a version graph. Intended to replace
rust/garbage_collector/src/operators/compute_versions_to_delete.rs.Test plan
How are these changes tested?
pytestfor python,yarn testfor js,cargo testfor rustAdded new tests for operator.
Documentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?
n/a