Skip to content

[zephyr] skip status log when no counters recorded#5176

Open
wmoss wants to merge 1 commit intomainfrom
claude/issue-5175-20260425-1716
Open

[zephyr] skip status log when no counters recorded#5176
wmoss wants to merge 1 commit intomainfrom
claude/issue-5175-20260425-1716

Conversation

@wmoss
Copy link
Copy Markdown
Collaborator

@wmoss wmoss commented Apr 25, 2026

Fixes #5175.

Map-only stages don't run their output through StatisticsGenerator, so the periodic status logger in subprocess_worker._periodic_status_logger kept emitting items=0 (0.0/s), bytes_processed=0.0MiB for the entire shard runtime — noisy and misleading. Now we skip the log line entirely until at least one of the item/byte counters has been recorded.

Generated with Claude Code

Map-only stages don't run their output through StatisticsGenerator, so
the periodic status logger spammed `items=0 (0.0/s), bytes_processed=0.0MiB`
for the entire shard runtime. Skip the log line until at least one of
the item/byte counters has been recorded — once data starts flowing,
zeroes from one side are still informative.

Fixes #5175.

Co-authored-by: Will Moss <wmoss@users.noreply.github.com>
@wmoss wmoss requested a review from ravwojdyla April 25, 2026 17:32
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 25, 2026

Claude finished @wmoss's task in 2m 10s —— View job


Code review

No issues found. Checked for bugs and CLAUDE.md/AGENTS.md compliance.

The change is minimal and well-scoped:

  • _periodic_status_logger short-circuits with continue when neither item_key nor byte_key has been recorded in ctx._counters. Once either appears, normal logging resumes — so map-only stages stay quiet without affecting stages that do flow through StatisticsGenerator.

  • The two new tests cover both branches (skip on empty counters, log once at least one is present), use caplog filtered by message substring, and stop the daemon thread cleanly via stop_event.

  • Read PR diff and context

  • Read AGENTS.md files for changed paths

  • Audit for CLAUDE.md/AGENTS.md compliance

  • Scan for bugs/logic issues

  • Validate any issues found

  • Post review summary
    Branch

Copy link
Copy Markdown
Contributor

@ravwojdyla ravwojdyla left a comment

Choose a reason for hiding this comment

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

We need to update the coordinator as well?

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.

Optional counter/perf stats logs

2 participants