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

[WIP] Index cache clients : Dedup Index Cache requests #7324

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Vanshikav123
Copy link
Contributor

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

Changes

#7169
This change makes sure that if several requests ask for the same information at almost the same time, the system handles them together instead of repeating the same task multiple times unnecessarily.

Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
@pull-request-size pull-request-size bot added size/L and removed size/M labels May 2, 2024
@Vanshikav123
Copy link
Contributor Author

Vanshikav123 commented May 2, 2024

@yeya24 @GiedriusS @MichaHoffmann can you PTAL , am i heading towards correct approach?

@@ -456,6 +462,43 @@ func (c *memcachedClient) GetMulti(ctx context.Context, keys []string) map[strin
return hits
}

func (c *memcachedClient) GrabKeys(ctx context.Context, keys []string) map[string][]byte {
var group singleflight.Group
Copy link
Member

Choose a reason for hiding this comment

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

You need to create this group only once, inside the memcachedClient.

// RemoteCacheClient is a high level client to interact with remote cache.
type RemoteCacheClient interface {
// ReadThroughRemoteCache is a high level client to interact with remote cache.
type ReadThroughRemoteCache interface {
Copy link
Member

@GiedriusS GiedriusS May 9, 2024

Choose a reason for hiding this comment

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

Read-through means that we should start our work from refactoring the internal interfaces so that the order of operations wouldn't be (this is the state right now):

  • Get from cache
  • If not in cache, fetch the actual data
  • Put it back in cache

It must be:

  • GetSomeData() would be called consistently inside Thanos and it would do the following only once:
    • Get from cache
    • If not in cache, fetch the actual data
    • Put it back in cache

So, instead of each action being separate, you would do all of this only once. Does it make sense? :) I would suggest starting from one data type only e.g. postings and then work from there.

@Vanshikav123 Vanshikav123 changed the title Index cache clients : Dedup Index Cache requests [WIP] Index cache clients : Dedup Index Cache requests May 13, 2024
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.

None yet

2 participants