Merge pull request #4470 from butonic/backport/stable-2.16/4449

keep failed processing status

changelog/unreleased/keep-failed-processing-status.md

@@ -0,0 +1,6 @@
+Bugfix: Keep failed processing status
+
+We now keep the postprocessing status when a blob could not be copied to the blobstore.
+
+https://github.com/cs3org/reva/pull/4470
+https://github.com/cs3org/reva/pull/4449

go.mod

@@ -46,7 +46,7 @@ require (
 	github.com/jedib0t/go-pretty v4.3.0+incompatible
 	github.com/jellydator/ttlcache/v2 v2.11.1
 	github.com/juliangruber/go-intersect v1.1.0
-	github.com/mattn/go-sqlite3 v2.0.3+incompatible
+	github.com/mattn/go-sqlite3 v1.14.19
 	github.com/maxymania/go-system v0.0.0-20170110133659-647cc364bf0b
 	github.com/mileusna/useragent v1.2.1
 	github.com/minio/minio-go/v7 v7.0.42
@@ -224,3 +224,7 @@ require (
 )
 
 replace github.com/cs3org/go-cs3apis => github.com/2403905/go-cs3apis v0.0.0-20230517122726-727045414fd1
+
+// exclude the v2 line of go-sqlite3 which was released accidentally and prevents pulling in newer versions of go-sqlite3
+// see https://github.com/mattn/go-sqlite3/issues/965 for more details
+exclude github.com/mattn/go-sqlite3 v2.0.3+incompatible

go.sum

@@ -966,8 +966,8 @@ github.com/mattn/go-runewidth v0.0.6/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m
 github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
 github.com/mattn/go-runewidth v0.0.13 h1:lTGmDsbAYt5DmK6OnoV7EuIF1wEIFAcxld6ypU4OSgU=
 github.com/mattn/go-runewidth v0.0.13/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
-github.com/mattn/go-sqlite3 v2.0.3+incompatible h1:gXHsfypPkaMZrKbD5209QV9jbUTJKjyR5WD3HYQSd+U=
-github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
+github.com/mattn/go-sqlite3 v1.14.19 h1:fhGleo2h1p8tVChob4I9HpmVFIAkKGpiukdrgQbWfGI=
+github.com/mattn/go-sqlite3 v1.14.19/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg=
 github.com/mattn/go-tty v0.0.3 h1:5OfyWorkyO7xP52Mq7tB36ajHDG5OHrmBGIS/DtakQI=
 github.com/mattn/go-tty v0.0.3/go.mod h1:ihxohKRERHTVzN+aSVRwACLCeqIoZAWpoICkkvrWyR0=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -252,6 +252,7 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) {
 				failed     bool
 				keepUpload bool
 			)
+			unmarkPostprocessing := true
 
 			n, err := node.ReadNode(ctx, fs.lu, up.Info.Storage["SpaceRoot"], up.Info.Storage["NodeId"], false, nil, true)
 			if err != nil {
@@ -270,8 +271,10 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) {
 			case events.PPOutcomeContinue:
 				if err := up.Finalize(); err != nil {
 					log.Error().Err(err).Str("uploadID", ev.UploadID).Msg("could not finalize upload")
-					keepUpload = true // should we keep the upload when assembling failed?
 					failed = true
+					keepUpload = true
+					// keep postprocessing status so the upload is not deleted during housekeeping
+					unmarkPostprocessing = false
 				}
 			case events.PPOutcomeDelete:
 				failed = true
@@ -300,7 +303,7 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) {
 				}
 			}
 
-			upload.Cleanup(up, failed, keepUpload)
+			upload.Cleanup(up, failed, keepUpload, unmarkPostprocessing)
 
 			// remove cache entry in gateway
 			fs.cache.RemoveStatContext(ctx, ev.ExecutingUser.GetId(), &provider.ResourceId{SpaceId: n.SpaceID, OpaqueId: n.ID})

pkg/storage/utils/decomposedfs/upload/upload.go

@@ -132,15 +132,17 @@ func buildUpload(ctx context.Context, info tusd.FileInfo, binPath string, infoPa
 }
 
 // Cleanup cleans the upload
-func Cleanup(upload *Upload, failure bool, keepUpload bool) {
+func Cleanup(upload *Upload, revertNodeMetadata, keepUpload, unmarkPostprocessing bool) {
 	ctx, span := tracer.Start(upload.Ctx, "Cleanup")
 	defer span.End()
-	upload.cleanup(failure, !keepUpload, !keepUpload)
+	upload.cleanup(revertNodeMetadata, !keepUpload, !keepUpload)
 
-	// unset processing status
-	if upload.Node != nil { // node can be nil when there was an error before it was created (eg. checksum-mismatch)
-		if err := upload.Node.UnmarkProcessing(ctx, upload.Info.ID); err != nil {
-			upload.log.Info().Str("path", upload.Node.InternalPath()).Err(err).Msg("unmarking processing failed")
+	if unmarkPostprocessing {
+		// unset processing status
+		if upload.Node != nil { // node can be nil when there was an error before it was created (eg. checksum-mismatch)
+			if err := upload.Node.UnmarkProcessing(ctx, upload.Info.ID); err != nil {
+				upload.log.Info().Str("path", upload.Node.InternalPath()).Err(err).Msg("unmarking processing failed")
+			}
 		}
 	}
 }
@@ -247,7 +249,7 @@ func (upload *Upload) FinishUpload(_ context.Context) error {
 			err = errtypes.BadRequest("unsupported checksum algorithm: " + parts[0])
 		}
 		if err != nil {
-			Cleanup(upload, true, false)
+			Cleanup(upload, true, false, false)
 			return err
 		}
 	}
@@ -261,7 +263,7 @@ func (upload *Upload) FinishUpload(_ context.Context) error {
 
 	n, err := CreateNodeForUpload(upload, attrs)
 	if err != nil {
-		Cleanup(upload, true, false)
+		Cleanup(upload, true, false, false)
 		return err
 	}
 
@@ -290,7 +292,7 @@ func (upload *Upload) FinishUpload(_ context.Context) error {
 	if !upload.async {
 		// handle postprocessing synchronously
 		err = upload.Finalize()
-		Cleanup(upload, err != nil, false)
+		Cleanup(upload, err != nil, false, err == nil)
 		if err != nil {
 			log.Error().Err(err).Msg("failed to upload")
 			return err
@@ -382,8 +384,8 @@ func (upload *Upload) checkHash(expected string, h hash.Hash) error {
 }
 
 // cleanup cleans up after the upload is finished
-func (upload *Upload) cleanup(cleanNode, cleanBin, cleanInfo bool) {
-	if cleanNode && upload.Node != nil {
+func (upload *Upload) cleanup(revertNodeMetadata, cleanBin, cleanInfo bool) {
+	if revertNodeMetadata && upload.Node != nil {
 		switch p := upload.versionsPath; p {
 		case "":
 			// remove node