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

cache: add timeout for groupcache's fetch operation #5206

Merged
merged 5 commits into from
Mar 3, 2022

Conversation

GiedriusS
Copy link
Member

@GiedriusS GiedriusS commented Mar 3, 2022

Add a timeout for groupcache's fetch operation. It is useful when there
are network errors - if loading from a peer fails then we still might
have a chance to be able to load data from remote object storage
ourselves.

Signed-off-by: Giedrius Statkevičius giedrius.statkevicius@vinted.com

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Added creation of a new context with the specified deadline if it is not zero.

Verification

N/A

Add a timeout for groupcache's fetch operation. It is useful when there
are network errors - if loading from a peer fails then we still might
have a chance to be able to load data from remote object storage
ourselves.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
pkg/cache/groupcache.go Outdated Show resolved Hide resolved
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
matej-g
matej-g previously approved these changes Mar 3, 2022
@@ -441,6 +442,8 @@ In the `peers` section it is possible to use the prefix form to automatically lo

Note that there must be no trailing slash in the `peers` configuration i.e. one of the strings must be identical to `self_url` and others should have the same form. Without this, loading data from peers may fail.

If timeout is set to zero then there is no timeout for fetching and fetching's lifetime is equal to the lifetime to the original request's lifetime. It is recommended to keep it more than zero. It is generally preferred to keep this value higher because the fetching operation potentially includes loading that data from remote object storage.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nits:

Suggested change
If timeout is set to zero then there is no timeout for fetching and fetching's lifetime is equal to the lifetime to the original request's lifetime. It is recommended to keep it more than zero. It is generally preferred to keep this value higher because the fetching operation potentially includes loading that data from remote object storage.
If timeout is set to zero then there is no timeout for fetching and fetching's lifetime is equal to the lifetime to the original request's lifetime. It is recommended to keep it higher than zero. It is generally preferred to keep this value higher because the fetching operation potentially includes loading of data from remote object storage.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
@GiedriusS GiedriusS enabled auto-merge (squash) March 3, 2022 10:01
@GiedriusS GiedriusS merged commit 4955c01 into thanos-io:main Mar 3, 2022
@GiedriusS GiedriusS deleted the feature/add_groupcache_timeout branch March 3, 2022 10:10
GiedriusS added a commit to vinted/thanos that referenced this pull request May 11, 2022
* cache: add timeout for groupcache's fetch operation

Add a timeout for groupcache's fetch operation. It is useful when there
are network errors - if loading from a peer fails then we still might
have a chance to be able to load data from remote object storage
ourselves.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: add entry

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* cache: add yaml tag for new field

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* cache: bump default timeout, improve docs

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* docs: make changes according to Matej's suggestions

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants