Skip to content

Conversation

grobinson-grafana
Copy link
Contributor

What this PR does / why we need it:

This pull request adds a new flag max-age to builders. It fixes a bug where low volume partitions would not have data objects built within a reasonable time.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@grobinson-grafana grobinson-grafana requested a review from a team as a code owner August 1, 2025 14:08
b.state = builderStateDirty
if b.state == builderStateEmpty {
b.state = builderStateDirty
b.firstAppendedAt = now
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I specifically chose now instead of the record timestamp. If we used the record timestamp, when replaying a partition (i.e. following a crash), we would build one data object per record, which is not what we want.

Copy link
Member

Choose a reason for hiding this comment

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

I think this might be better implemented at the consumer level, since that's also where we handle the idle timeout. WDYT?

Copy link
Member

@rfratto rfratto Aug 1, 2025

Choose a reason for hiding this comment

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

#18622 exposes the time range for a builder which would make doing this within the consumer possible, but nobody has been able to review it yet

Copy link
Contributor

github-actions bot commented Aug 1, 2025

@grobinson-grafana
Copy link
Contributor Author

Replaced with #18816.

@grobinson-grafana grobinson-grafana deleted the grobinson/add-max-age-to-builder branch August 12, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants