Skip to content

fix(log-ingestor): Handle killed compression jobs in CompressionJobSubmitter (fixes #1983).#1988

Merged
LinZhihao-723 merged 3 commits into
y-scope:mainfrom
LinZhihao-723:handle-killed-compression-job
Feb 13, 2026
Merged

fix(log-ingestor): Handle killed compression jobs in CompressionJobSubmitter (fixes #1983).#1988
LinZhihao-723 merged 3 commits into
y-scope:mainfrom
LinZhihao-723:handle-killed-compression-job

Conversation

@LinZhihao-723

@LinZhihao-723 LinZhihao-723 commented Feb 12, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Description

This PR fixes #1983 by properly handling the killed compression job inside the compression job submitter.

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

  • Ensure all workflows pass.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error logging for compression job status changes so messages appear as readable text instead of debug representations.
    • Ensured compression jobs exit immediately and cleanly when they fail or are terminated, preventing further processing and reducing erroneous follow-up actions.

@LinZhihao-723 LinZhihao-723 requested a review from a team as a code owner February 12, 2026 22:35
@coderabbitai

coderabbitai Bot commented Feb 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Modify compression job status handling to log Failed and Killed states using plain string formatting, use status_message.as_deref().unwrap_or("None"), and immediately return after logging to stop further processing for those states.

Changes

Cohort / File(s) Summary
Compression Job Status Handling
components/log-ingestor/src/compression/compression_job_submitter.rs		 Replaced debug formatting ({:?}) with string formatting ({}) for Failed and Killed statuses; use status_message.as_deref().unwrap_or("None") when printing; add explicit return after logging so both branches exit immediately.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The code changes address issue #1983 by implementing handling for killed compression jobs with logging and early returns, but do not show implementation for marking ingested files as failed. Verify that ingested files associated with killed compression jobs are marked as 'failed' as required by issue #1983. The current changes only show logging and control flow updates.
✅ Passed checks (5 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: handling killed compression jobs in CompressionJobSubmitter, with a specific reference to the linked issue.
Out of Scope Changes check ✅ Passed All changes appear scoped to handling killed compression jobs. Logging improvements and control flow adjustments are directly related to the issue objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

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

No actionable comments were generated in the recent review. 🎉

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.

coderabbitai[bot]
coderabbitai Bot reviewed Feb 12, 2026
View reviewed changes

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/log-ingestor/src/compression/compression_job_submitter.rs (1)

42-46: ⚠️ Potential issue | 🔴 Critical

Spawned compression task failures are not propagated; files are not marked as failed.

The JoinHandle is dropped, so when the spawned submit_clp_compression_job_and_wait_for_completion detects a Failed or Killed job (lines 166–180), it only logs and exits. The submit() method has already returned Ok(()) to the caller, and there is no mechanism to mark the ingested files associated with that failed job as failed. Either this is handled at a different layer, or it remains TODO per the linked issue.

🤖 Fix all issues with AI agents 
In `@components/log-ingestor/src/compression/compression_job_submitter.rs`:
- Line 170: In the tracing::error! calls where status_message is passed (in the
branches handling the process status, including the Killed branch), replace
status_message.unwrap_or_else(|| "None".to_string()) with
status_message.as_deref().unwrap_or("None") so you borrow an &str instead of
allocating a new String; update both occurrences that reference status_message
(the normal failure branch and the Killed branch) to use
as_deref().unwrap_or("None") so the log takes a &str with zero allocation.

Comment thread components/log-ingestor/src/compression/compression_job_submitter.rs Outdated
hoophalab
hoophalab previously approved these changes Feb 12, 2026
View reviewed changes
@LinZhihao-723 LinZhihao-723 merged commit 969b515 into y-scope:main Feb 13, 2026
22 checks passed
@junhaoliao junhaoliao added this to the February 2026 milestone Feb 26, 2026
junhaoliao pushed a commit to junhaoliao/clp that referenced this pull request May 17, 2026
@LinZhihao-723
fix(log-ingestor): Handle killed compression jobs in `CompressionJobS…
01d3ad9 
…ubmitter` (fixes y-scope#1983). (y-scope#1988)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@hoophalab hoophalab hoophalab approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

February 2026

Development

Successfully merging this pull request may close these issues.

[log-ingestor] Handling for killed compression jobs is missing.

3 participants

@LinZhihao-723 @hoophalab @junhaoliao