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

proposal: the possibility of integrating fsnotify to update index internal cache #545

Open
scbizu opened this issue Feb 5, 2022 · 5 comments
Labels
proposal which still in discussion

Comments

@scbizu
Copy link
Contributor

scbizu commented Feb 5, 2022

Background

In v0.13.0 release , we introduce the cache ticker to update the internal index . However , per #148 , if we deploy multiple replicas without external cache (redis cluster) , and upload a chart to replica A , the replica B's cache index will not recognize the chart , and then result in cache missing until the ticker reached . Even worser , the replica B will never update if user not open up the cache ticker option .

Proposal

I read through the comments in #148 , most of our users deploy CM with the same backend storage . Thus , multiple replicas(index cache) to one storage backend storage path .

With fsnotify , it allow us to register a watcher to fs , so it seems that we can register a index file watcher and listen the file operation event , emit a cache action event when fs event reaches rather than emit the event in post http request handler.

With above setup , I think we can achieve the HA goal which community expects.

The cache ticker should be kept for those storage providers who do not support fs event .

Behavior changes

  • local fs (in chartmuseum/storage) will impl a new interface(wraps the old storage interface) to integrate with the new fsnotify mechanism.
  • The backend storage 's (cache) event emitters in pkg/server/multitenant/handlers.go and pkg/server/multitenant/api.go will be removed if it impls the new interface , for those storage do not impl the new interface , the emitter will be kept (as the fallback case).
@scbizu scbizu added the proposal which still in discussion label Feb 5, 2022
@scbizu
Copy link
Contributor Author

scbizu commented Feb 5, 2022

/cc @cbuto @jdolitsky

What do you think ?

@cbuto
Copy link
Contributor

cbuto commented Feb 7, 2022

@scbizu this looks interesting! Will this be able to handle a multitenancy setup as well? I'm not sure if its related to handling depth/dynamic depth but I saw this issue fsnotify/fsnotify#18 👀.

@scbizu
Copy link
Contributor Author

scbizu commented Feb 8, 2022

@cbuto good catchup , the issue really will block us with local fs storage's dynamic depth , but maybe we can have some workaround with this 🤔 , for example registering the watcher when the tenant's first chart coming .

@cbuto
Copy link
Contributor

cbuto commented Feb 9, 2022

@scbizu that sounds like it could work!

scbizu added a commit to chartmuseum/storage that referenced this issue Mar 12, 2022
implements the helm/chartmuseum#545

Signed-off-by: scnace <scbizu@gmail.com>
scbizu added a commit that referenced this issue Mar 12, 2022
Closes #545

Signed-off-by: scnace <scbizu@gmail.com>
@scbizu
Copy link
Contributor Author

scbizu commented Mar 12, 2022

@cbuto I write a prototype , looks a little weird 😰

scbizu added a commit to chartmuseum/storage that referenced this issue Apr 9, 2022
implements the helm/chartmuseum#545

Signed-off-by: scnace <scbizu@gmail.com>
scbizu added a commit to chartmuseum/storage that referenced this issue Apr 9, 2022
implements the helm/chartmuseum#545

Signed-off-by: scnace <scbizu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal which still in discussion
Projects
None yet
Development

No branches or pull requests

2 participants