Remove whole stream deletion mode #6435

MichelHollands · 2022-06-20T11:51:07Z

What this PR does / why we need it:

The whole stream deletion mode is no longer required as the filter-and-delete mode can do the same and more.

Checklist

Documentation added
Tests updated
Is this an important fix or new feature? Add an entry in the CHANGELOG.md.
Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

grafanabot · 2022-06-20T14:42:24Z

./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%
+        distributor	0.3%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-06-21T07:39:06Z

./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%
-        distributor	-0.3%
+            querier	0%
- querier/queryrange	-0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-06-21T08:10:57Z

./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%
-        distributor	-0.3%
+            querier	0%
- querier/queryrange	-0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
-               loki	-0.6%

grafanabot · 2022-06-21T14:39:28Z

./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%
+        distributor	0%
+            querier	0%
- querier/queryrange	-0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-06-21T14:40:58Z

./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%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

MasslessParticle

This looks good. I think things can be simplified by replacing the deletion mode switch statements with if statements.

pkg/loki/modules.go

pkg/storage/stores/shipper/compactor/compactor.go

pkg/storage/stores/shipper/compactor/deletion/mode_test.go

grafanabot · 2022-06-21T17:01:03Z

./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%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0.1%

Signed-off-by: Michel Hollands <[email protected]>

grafanabot · 2022-06-22T11:44:04Z

./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%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0.1%

MasslessParticle · 2022-06-22T16:15:59Z

LGTM

KMiller-Grafana

Docs look good to me.

grafanabot · 2022-06-23T11:24:23Z

The backport to k102 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-6435-to-k102 origin/k102
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x f80e487dcf106a6c4ea0a82d8afe7b0104addbc3
# Push it to GitHub
git push --set-upstream origin backport-6435-to-k102
git switch main
# Remove the local backport branch
git branch -D backport-6435-to-k102

Then, create a pull request where the base branch is k102 and the compare/head branch is backport-6435-to-k102.

* Remove whole-stream-deletion mode Signed-off-by: Michel Hollands <[email protected]> * Remove whole-stream-deletion from docs Signed-off-by: Michel Hollands <[email protected]> * Update the changelog Signed-off-by: Michel Hollands <[email protected]> * Sort changelog entries Signed-off-by: Michel Hollands <[email protected]> * Remove link to wrong configuration Signed-off-by: Michel Hollands <[email protected]> * Fix integration test Signed-off-by: Michel Hollands <[email protected]> * Set default deletion mode to disabled Signed-off-by: Michel Hollands <[email protected]> * Remove extra white line in documentation Signed-off-by: Michel Hollands <[email protected]> * Fix default value in docs Signed-off-by: Michel Hollands <[email protected]> * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Add DeletionEnabled method on mode Signed-off-by: Michel Hollands <[email protected]> * Rename test Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit f80e487)

* Remove whole stream deletion mode (#6435) * Remove whole-stream-deletion mode Signed-off-by: Michel Hollands <[email protected]> * Remove whole-stream-deletion from docs Signed-off-by: Michel Hollands <[email protected]> * Update the changelog Signed-off-by: Michel Hollands <[email protected]> * Sort changelog entries Signed-off-by: Michel Hollands <[email protected]> * Remove link to wrong configuration Signed-off-by: Michel Hollands <[email protected]> * Fix integration test Signed-off-by: Michel Hollands <[email protected]> * Set default deletion mode to disabled Signed-off-by: Michel Hollands <[email protected]> * Remove extra white line in documentation Signed-off-by: Michel Hollands <[email protected]> * Fix default value in docs Signed-off-by: Michel Hollands <[email protected]> * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Add DeletionEnabled method on mode Signed-off-by: Michel Hollands <[email protected]> * Rename test Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit f80e487) * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Fix changelog link Signed-off-by: Michel Hollands <[email protected]>

* Remove whole stream deletion mode (#6435) * Remove whole-stream-deletion mode Signed-off-by: Michel Hollands <[email protected]> * Remove whole-stream-deletion from docs Signed-off-by: Michel Hollands <[email protected]> * Update the changelog Signed-off-by: Michel Hollands <[email protected]> * Sort changelog entries Signed-off-by: Michel Hollands <[email protected]> * Remove link to wrong configuration Signed-off-by: Michel Hollands <[email protected]> * Fix integration test Signed-off-by: Michel Hollands <[email protected]> * Set default deletion mode to disabled Signed-off-by: Michel Hollands <[email protected]> * Remove extra white line in documentation Signed-off-by: Michel Hollands <[email protected]> * Fix default value in docs Signed-off-by: Michel Hollands <[email protected]> * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Add DeletionEnabled method on mode Signed-off-by: Michel Hollands <[email protected]> * Rename test Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit f80e487) * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Fix changelog link Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit da3fbd9)

) * Remove whole stream deletion mode (#6435) * Remove whole-stream-deletion mode Signed-off-by: Michel Hollands <[email protected]> * Remove whole-stream-deletion from docs Signed-off-by: Michel Hollands <[email protected]> * Update the changelog Signed-off-by: Michel Hollands <[email protected]> * Sort changelog entries Signed-off-by: Michel Hollands <[email protected]> * Remove link to wrong configuration Signed-off-by: Michel Hollands <[email protected]> * Fix integration test Signed-off-by: Michel Hollands <[email protected]> * Set default deletion mode to disabled Signed-off-by: Michel Hollands <[email protected]> * Remove extra white line in documentation Signed-off-by: Michel Hollands <[email protected]> * Fix default value in docs Signed-off-by: Michel Hollands <[email protected]> * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Add DeletionEnabled method on mode Signed-off-by: Michel Hollands <[email protected]> * Rename test Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit f80e487) * Fix changelog Signed-off-by: Michel Hollands <[email protected]> * Fix changelog link Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit da3fbd9) Co-authored-by: Michel Hollands <[email protected]>

pull-request-size bot added the size/M label Jun 20, 2022

github-actions bot added the area/docs label Jun 20, 2022

MichelHollands force-pushed the remove_whole_stream_deletion_mode branch from c9839c3 to 6edecdc Compare June 20, 2022 11:58

MichelHollands marked this pull request as ready for review June 20, 2022 14:50

MichelHollands requested review from KMiller-Grafana and a team as code owners June 20, 2022 14:50

MichelHollands closed this Jun 21, 2022

MichelHollands reopened this Jun 21, 2022

MichelHollands requested a review from MasslessParticle June 21, 2022 14:33

MichelHollands force-pushed the remove_whole_stream_deletion_mode branch from 5a6bcdb to 7626deb Compare June 21, 2022 14:34

MasslessParticle requested changes Jun 21, 2022

View reviewed changes

pkg/loki/modules.go Outdated Show resolved Hide resolved

pkg/storage/stores/shipper/compactor/compactor.go Outdated Show resolved Hide resolved

pkg/storage/stores/shipper/compactor/deletion/mode_test.go Outdated Show resolved Hide resolved

pull-request-size bot added size/L and removed size/M labels Jun 21, 2022

MichelHollands added 11 commits June 22, 2022 12:38

Remove whole-stream-deletion mode

7313494

Signed-off-by: Michel Hollands <[email protected]>

Remove whole-stream-deletion from docs

f4dbf78

Signed-off-by: Michel Hollands <[email protected]>

Update the changelog

9c71c28

Signed-off-by: Michel Hollands <[email protected]>

Sort changelog entries

6a9a7be

Signed-off-by: Michel Hollands <[email protected]>

Remove link to wrong configuration

1c845c8

Signed-off-by: Michel Hollands <[email protected]>

Fix integration test

bd8ae6e

Signed-off-by: Michel Hollands <[email protected]>

Set default deletion mode to disabled

a326b81

Signed-off-by: Michel Hollands <[email protected]>

Remove extra white line in documentation

a2e38ad

Signed-off-by: Michel Hollands <[email protected]>

Fix default value in docs

3a7d38c

Signed-off-by: Michel Hollands <[email protected]>

Fix changelog

4f02c37

Signed-off-by: Michel Hollands <[email protected]>

Add DeletionEnabled method on mode

100fc9d

Signed-off-by: Michel Hollands <[email protected]>

Rename test

1610706

Signed-off-by: Michel Hollands <[email protected]>

MichelHollands force-pushed the remove_whole_stream_deletion_mode branch from c85169b to 1610706 Compare June 22, 2022 11:38

MichelHollands requested a review from MasslessParticle June 22, 2022 11:58

MasslessParticle approved these changes Jun 22, 2022

View reviewed changes

MasslessParticle merged commit f80e487 into grafana:main Jun 22, 2022

KMiller-Grafana reviewed Jun 22, 2022

View reviewed changes

MichelHollands added the backport k102 label Jun 23, 2022

This was referenced Jun 28, 2022

dannykopping/remove cache stats dannykopping/loki#13

Closed

dannykopping/remove cache stats dannykopping/loki#14

Draft

Remove cache statistics PR from release-2.6.x #6525

Merged

osg-grafana added type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories and removed area/docs labels Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove whole stream deletion mode #6435

Remove whole stream deletion mode #6435

MichelHollands commented Jun 20, 2022 •

edited

Loading

grafanabot commented Jun 20, 2022

grafanabot commented Jun 21, 2022

grafanabot commented Jun 21, 2022

grafanabot commented Jun 21, 2022

grafanabot commented Jun 21, 2022

MasslessParticle left a comment

grafanabot commented Jun 21, 2022

grafanabot commented Jun 22, 2022

MasslessParticle commented Jun 22, 2022

KMiller-Grafana left a comment

grafanabot commented Jun 23, 2022

Remove whole stream deletion mode #6435

Remove whole stream deletion mode #6435

Conversation

MichelHollands commented Jun 20, 2022 • edited Loading

grafanabot commented Jun 20, 2022

grafanabot commented Jun 21, 2022

grafanabot commented Jun 21, 2022

grafanabot commented Jun 21, 2022

grafanabot commented Jun 21, 2022

MasslessParticle left a comment

Choose a reason for hiding this comment

grafanabot commented Jun 21, 2022

grafanabot commented Jun 22, 2022

MasslessParticle commented Jun 22, 2022

KMiller-Grafana left a comment

Choose a reason for hiding this comment

grafanabot commented Jun 23, 2022

MichelHollands commented Jun 20, 2022 •

edited

Loading