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

scbizu commented 2 years ago

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 commented 2 years ago

/cc @cbuto @jdolitsky

What do you think ?

cbuto commented 2 years ago

@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 https://github.com/fsnotify/fsnotify/issues/18 👀.

scbizu commented 2 years ago

@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 commented 2 years ago

@scbizu that sounds like it could work!

scbizu commented 2 years ago

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

helm / chartmuseum