[ENH]: garbage collector v2 orchestrator (supports forking)

[codetheweb]

Description of changes

Adds a new garbage collector orchestrator that supports GC'ing fork trees. The new orchestrator is extensively tested with a proptest. It is not currently used outside of tests.

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[codetheweb]

• [ENH]: wire GC v2 to new cleanup modes & call FinishDatabaseDeletion from garbage collector #4671
• [ENH]: soft delete databases, add FinishDatabaseDeletion gRPC method to hard delete databases #4627
• [ENH]: perform collection hard deletes from garbage collector #4605
• [ENH]: sysdb changes to support moving collection hard deletes to garbage collector #4607
• [ENH]: add validation during garbage collection for empty file paths #4586
• [ENH]: garbage collector v2 orchestrator (supports forking) #4468 👈 (View in Graphite)
• [ENH]: allow deleting v0 from version file & correctly propagate errors on DeleteCollectionVersion #4579
• [ENH]: add operator for garbage collection to list all files used by collection version #4466
• [ENH]: add operator to compute versions to garbage collect from version graph #4464
• [ENH]: add orchestrator to construct version graph for garbage collection #4463
• [ENH]: add batch get version file paths method to Sysdb #4432 : 1 other dependent PR (#4447 )
• [ENH]: SysDb should return lineage, version file paths and root collection ID on collections #4557
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[sanketkedia]

Not sure what the right abstraction is - when do you override on_start() v/s set initial_tasks() and the default impl of on_start() takes care of creating the tasks

[codetheweb]

I have to override on_start() here because ConstructVersionGraphRequest is not a task. I would be in favor of removing initial_tasks() from the interface.

[propel-code-bot]

Garbage Collector V2 Orchestrator with Fork Tree Support and Proptest-Based Validation

This PR introduces a new, extensive, and modularized garbage collector orchestrator (garbage_collector_orchestrator_v2) in Rust that can handle collection version fork trees. Accompanied by a comprehensive property-based testing framework (proptest), the design validates correctness under a wide variety of complex version/fork scenarios. The orchestrator collaborates with modular operator components, and is currently only utilized by the test suite (not active production code), enabling rigorous validation before production rollout.

Key Changes:
• Addition of garbage_collector_orchestrator_v2.rs, a stand-alone GC orchestrator that supports version fork trees across collections.
• Complete module for property-based testing: reference model, arbitrary transition/graph generators, and a state machine test harness.
• Significant supporting test infrastructure: segment file strategies, proptest helpers, and detailed invariants/stats tracking.
• Refinements in library/operator interfaces (notably for delete_unused_files, list_files_at_version) to accommodate new GC orchestrator needs (parameter corrections, clear semantics, vector types, tenant-identification).
• Build and test updates (Cargo.toml, Cargo.lock) to support new dependencies like tracing-subscriber.
• Some refactorings and cleanup in existing operator components for compatibility and testability.

Affected Areas:
• rust/garbage_collector/src/garbage_collector_orchestrator_v2.rs (new orchestrator core)
• rust/garbage_collector/tests/proptest_helpers/* (test infrastructure)
• rust/garbage_collector/src/operators/* (GC-related operators refined)
• rust/garbage_collector/tests/prop_test_garbage_collection.rs (property-based top-level test)
• Cargo.toml/Cargo.lock
• system orchestrator trait defaulting (on_start/initial_tasks signature changes)
• blockstore sparse_index_test_utils

Potential Impact:

Functionality: No production functionality is changed, as GC v2 is not yet wired in; introduces rigorous GC/retention logic for potential future use.

Performance: No impact on runtime; test code is more thorough and possibly slower, but relegated to CI/testing.

Security: No changes.

Scalability: New test harness can validate GC correctness over arbitrarily large and complex version trees, improving confidence in future scalability.

Review Focus:
• Correctness of the garbage collector orchestrator logic for forked version trees.
• Soundness and thoroughness of the proptest state machine and invariants.
• Operator interface changes (delete_unused_files, list_files_at_version): review for breaking changes or backward compatibility.
• Test isolation: no accidental activation of new orchestrator in non-test code.
• Clarity/documentation within orchestrator and test-helper modules.

Testing Needed

• Run the property-based tests (test_k8s_integration_garbage_collection) to validate invariants for varied random workloads.
• CI should pass all unit/integration tests to confirm non-regression in operators and orchestrator.
• Manual (optional): review test statistics output to observe coverage and file re-use/fork characteristics.

Code Quality Assessment

rust/garbage_collector/tests/proptest_helpers/garbage_collector_under_test.rs: Complex but logically sound; follows proptest and state machine testing best practices.

rust/garbage_collector/src/operators/delete_unused_files.rs: Interface clarified; parameter types improved; explicit about tenant and input types.

rust/garbage_collector/tests/proptest_helpers/*: High-quality, readable proptest/test harness code.

system/src/execution/orchestrator.rs: Default trait implementations clarified and made safer.

rust/garbage_collector/src/garbage_collector_orchestrator_v2.rs: Well-structured, modular, focused on correctness/lint, clear error boundaries; documentation is robust.

Best Practices

Test Coverage:
• comprehensive invariant checks across generated fork/version trees

Testing:
• property-based testing
• state-machine equivalence
• invariants

Modularity:
• separation of orchestrator from operators
• explicit error handling

Documentation:
• inline docs for orchestrator and testing framework

Potential Issues

• Large test-generated graphs may expose subtle, unanticipated invariants or performance bottlenecks in end-to-end testing.
• Operator interface adjustments (e.g. input structures for file deletion) may require careful updating in production code when GC v2 is activated.

This summary was automatically generated by @propel-code-bot

[codetheweb]

Merge activity

• May 23, 10:54 PM UTC: @codetheweb merged this pull request with Graphite.