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

Backport 7804 into 2.7.1 #7896

Conversation

trevorwhitney
Copy link
Collaborator

@trevorwhitney trevorwhitney commented Dec 9, 2022

use grpc for communicating with compactor for query time filtering of data requested for deletion (#7804)

Manual backport of #7804 and #7814

What this PR does / why we need it:
Add grpc support to compactor for getting delete requests and gen number for query time filtering.
Since these requests are internal to Loki, it would be good to use grpc instead of HTTP same as all the internal requests we do in Loki.

I have added a new config for accepting the grpc address of the compactor. I tried having just the existing config and detecting if it is a grpc server, but it was hard to do it reliably, considering the different deployment modes we support. I think it is safe to keep it the same and eventually deprecate the existing config.

Checklist

  • Documentation added
  • Tests updated
  • CHANGELOG.md updated

(cherry picked from commit 1410808)

@trevorwhitney trevorwhitney requested a review from a team as a code owner December 9, 2022 17:48
@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 9, 2022
@trevorwhitney trevorwhitney changed the base branch from backport-7804-to-release-2.7.x to release-2.7.x December 9, 2022 17:52
sandeepsukhani and others added 3 commits December 9, 2022 11:17
… data requested for deletion (grafana#7804)

**What this PR does / why we need it**:
Add grpc support to compactor for getting delete requests and gen number
for query time filtering.
Since these requests are internal to Loki, it would be good to use grpc
instead of HTTP same as all the internal requests we do in Loki.

I have added a new config for accepting the grpc address of the
compactor. I tried having just the existing config and detecting if it
is a grpc server, but it was hard to do it reliably, considering the
different deployment modes we support. I think it is safe to keep it the
same and eventually deprecate the existing config.

**Checklist**
- [x] Documentation added
- [x] Tests updated
- [x] `CHANGELOG.md` updated

(cherry picked from commit 1410808)
**What this PR does / why we need it**:
I had enabled auto-merge in PR grafana#7804, but somehow it still merged the PR
without all the checks passing.
This PR fixes the failing lint and tests.
@trevorwhitney trevorwhitney force-pushed the backport-7804-to-release-2.7.x branch from 48514cf to a591ad7 Compare December 9, 2022 18:19
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

-           ingester	-0.5%
-        distributor	-0.8%
-            querier	-0.1%
- querier/queryrange	-1.1%
+               iter	0%
+            storage	0.2%
+           chunkenc	0%
+              logql	1.7%
+               loki	0.5%

@trevorwhitney trevorwhitney merged commit e0af1cc into grafana:release-2.7.x Dec 9, 2022
trevorwhitney added a commit to trevorwhitney/loki that referenced this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL 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.

4 participants