From be196a3e55ff34d90dd47971cf80898de0f299f5 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Wed, 31 Jan 2024 16:05:04 +0100 Subject: [PATCH] lazy initialize public share manager Signed-off-by: jkoberg --- .../lazy-initialize-publicsharemanager.md | 6 +++ pkg/publicshare/manager/json/json.go | 53 ++++++++++++++----- .../manager/json/persistence/file/file.go | 23 +++++++- 3 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 changelog/unreleased/lazy-initialize-publicsharemanager.md diff --git a/changelog/unreleased/lazy-initialize-publicsharemanager.md b/changelog/unreleased/lazy-initialize-publicsharemanager.md new file mode 100644 index 0000000000..693c8e9c0b --- /dev/null +++ b/changelog/unreleased/lazy-initialize-publicsharemanager.md @@ -0,0 +1,6 @@ +Enhancement: Lazy initialize public share manager + +Unlike the share manager the public share manager was initializing its data structure on startup. This can lead to failed ocis +starts (in single binary case) or to restarting `sharing` pods when running in containerized environment. + +https://github.com/cs3org/reva/pull/4490 diff --git a/pkg/publicshare/manager/json/json.go b/pkg/publicshare/manager/json/json.go index 7e809f5528..1d2b44265e 100644 --- a/pkg/publicshare/manager/json/json.go +++ b/pkg/publicshare/manager/json/json.go @@ -72,10 +72,6 @@ func NewFile(c map[string]interface{}) (publicshare.Manager, error) { } p := file.New(conf.File) - if err := p.Init(context.Background()); err != nil { - return nil, err - } - return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p) } @@ -89,10 +85,6 @@ func NewMemory(c map[string]interface{}) (publicshare.Manager, error) { conf.init() p := memory.New() - if err := p.Init(context.Background()); err != nil { - return nil, err - } - return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p) } @@ -111,10 +103,6 @@ func NewCS3(c map[string]interface{}) (publicshare.Manager, error) { } p := cs3.New(s) - if err := p.Init(context.Background()); err != nil { - return nil, err - } - return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p) } @@ -174,6 +162,10 @@ type manager struct { enableExpiredSharesCleanup bool } +func (m *manager) init() error { + return m.persistence.Init(context.Background()) +} + func (m *manager) startJanitorRun() { if !m.enableExpiredSharesCleanup { return @@ -200,6 +192,10 @@ func (m *manager) Dump(ctx context.Context, shareChan chan<- *publicshare.WithPa m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return err + } + db, err := m.persistence.Read(ctx) if err != nil { return err @@ -222,6 +218,10 @@ func (m *manager) Load(ctx context.Context, shareChan <-chan *publicshare.WithPa m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return err + } + db, err := m.persistence.Read(ctx) if err != nil { return err @@ -296,6 +296,10 @@ func (m *manager) CreatePublicShare(ctx context.Context, u *user.User, rInfo *pr m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return nil, err + } + encShare, err := utils.MarshalProtoV1ToJSON(&ps.PublicShare) if err != nil { return nil, err @@ -385,6 +389,10 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return nil, err + } + db, err := m.persistence.Read(ctx) if err != nil { return nil, err @@ -420,6 +428,10 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return nil, err + } + if ref.GetToken() != "" { ps, pw, err := m.getByToken(ctx, ref.GetToken()) if err != nil { @@ -473,6 +485,10 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters [] m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return nil, err + } + log := appctx.GetLogger(ctx) db, err := m.persistence.Read(ctx) @@ -555,6 +571,10 @@ func (m *manager) cleanupExpiredShares() { m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return + } + db, _ := m.persistence.Read(context.Background()) for _, v := range db { @@ -594,6 +614,11 @@ func (m *manager) revokeExpiredPublicShare(ctx context.Context, s *link.PublicSh func (m *manager) RevokePublicShare(ctx context.Context, _ *user.User, ref *link.PublicShareReference) error { m.mutex.Lock() defer m.mutex.Unlock() + + if err := m.init(); err != nil { + return err + } + return m.revokePublicShare(ctx, ref) } @@ -651,6 +676,10 @@ func (m *manager) GetPublicShareByToken(ctx context.Context, token string, auth m.mutex.Lock() defer m.mutex.Unlock() + if err := m.init(); err != nil { + return nil, err + } + db, err := m.persistence.Read(ctx) if err != nil { return nil, err diff --git a/pkg/publicshare/manager/json/persistence/file/file.go b/pkg/publicshare/manager/json/persistence/file/file.go index d9a84d6742..e813b95dc8 100644 --- a/pkg/publicshare/manager/json/persistence/file/file.go +++ b/pkg/publicshare/manager/json/persistence/file/file.go @@ -24,6 +24,7 @@ import ( "fmt" "os" "path/filepath" + "sync" "github.com/cs3org/reva/v2/pkg/publicshare/manager/json/persistence" ) @@ -31,16 +32,28 @@ import ( type file struct { path string initialized bool + lock *sync.RWMutex } // New returns a new Cache instance func New(path string) persistence.Persistence { return &file{ path: path, + lock: &sync.RWMutex{}, } } func (p *file) Init(_ context.Context) error { + if p.isInitialized() { + return nil + } + + p.lock.Lock() + defer p.lock.Unlock() + if p.initialized { + return nil + } + // attempt to create the db file var fi os.FileInfo var err error @@ -65,7 +78,7 @@ func (p *file) Init(_ context.Context) error { } func (p *file) Read(_ context.Context) (persistence.PublicShares, error) { - if !p.initialized { + if !p.isInitialized() { return nil, fmt.Errorf("not initialized") } db := map[string]interface{}{} @@ -80,7 +93,7 @@ func (p *file) Read(_ context.Context) (persistence.PublicShares, error) { } func (p *file) Write(_ context.Context, db persistence.PublicShares) error { - if !p.initialized { + if !p.isInitialized() { return fmt.Errorf("not initialized") } dbAsJSON, err := json.Marshal(db) @@ -90,3 +103,9 @@ func (p *file) Write(_ context.Context, db persistence.PublicShares) error { return os.WriteFile(p.path, dbAsJSON, 0644) } + +func (p *file) isInitialized() bool { + p.lock.RLock() + defer p.lock.RUnlock() + return p.initialized +}