Skip to content

Nuke some old deprecated flags #1330

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

Merged
merged 3 commits into from
May 2, 2019
Merged

Conversation

gouthamve
Copy link
Contributor

We have some deprecated flags in our code base for long and I think it's time we cleaned up some of them :)

Deleted the useless flags (they do nothing even if set):

bigtable.max-recv-msg-size
ingester.client.max-recv-message-size
ingester.client.compress-to-ingester
distributor.compress-to-ingester
ingester.search-pending-for
ingester.reject-old-samples
ingester.reject-old-samples.max-age
ingester.validation.max-length-label-name
ingester.validation.max-length-label-value
ingester.max-label-names-per-series

Other flags (use -store.index-cache-read.*):

store.index-cache-size
index.memcached.*

Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
@bboreham
Copy link
Contributor

ingester.search-pending-for was only removed a month ago.

Upon checking, I see we have two more of these in our config; silently removing options is a really bad idea and is not what "deprecated" means.

@gouthamve
Copy link
Contributor Author

I'd like to disagree here. Having the flags set causes the operator to think that they still work when they don't. I prefer breaking things in this case so we can fix them. Case in point which drove me to do this (in our ksonnet):

-      'ingester.search-pending-for': '5m',
+      'ingester.max-transfer-retries': 60,  // Each retry is backed off by 5s, so 5mins for new ingester to come up.

Before we used to wait for 5m and now we wait less than 50s (before I made the changes above, a month after I should have).

Changing the behavior like above is a breaking change and making them explicit is much better than implicit.

Signed-off-by: Goutham Veeramachaneni <[email protected]>
@bboreham
Copy link
Contributor

bboreham commented Apr 15, 2019

A warning message is better than silent.
The radical change in wait time is a bug #1307

I don't think you are disagreeing with me: I am arguing that silently ignoring options is the worst thing to do.

@gouthamve gouthamve merged commit e1b192b into cortexproject:master May 2, 2019
@gouthamve gouthamve deleted the nuke-flags branch May 2, 2019 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants