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

[Remote Store] Simplify RemoteStoreRefresh Listener #11908

Open
gbbafna opened this issue Jan 17, 2024 · 0 comments
Open

[Remote Store] Simplify RemoteStoreRefresh Listener #11908

gbbafna opened this issue Jan 17, 2024 · 0 comments
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request Storage:Remote

Comments

@gbbafna
Copy link
Collaborator

gbbafna commented Jan 17, 2024

Is your feature request related to a problem? Please describe

RemoteStoreRefresh Listener is too complex in two aspects :

  1. When to upload - Plethora of if conditions, which are hard to understand/debug.
  2. Retry methodology - When upload fails, we retry the upload . Refreshes and Retries can happen concurrently , hence many code paths can run concurrently as well . For ex : [Remote Store] Root cause deleted segment files during remote uploads  #11025

Describe the solution you'd like

  1. May be we can use isRemoteStoreInSync as a trigger for refresh . The only catch is then we would need a higher bound on refresh interval , say 60 sec .
  2. [Preferred] Trigger refresh again when the upload fails . This would fare better for replication lag and wouldn't force an upper bound on refresh interval .

Explore more possible solutions .

Related component

Storage:Remote

Describe alternatives you've considered

None

Additional context

Refer #11720 for IndexShard#isRemoteStoreInSync

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Storage:Remote
Projects
Status: Next (Next Quarter)
Development

No branches or pull requests

2 participants