From d76e4c409b23dbddca3687f08b2b994f313e002d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 21 Feb 2024 08:26:24 +0100 Subject: [PATCH] jsoncs3 cache fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/jsoncs3-cache-fixes.md | 5 +++++ .../manager/jsoncs3/providercache/providercache.go | 10 +++++++--- .../jsoncs3/receivedsharecache/receivedsharecache.go | 9 ++++++++- pkg/share/manager/jsoncs3/sharecache/sharecache.go | 7 +++++++ 4 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/jsoncs3-cache-fixes.md diff --git a/changelog/unreleased/jsoncs3-cache-fixes.md b/changelog/unreleased/jsoncs3-cache-fixes.md new file mode 100644 index 0000000000..aa2537c509 --- /dev/null +++ b/changelog/unreleased/jsoncs3-cache-fixes.md @@ -0,0 +1,5 @@ +Bugfix: jsoncs3 cache fixes + +The jsoncs3 share manager now retries persisting if the file already existed and picks up the etag of the upload response in all cases. + +https://github.com/cs3org/reva/pull/4532 diff --git a/pkg/share/manager/jsoncs3/providercache/providercache.go b/pkg/share/manager/jsoncs3/providercache/providercache.go index c9c291f2c6..5286be783d 100644 --- a/pkg/share/manager/jsoncs3/providercache/providercache.go +++ b/pkg/share/manager/jsoncs3/providercache/providercache.go @@ -202,6 +202,11 @@ func (c *Cache) Add(ctx context.Context, storageID, spaceID, shareID string, sha log.Debug().Msg("precondition failed when persisting added provider share: etag changed. retrying...") // actually, this is the wrong status code and we treat it like errtypes.Aborted because of inconsistencies on the server side // continue with sync below + case errtypes.AlreadyExists: + log.Debug().Msg("already exists when persisting added provider share. retrying...") + // CS3 uses an already exists error instead of precondition failed when using an If-None-Match=* header / IfExists flag in the InitiateFileUpload call. + // Thas happens when the cache thinks there is no file. + // continue with sync below default: span.SetStatus(codes.Error, fmt.Sprintf("persisting added provider share failed. giving up: %s", err.Error())) log.Error().Err(err).Msg("persisting added provider share failed") @@ -393,10 +398,8 @@ func (c *Cache) Persist(ctx context.Context, storageID, spaceID string) error { // > If the field value is "*", the condition is false if the origin server has a current representation for the target resource. if space.Etag == "" { ur.IfNoneMatch = []string{"*"} - log.Debug().Msg("setting IfNoneMatch to *") - } else { - log.Debug().Msg("setting IfMatchEtag") } + res, err := c.storage.Upload(ctx, ur) if err != nil { span.RecordError(err) @@ -405,6 +408,7 @@ func (c *Cache) Persist(ctx context.Context, storageID, spaceID string) error { return err } space.Etag = res.Etag + span.SetStatus(codes.Ok, "") shares := []string{} for _, s := range space.Shares { diff --git a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go index 1080a808d1..8bb6bb6ad0 100644 --- a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go +++ b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go @@ -146,6 +146,11 @@ func (c *Cache) Add(ctx context.Context, userID, spaceID string, rs *collaborati log.Debug().Msg("precondition failed when persisting added received share: etag changed. retrying...") // actually, this is the wrong status code and we treat it like errtypes.Aborted because of inconsistencies on the server side // continue with sync below + case errtypes.AlreadyExists: + log.Debug().Msg("already exists when persisting added received share. retrying...") + // CS3 uses an already exists error instead of precondition failed when using an If-None-Match=* header / IfExists flag in the InitiateFileUpload call. + // Thas happens when the cache thinks there is no file. + // continue with sync below default: span.SetStatus(codes.Error, fmt.Sprintf("persisting added received share failed. giving up: %s", err.Error())) log.Error().Err(err).Msg("persisting added received share failed") @@ -294,12 +299,14 @@ func (c *Cache) persist(ctx context.Context, userID string) error { ur.IfNoneMatch = []string{"*"} } - _, err = c.storage.Upload(ctx, ur) + res, err := c.storage.Upload(ctx, ur) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return err } + rss.etag = res.Etag + span.SetStatus(codes.Ok, "") return nil } diff --git a/pkg/share/manager/jsoncs3/sharecache/sharecache.go b/pkg/share/manager/jsoncs3/sharecache/sharecache.go index d75061ae04..3d6fbe7037 100644 --- a/pkg/share/manager/jsoncs3/sharecache/sharecache.go +++ b/pkg/share/manager/jsoncs3/sharecache/sharecache.go @@ -137,6 +137,11 @@ func (c *Cache) Add(ctx context.Context, userid, shareID string) error { log.Debug().Msg("precondition failed when persisting added share: etag changed. retrying...") // actually, this is the wrong status code and we treat it like errtypes.Aborted because of inconsistencies on the server side // continue with sync below + case errtypes.AlreadyExists: + log.Debug().Msg("already exists when persisting added share. retrying...") + // CS3 uses an already exists error instead of precondition failed when using an If-None-Match=* header / IfExists flag in the InitiateFileUpload call. + // Thas happens when the cache thinks there is no file. + // continue with sync below default: span.SetStatus(codes.Error, fmt.Sprintf("persisting added share failed. giving up: %s", err.Error())) log.Error().Err(err).Msg("persisting added share failed") @@ -330,6 +335,7 @@ func (c *Cache) Persist(ctx context.Context, userid string) error { if us.Etag == "" { ur.IfNoneMatch = []string{"*"} } + res, err := c.storage.Upload(ctx, ur) if err != nil { span.RecordError(err) @@ -337,6 +343,7 @@ func (c *Cache) Persist(ctx context.Context, userid string) error { return err } us.Etag = res.Etag + span.SetStatus(codes.Ok, "") return nil }