Skip to content
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

Fix creation of already pruned tenants #5655

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

fpetkovski
Copy link
Contributor

@fpetkovski fpetkovski commented Aug 29, 2022

When a tenant is pruned from a Receiver, its metrics will remain in
the shipper's Prometheus registry. This leads to a panic when the same
tenant is created again in the Receiver since multiTSDB will attempt to register
the same same metrics again.

The current solution for the TSDB metrics is to wrap the registry in
an UnRegisterer which can deregister metrics with the same name before
registering new metrics. This commit applies the same solution to the
shipper metrics.

Fixes #5626

Signed-off-by: Filip Petkovski filip.petkovsky@gmail.com

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  • Fix creating already pruned tenants in Receiver..

Verification

Verified with unit tests and some manual local testing.

@fpetkovski fpetkovski force-pushed the tenant-metrics-registration branch 2 times, most recently from 3aea6b9 to 0a65192 Compare August 29, 2022 14:54
@pull-request-size pull-request-size bot added size/M and removed size/S labels Aug 29, 2022
When a tenant is pruned from a Receiver, its metrics will remain in
the shipper's Prometheus registry. This leads to a panic when the same
tenant is created again in the Receiver since multiTSDB will attempt to
the same metrics again.

The current solution for the TSDB metrics is to wrap the registry in
an UnRegisterer which can deregister metrics with the same name before
registering new metrics. This commit applies the same solution to the
shipper metrics.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
@@ -446,7 +448,7 @@ func (t *MultiTSDB) startTSDB(logger log.Logger, tenantID string, tenant *tenant
s, err := tsdb.Open(
dataDir,
logger,
&UnRegisterer{Registerer: reg},
reg,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue was happening because we did not have an UnRegisterer here

Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

💪 nice find!

@GiedriusS GiedriusS enabled auto-merge (squash) August 29, 2022 15:11
@GiedriusS GiedriusS merged commit 06e5c94 into thanos-io:main Aug 29, 2022
prajain12 pushed a commit to prajain12/thanos that referenced this pull request Sep 6, 2022
When a tenant is pruned from a Receiver, its metrics will remain in
the shipper's Prometheus registry. This leads to a panic when the same
tenant is created again in the Receiver since multiTSDB will attempt to
the same metrics again.

The current solution for the TSDB metrics is to wrap the registry in
an UnRegisterer which can deregister metrics with the same name before
registering new metrics. This commit applies the same solution to the
shipper metrics.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Prakul Jain <prakul.jain@udaan.com>
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.

Receiver crashes occasionally with panic
2 participants