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

[bloom-compactor] remove BloomCompactorMinTableAge check #11546

Merged
merged 6 commits into from
Jan 4, 2024

Conversation

poyzannur
Copy link
Contributor

What this PR does / why we need it:
BloomCompactorMinTableAge was added with the idea that we will skip most recent index files from ingesters that are not compacted into TSDB indexes yet.

The default value is an hour. This blocks bloom-compacter processing TSBD of the day, because end timestamp is always in the last 15 mins. We can either reduce it something lower than the frequency of TSDB indexes being built (< 15mins).
Here I choose to remove it altogether, assuming uncompacted indexes from ingesters will not be processed as a table as there won't be a schema for that table with the check here.

@poyzannur poyzannur requested a review from a team as a code owner December 21, 2023 16:08
@poyzannur poyzannur changed the title Poyzannur/remove min table age check [bloom-compactor] remove BloomCompactorMinTableAge check Dec 21, 2023
Copy link
Contributor

github-actions bot commented Dec 21, 2023

Trivy scan found the following vulnerabilities:

  • HIGH, Target: docker.io/grafana/loki:main-ce57448 (alpine 3.18.4), Type: alpine openssl: Incorrect cipher key and IV length processing in libcrypto3 v3.1.3-r0. Fixed in v3.1.4-r0
  • HIGH, Target: docker.io/grafana/loki:main-ce57448 (alpine 3.18.4), Type: alpine openssl: Incorrect cipher key and IV length processing in libssl3 v3.1.3-r0. Fixed in v3.1.4-r0
    \nTo see more details on these vulnerabilities, and how/where to fix them, please run docker build -t grafana/loki:main-ce57448 -f cmd/loki/Dockerfile .
    trivy i grafana/loki:main-ce57448 on your branch. If these were not introduced by your PR, please considering fixing them in via a subsequent PR. Thanks!

@@ -307,12 +305,7 @@ func (c *Compactor) compactUsers(ctx context.Context, logger log.Logger, sc stor

// Skip this table if it is too new/old for the tenant limits.
now := model.Now()
tableMinAge := c.limits.BloomCompactorMinTableAge(tenant)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also remove per-tenant setting as it is not used any more.

@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Dec 22, 2023
@poyzannur poyzannur merged commit 599eed7 into main Jan 4, 2024
9 checks passed
@poyzannur poyzannur deleted the poyzannur/remove-minTableAgeCheck branch January 4, 2024 18:40
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
**What this PR does / why we need it**:
`BloomCompactorMinTableAge` was added with the idea that we will skip
most recent index files from ingesters that are not compacted into TSDB
indexes yet.

The default value is an hour. This blocks bloom-compacter processing
TSBD of the day, because end timestamp is always in the last 15 mins. We
can either reduce it something lower than the frequency of TSDB indexes
being built (< 15mins).
Here I choose to remove it altogether, assuming uncompacted indexes from
ingesters will not be processed as a table as there won't be a schema
for that table with the [check
here](https://github.com/grafana/loki/blob/main/pkg/bloomcompactor/bloomcompactor.go#L268-L272).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants