Skip to content

Metrics Generator: Max limit on number of failed flushes - #4254

Merged
joe-elliott merged 4 commits into
grafana:mainfrom
joe-elliott:dont-retry-generator-flush
Oct 31, 2024
Merged

Metrics Generator: Max limit on number of failed flushes#4254
joe-elliott merged 4 commits into
grafana:mainfrom
joe-elliott:dont-retry-generator-flush

Conversation

@joe-elliott

Copy link
Copy Markdown
Collaborator

What this PR does:
Institutes a very generous limit of 100 on failed flush attempts as well as a metric to help track this. This prevents constant retries on blocks that cannot be flushed due to corruption.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott
joe-elliott requested a review from yvrhdn as a code owner October 31, 2024 12:16

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

LGTM

go func() {
time.Sleep(delay)
if _, err := p.flushqueue.Enqueue(op); err != nil {
_ = level.Error(p.logger).Log("msg", "failed to requeue block for flushing", "err", err)

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.

What happens if we fail to requeue? Should we delete the block as well?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

i think no. the only time enqueue can fail is if the queue is stopped:

https://github.com/grafana/tempo/blob/main/pkg/flushqueues/priority_queue.go#L91-L93

in this case the process is shutting down and so we want to keep the block for when it starts back up. we could definitely be more explicit about this tho.

Comment thread modules/generator/processor/localblocks/processor.go Outdated
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott
joe-elliott merged commit d035888 into grafana:main Oct 31, 2024
mattdurham pushed a commit to mattdurham/tempo that referenced this pull request Jun 18, 2026
* dont retry failed flushes

Signed-off-by: Joe Elliott <number101010@gmail.com>

* changelog

Signed-off-by: Joe Elliott <number101010@gmail.com>

* add sleep for backoff

Signed-off-by: Joe Elliott <number101010@gmail.com>

* add err

Signed-off-by: Joe Elliott <number101010@gmail.com>

---------

Signed-off-by: Joe Elliott <number101010@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants