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

Change memcached client default timeout setting #1000

Merged
merged 4 commits into from
Feb 2, 2022

Conversation

ortuman
Copy link
Contributor

@ortuman ortuman commented Feb 2, 2022

What this PR does:

  • Change default *.memcached.timeout to 200ms

Which issue(s) this PR fixes:

Fixes grafana/mimir-squad#495

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM! Could you rebase, please? I just merged another PR touching config.

@ortuman ortuman force-pushed the ortuman/change-memcached-default-timeout branch from b4f533d to 84c641c Compare February 2, 2022 09:35
@ortuman ortuman force-pushed the ortuman/change-memcached-default-timeout branch from 84c641c to 2463ca3 Compare February 2, 2022 09:38
@ortuman ortuman marked this pull request as ready for review February 2, 2022 09:45
CHANGELOG.md Outdated Show resolved Hide resolved
@@ -242,6 +242,11 @@
* `-blocks-storage.bucket-store.index-cache.memcached.max-idle-connections`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed the indentation of the sublists above is wrong. Can you fix them too, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That must be very subtle.

Maybe my eye is betraying me, but after reviewing, that list seems to have the same indentation level as the rest. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be some rendering issue on GitHub, when checking out the code it looks the same as the others to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be. I'm going to merge this PR and I will double check it, just in case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I can't see anything wrong locally and it also renders fine here:
https://github.com/grafana/mimir/blob/main/CHANGELOG.md

@pracucci pracucci merged commit af1f249 into main Feb 2, 2022
@pracucci pracucci deleted the ortuman/change-memcached-default-timeout branch February 2, 2022 15:15
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.

3 participants