Skip to content

fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823).#1834

Open
junhaoliao wants to merge 8 commits intoy-scope:mainfrom
junhaoliao:docker-compose-tmp-tmpfs
Open

fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823).#1834
junhaoliao wants to merge 8 commits intoy-scope:mainfrom
junhaoliao:docker-compose-tmp-tmpfs

Conversation

@junhaoliao
Copy link
Member

@junhaoliao junhaoliao commented Dec 26, 2025

Description

This PR moves the garbage collector recovery file from the logs directory to the temporary directory, addressing the issues raised in #1823.

The archive_garbage_collector.py was writing its recovery file to clp_config.logs_directory, which:

  • Violated the convention of writing temporary files to temporary directories
  • Caused issues because the garbage-collector service didn't have a writable logs directory mounted

This PR:

  • Changed the recovery file location from clp_config.logs_directory to clp_config.tmp_directory
  • Added the *volume_clp_tmp mount to the garbage-collector service in Docker Compose
  • Fixed the Helm chart garbage-collector deployment to mount the tmp volume at /var/tmp

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Verified that the recovery file path now uses clp_config.tmp_directory
    Confirmed that the garbage-collector service has the tmp volume * mounted in Docker Compose
  • Verified the Helm chart mounts the tmp volume at the correct path /var/tmp

Docker Compose deployment

task && cd build/clp-package && ./sbin/start-clp.sh
2026-01-14T23:21:12.117 INFO [controller] Started CLP.

All services started successfully including garbage-collector with the tmp volume mounted.

Helm chart deployments

Single-node setup

cd tools/deployment/package-helm && ./set-up-test.sh
pod/test-clp-garbage-collector-6bd59567d7-8b54g condition met
...
All jobs completed and services are ready.

Multi-node setup with shared worker nodes

cd tools/deployment/package-helm && ./set-up-multi-shared-test.sh
pod/test-clp-garbage-collector-6bd59567d7-875ws condition met
...
All jobs completed and services are ready.

Multi-node setup with dedicated worker nodes

cd tools/deployment/package-helm && ./set-up-multi-dedicated-test.sh
pod/test-clp-garbage-collector-6bd59567d7-fccqm condition met
...
All jobs completed and services are ready.

Summary by CodeRabbit

Release Notes

  • Chores
    • Bumped Helm chart version to 0.1.4-dev.3.
    • Adjusted temporary file storage location for the garbage collection service to use the system temp directory (*.tmp).
    • Updated deployment manifests and compose config to mount the tmp volume at the new path (/var/tmp) across environments.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 26, 2025

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0aa5f and df52e4f.

📒 Files selected for processing (2)
  • tools/deployment/package-helm/Chart.yaml
  • tools/deployment/package/docker-compose-all.yaml

Walkthrough

The archive garbage collector's recovery file location is changed from the logs directory to the temporary directory. Deployment manifests (Helm and Docker Compose) are updated to mount the tmp volume at the new path, and the Helm chart version is incremented.

Changes

Cohort / File(s) Summary
Archive garbage collector code
components/job-orchestration/job_orchestration/garbage_collector/archive_garbage_collector.py
Recovery file path moved from logs directory to temporary directory; no logic or control-flow changes.
Helm deployment template
tools/deployment/package-helm/templates/garbage-collector-deployment.yaml
tmp volume mountPath changed from /var/log to /var/tmp.
Docker Compose configuration
tools/deployment/package/docker-compose-all.yaml
Added volume_clp_tmp to the garbage_collector service's volume mounts.
Helm chart metadata
tools/deployment/package-helm/Chart.yaml
Chart version bumped from 0.1.4-dev.2 to 0.1.4-dev.3.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: moving the garbage collector recovery file from the logs directory to the temporary directory, which is the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

junhaoliao and others added 2 commits January 7, 2026 15:22
# Conflicts:
#	tools/deployment/package-helm/Chart.yaml
#	tools/deployment/package-helm/templates/garbage-collector-deployment.yaml
@junhaoliao
Copy link
Member Author

Since "using tmpfs instead of binding mount" is a different requirement from changing the GC file list directory to use clpConfig.tmp_directory, i believe that change should be submitted in a separate PR.

@junhaoliao junhaoliao changed the title fix(clp-package)!: Use tmpfs for temporary directory mounts in Docker Compose. fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823). Jan 14, 2026
@junhaoliao junhaoliao marked this pull request as ready for review January 14, 2026 23:31
@junhaoliao junhaoliao requested a review from a team as a code owner January 14, 2026 23:31
@junhaoliao junhaoliao modified the milestones: Backlog, February 2026 Jan 19, 2026
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