Skip to content

[CHORE](garbage_collector): remove log-only orchestrator - #6971

Open
rescrv wants to merge 1 commit into
mainfrom
rescrv/excise-log-only-orchestrator
Open

[CHORE](garbage_collector): remove log-only orchestrator#6971
rescrv wants to merge 1 commit into
mainfrom
rescrv/excise-log-only-orchestrator

Conversation

@rescrv

@rescrv rescrv commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Description of changes

Excise the HardDeleteLogOnlyGarbageCollectorOrchestrator, which was a
separate code path for hard-deleting logs of destroyed collections.
Manual GC requests now look up collection info via sysdb and feed
them into the regular GC pipeline. Collections that no longer exist
are skipped with a log message instead of being routed to a separate
orchestrator.

  • Replace HashMap<CollectionUuid, DatabaseName> with
    HashSet for manual_collections since the database
    name is now looked up on demand
  • Add lookup_manual_collection_to_gc to query sysdb directly
  • Remove second job stream and its duplicated result processing loop
  • Add test for lookup returning None after collection removal

Test plan

Locally tested gc lib + CI

Migration plan

N/A

Observability plan

N/A

Documentation Changes

N/A

Co-authored-by: AI

@github-actions

Copy link
Copy Markdown

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)

@propel-code-bot

propel-code-bot Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

garbage_collector: remove separate log-only orchestrator and unify manual GC path

This PR removes the dedicated HardDeleteLogOnlyGarbageCollectorOrchestrator flow and routes manual GC requests through the existing main GC pipeline. Manual requests now store only CollectionUuid values in manual_collections, and collection metadata (including database) is resolved on demand via a new lookup_manual_collection_to_gc helper that queries sysdb.

The change simplifies execution by eliminating the second job stream and duplicate result-processing loop for log-only hard delete work. If a manually requested collection no longer exists (or has invalid/multiple lookup results), it is skipped with logging instead of being routed to a separate orchestrator path. Tests were updated to reflect HashSet-based manual tracking and to verify lookup returns None after collection removal.

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

@propel-code-bot propel-code-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review found no issues; refactor appears sound with simplified GC flow and updated tests.

Status: No Issues Found | Risk: Low

Review Details

📁 3 files reviewed | 💬 0 comments

@rescrv
rescrv requested a review from Sicheng-Pan April 24, 2026 21:38
Base automatically changed from rescrv/gc-version-files to main April 25, 2026 01:26
@rescrv
rescrv force-pushed the rescrv/excise-log-only-orchestrator branch from a10c1a6 to a696382 Compare April 27, 2026 17:41
Excise the HardDeleteLogOnlyGarbageCollectorOrchestrator, which was a
separate code path for hard-deleting logs of destroyed collections.
Manual GC requests now look up collection info via sysdb and feed
them into the regular GC pipeline. Collections that no longer exist
are skipped with a log message instead of being routed to a separate
orchestrator.

- Replace HashMap<CollectionUuid, DatabaseName> with
  HashSet<CollectionUuid> for manual_collections since the database
  name is now looked up on demand
- Add lookup_manual_collection_to_gc to query sysdb directly
- Remove second job stream and its duplicated result processing loop
- Add test for lookup returning None after collection removal

Co-authored-by: AI
@rescrv
rescrv force-pushed the rescrv/excise-log-only-orchestrator branch from a696382 to 0c46f47 Compare April 27, 2026 23:31
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