feat(clp-package): Add Python package config, metadata-DB column, and scheduler handoff for the compression coordinator. - #2421
Conversation
WalkthroughAdds Spider and compression coordinator configuration with validation, extends the compression jobs schema for coordinator tracking, and defers S3 metadata jobs to coordinator handling when enabled. ChangesSpider compression coordinator
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/clp-py-utils/clp_py_utils/initialize-orchestration-db.py`:
- Around line 71-87: Extend the existing compression_jobs migration logic in
initialize-orchestration-db.py beyond the current JOB_START_TIME_STATUS
alteration: add idempotent migrations for spider_id, dispatch_time,
JOB_SPIDER_ID, and the revised update_time definition. Ensure upgraded tables
receive the same columns, index, and update behavior declared by the CREATE
TABLE schema without affecting fresh installations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7503cb9-9232-4b84-8dd8-f5769271af3d
📒 Files selected for processing (3)
components/clp-py-utils/clp_py_utils/clp_config.pycomponents/clp-py-utils/clp_py_utils/initialize-orchestration-db.pycomponents/job-orchestration/job_orchestration/scheduler/compress/compression_scheduler.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/clp-py-utils/clp_py_utils/initialize-orchestration-db.py (1)
71-87: 🗄️ Data Integrity & Integration | 🟠 MajorMigrate existing
compression_jobstables as well.
CREATE TABLE IF NOT EXISTSdoes not update existing deployments, and the onlyALTER TABLEbelow addsJOB_START_TIME_STATUS. Upgraded databases will still lackspider_id,dispatch_time,JOB_SPIDER_ID, and the revisedupdate_timebehaviour, causing coordinator reads or writes to fail. Add idempotent migrations for each schema change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/clp-py-utils/clp_py_utils/initialize-orchestration-db.py` around lines 71 - 87, Update the compression_jobs schema initialization and migration logic around the CREATE TABLE and existing ALTER TABLE statements to add idempotent migrations for spider_id, dispatch_time, JOB_SPIDER_ID, and the revised update_time definition. Ensure existing tables receive these columns, index, and update behavior without failing when already applied, while preserving the current creation path for new tables.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/clp-py-utils/clp_py_utils/clp_config.py`:
- Around line 861-862: Update transform_for_container() to apply the same
container hostname and port mapping used for spider_scheduler to the spider
endpoint, preserving the configured external behavior only if that is explicitly
intended and documented. Ensure containerized CompressionCoordinator connections
use the transformed Spider service endpoint instead of the default
localhost:6000.
---
Duplicate comments:
In `@components/clp-py-utils/clp_py_utils/initialize-orchestration-db.py`:
- Around line 71-87: Update the compression_jobs schema initialization and
migration logic around the CREATE TABLE and existing ALTER TABLE statements to
add idempotent migrations for spider_id, dispatch_time, JOB_SPIDER_ID, and the
revised update_time definition. Ensure existing tables receive these columns,
index, and update behavior without failing when already applied, while
preserving the current creation path for new tables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d67628f-c429-4e36-b976-56c12a0f0af3
📒 Files selected for processing (3)
components/clp-py-utils/clp_py_utils/clp_config.pycomponents/clp-py-utils/clp_py_utils/initialize-orchestration-db.pycomponents/job-orchestration/job_orchestration/scheduler/compress/compression_scheduler.py
…tration-db.py; handled by y-scope#2421.
Description
This PR adds the Python-side support that the compression coordinator needs: the package configuration for the coordinator and its Spider connection, the
spider_idcolumn the coordinator uses to track and recover Spider jobs, and a handoff in the legacy compression scheduler so it stops competing for the jobs the coordinator now owns.The configuration added here mirrors the
spiderandcompression_coordinatorsections introduced on the Rust side in #2417, so it depends on #2417 for the config to be meaningful. It does not strictly build on top of #2417, though: none of these changes reference or require that PR's code, and this PR can be reviewed and merged independently.Package configuration
Adds
SpiderandCompressionCoordinatorconfig models (plus their nestedSpiderResourceGroupandPollingBackoff) toclp_config.py, and exposes them as optionalspiderandcompression_coordinatorfields onClpConfig(both defaultNone).CompressionCoordinator's defaults (polling interval, result-poll backoff, task retry limits, DB pool size, termination and commit-task timeouts) match the RustDefaultimplementation.SpiderResourceGroupandPollingBackoffcarry no field defaults of their own; their values are supplied where they are initialized insideCompressionCoordinator.validate_compression_coordinator_configmodel validator: whencompression_coordinatoris configured, it requires theclp-sstorage engine and a non-nullspiderconfig. It intentionally does not couple the coordinator tospider_scheduleror to Spider orchestration, so the coordinator can run alongside a Celery-based compression scheduler.Metadata-DB schema
Adds a
spider_id BIGINT UNSIGNED NULL DEFAULT NULLcolumn and aJOB_SPIDER_IDindex to thecompression_jobstable. The coordinator writes the Spider job ID here once it submits a job to Spider, and on startup it recovers in-flight jobs by selecting theRUNNINGrows whosespider_idis non-null. The column is nullable and defaults toNULLbecause a job has no Spider job ID until the coordinator submits it.Compression scheduler handoff
When
compression_coordinatoris configured, the legacy Celery-based compression scheduler skipsS3_OBJECT_METADATAjobs: it logs the skip and leaves the job inPENDINGso the coordinator picks it up. The coordinator handles onlyS3_OBJECT_METADATAinput, so this cleanly partitions the work — all other input types (FS, S3) continue to flow through the legacy scheduler unchanged.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Bug Fixes