-
Notifications
You must be signed in to change notification settings - Fork 595
block-builder: remove standalone mode #11643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
Signed-off-by: Vladimir Varankin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Living the dream.
@@ -221,13 +189,12 @@ func (b *BlockBuilder) runningPullMode(svcCtx context.Context) error { | |||
} | |||
|
|||
// Once we've gotten a job, we attempt to complete it even if the context is cancelled. | |||
|
|||
if _, err := b.consumeJob(context.Background(), key, spec); err != nil { | |||
if _, err := b.consumeJob(context.WithoutCancel(ctx), key, spec); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nice.
@@ -37,8 +28,9 @@ func newBlockBuilderMetrics(reg prometheus.Registerer) blockBuilderMetrics { | |||
}, []string{"success"}) | |||
|
|||
m.processPartitionDuration = promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{ | |||
Name: "cortex_blockbuilder_process_partition_duration_seconds", | |||
Help: "Time spent processing one partition.", | |||
Name: "cortex_blockbuilder_process_partition_duration_seconds", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is now redundant with the similar consume_job
metric. It's in some dashboards, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I will clean up the dashboards after
What this PR does
Now, as we have block-builder-scheduler, we can remove the standalone mode, and continue iterating on top of its architecture.
This PR mechanically removes everything related the standalone mode. My idea is that after this one gets merged, we can go ahead and start looking at if there are any test scenarios missing, and gradually add those.