feat(compression-coordinator): Add the Spider compression job-submission API and its task I/O types. - #2401
Conversation
WalkthroughAdds shared Serde types for CLP S3 compression tasks and introduces a workspace crate exposing an asynchronous API to submit Spider jobs and await terminal outcomes. ChangesCompression coordinator foundation
Estimated code review effort: 2 (Simple) | ~15 minutes 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: 2
🤖 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-rust-utils/src/task_io/compression.rs`:
- Around line 39-40: Update the file-size fields `size` and `uncompressed_size`
to use `u64` instead of `i64`, matching the unsigned type used by
`target_encoded_size` in `ClpSCompressionOption`; adjust any affected
construction, conversion, or arithmetic sites to preserve compilation and
unsigned size semantics.
In `@components/compression-coordinator/src/error.rs`:
- Around line 4-5: Add error variants to the Error enum for task graph
construction failures, task-input serialization failures, and rejected cluster
requests, with payloads that preserve the underlying error details where
applicable. Ensure these variants support the Result error paths exposed by
S3CompressionJobSubmitter.
🪄 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
Run ID: 7387285a-1de9-4ac7-8c87-19bc070d5ac6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
Cargo.tomlcomponents/clp-rust-utils/src/lib.rscomponents/clp-rust-utils/src/task_io.rscomponents/clp-rust-utils/src/task_io/compression.rscomponents/compression-coordinator/Cargo.tomlcomponents/compression-coordinator/src/compression_job_submitter.rscomponents/compression-coordinator/src/error.rscomponents/compression-coordinator/src/lib.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/compression-coordinator/src/compression_job_submitter/mod.rs (1)
30-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
Clonesupertrait to preserve object safety.Traits that require
Cloneare not object-safe becauseClone::clonereturnsSelf. The primary benefit of using the#[async_trait]macro in modern Rust is to allow dynamic dispatch (e.g.,Box<dyn S3CompressionJobSubmitter>), but adding theClonebound defeats this by making the trait non-object-safe.If consumers need to clone the submitter, they can either:
- Require
Cloneat the generic use-site (<T: S3CompressionJobSubmitter + Clone>).- Wrap an object-safe trait in an
Arc(e.g.,Arc<dyn S3CompressionJobSubmitter>).♻️ Proposed refactor
#[async_trait] -pub trait S3CompressionJobSubmitter: Clone + Send + Sync { +pub trait S3CompressionJobSubmitter: Send + Sync {🤖 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/compression-coordinator/src/compression_job_submitter/mod.rs` around lines 30 - 31, Remove the Clone supertrait from S3CompressionJobSubmitter so the #[async_trait] trait remains object-safe and usable through dynamic dispatch. Preserve Clone requirements only at generic consumer sites or via Arc-wrapped trait objects where cloning is needed.
🤖 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.
Outside diff comments:
In `@components/compression-coordinator/src/compression_job_submitter/mod.rs`:
- Around line 30-31: Remove the Clone supertrait from S3CompressionJobSubmitter
so the #[async_trait] trait remains object-safe and usable through dynamic
dispatch. Preserve Clone requirements only at generic consumer sites or via
Arc-wrapped trait objects where cloning is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b57c662-3e45-4902-88e3-4a26b2e9db4a
📒 Files selected for processing (3)
components/compression-coordinator/src/compression_job_submitter/mod.rscomponents/compression-coordinator/src/compression_job_submitter/spider.rscomponents/compression-coordinator/src/lib.rs
💤 Files with no reviewable changes (1)
- components/compression-coordinator/src/lib.rs
The initial and maximum job-state poll backoffs were fixed constants inside the Spider implementation, so a caller had no way to tune how aggressively a job is polled. Both are now `Duration` parameters of `run_s3_compression_job_to_completion`, leaving the pacing decision with the caller that knows how long its jobs typically run.
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/compression-coordinator/src/compression_job_submitter/mod.rs`:
- Around line 81-86: Update the Error enum in error.rs to represent failures
from run_s3_compression_job_to_completion and submit_s3_compression_job,
including task construction, serialization, rejected Spider requests, and
polling failures. Add concrete variants with preserved source errors where
applicable, so both fallible APIs can return meaningful errors instead of
relying on the empty enum.
🪄 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
Run ID: 737353d2-f106-4b3d-b362-0e3f5ff78995
📒 Files selected for processing (1)
components/compression-coordinator/src/compression_job_submitter/mod.rs
Bill's partition.rs is byte-identical to the version this branch started from, so the refactored version is kept as-is. His merge of main brings only y-scope#2401, which this branch already has via rebase. The net change is his combined crate doc in lib.rs.
Description
Adds the scaffolding for a compression coordinator that drives CLP compression jobs on a Spider cluster: the protocol types exchanged with the Spider tasks, and the trait that submits and awaits a job. This is the interface layer only — the coordinator that consumes it, the tasks that implement it, and the
SpiderClientimplementation of the trait are not part of this PR.clp-rust-utils:task_io::compressionThe types that cross the Spider FFI boundary as opaque, msgpack-encoded
bytespayloads:ClpSCompressionOption— theclp-stuning options shared by every task in a job (--target-encoded-size,--compression-level,--timestamp-key).S3InputSource— one compression task's partition, i.e. the exact S3 objects it should compress, along with the bucket and authentication needed to read them.CompressionTaskOutput— what one compression task returns: the job's dataset plus oneArchiveMetadataper archive produced.ArchiveMetadata— a single produced archive. The field set mirrors the values Python'supdate_archive_metadatainserts into thearchivestable, minus thecreator_id/creation_ixdefaults that the commit task injects at insert time.These live in
clp-rust-utilsrather than in the coordinator because both sides of the boundary need them: the coordinator serializes them when submitting a job, and the Spider-side tasks deserialize them when running one.compression-coordinator:S3CompressionJobSubmitterA new crate holding the submission API:
submit_s3_compression_jobbuilds the compression task graph for a set of input sources and registers it with Spider without starting it, returning theJobIdto be persisted against the CLP compression job.run_s3_compression_job_to_completionidempotently starts a job (safe to call whether it is not-yet-started, already running, or already terminal) and waits until it reaches a terminal state, reporting the outcome asCompressionJobOutcome::{Succeeded, Failed, Cancelled}.Splitting submission from execution lets the caller persist the Spider job id before any work starts, so a coordinator restart can still find and resume an in-flight job.
Several aspects of the API are deliberately left to the caller rather than fixed by the implementation:
ExecutionPolicy, so a task's timeout and retry behaviour can scale with the size of the partition it was given instead of every task sharing one blanket policy.ExecutionPolicy, since it is a different kind of work from compression — a single short database transaction rather than a long-running compression — and shouldn't inherit whatever policy suits the compression tasks.initial_poll_backoffandmax_poll_backoff, since how aggressively to poll depends on how long the caller's jobs typically run.Erroris introduced as the crate-level error type with no variants yet; the implementation PR adds them alongside the code that can fail.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Summary by CodeRabbit