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

Deprecate CLI flag -ruler.wal-cleaer.period in favor of CLI flag -ruler.wal-cleaner.period #7937

Merged
merged 3 commits into from
Dec 14, 2022

Conversation

ssncferreira
Copy link
Contributor

@ssncferreira ssncferreira commented Dec 14, 2022

What this PR does / why we need it:

Deprecate CLI flag -ruler.wal-cleaer.period in favor of CLI flag -ruler.wal-cleaner.period.

Which issue(s) this PR fixes:
Fixes #7916 (comment) and #7916 (comment)

NOTE: correctly registring the ruler.wal-cleaner configuration flags is not a breaking change and doesn't require an upgrade guide, because the ruler WAL cleaner initialization code already guarantees that the default values are used:

func NewWALCleaner(logger log.Logger, manager instance.Manager, metrics *Metrics, walDirectory string, cfg Config) *WALCleaner {
c := &WALCleaner{
logger: log.With(logger, "component", "cleaner"),
instanceManager: manager,
walDirectory: filepath.Clean(walDirectory),
walLastModified: lastModified,
minAge: DefaultCleanupAge,
period: DefaultCleanupPeriod,
done: make(chan bool),
metrics: metrics,
}
if cfg.MinAge > 0 {
c.minAge = cfg.MinAge
}
// We allow a period of 0 here because '0' means "don't run the task". This
// is handled by not running a ticker at all in the run method.
if cfg.Period >= 0 {
c.period = cfg.Period
}
go c.run()
return c
}

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@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 14, 2022
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-target-branch/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%

@ssncferreira ssncferreira marked this pull request as ready for review December 14, 2022 15:03
@ssncferreira ssncferreira requested a review from a team as a code owner December 14, 2022 15:03
Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-target-branch/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%

@ssncferreira ssncferreira merged commit e07fe7c into main Dec 14, 2022
@ssncferreira ssncferreira deleted the fix_ruler_wal_cleaner_cli_flag branch December 14, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M 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