From 69297a4a38cb2e734b2c4db69c0bfdee57a1db9b Mon Sep 17 00:00:00 2001
From: Denis Kirillov <d.kirillov@yadro.com>
Date: Fri, 26 Jan 2024 16:16:14 +0300
Subject: [PATCH] [#165] Delete object from tree in case of storage error

Extend storage node errors in case of which we continue deleting from tree
with 'object not found' error

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
---
 api/handler/delete_test.go | 38 +++++++++++++++++++-------------------
 api/layer/layer.go         | 21 ++++++---------------
 internal/logs/logs.go      |  3 +--
 3 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/api/handler/delete_test.go b/api/handler/delete_test.go
index 28af78e1..6f60dbbb 100644
--- a/api/handler/delete_test.go
+++ b/api/handler/delete_test.go
@@ -65,25 +65,25 @@ func TestDeleteBucket(t *testing.T) {
 	deleteBucket(t, tc, bktName, http.StatusNoContent)
 }
 
-//func TestDeleteBucketOnNotFoundError(t *testing.T) {
-//	hc := prepareHandlerContext(t)
-//
-//	bktName, objName := "bucket-for-removal", "object-to-delete"
-//	bktInfo := createTestBucket(hc, bktName)
-//
-//	putObject(hc, bktName, objName)
-//
-//	nodeVersion, err := hc.tree.GetUnversioned(hc.context, bktInfo, objName)
-//	require.NoError(t, err)
-//	var addr oid.Address
-//	addr.SetContainer(bktInfo.CID)
-//	addr.SetObject(nodeVersion.OID)
-//	hc.tp.SetObjectError(addr, &apistatus.ObjectNotFound{})
-//
-//	deleteObjects(t, hc, bktName, [][2]string{{objName, emptyVersion}})
-//
-//	deleteBucket(t, hc, bktName, http.StatusNoContent)
-//}
+func TestDeleteBucketOnNotFoundError(t *testing.T) {
+	hc := prepareHandlerContext(t)
+
+	bktName, objName := "bucket-for-removal", "object-to-delete"
+	bktInfo := createTestBucket(hc, bktName)
+
+	putObject(hc, bktName, objName)
+
+	nodeVersion, err := hc.tree.GetUnversioned(hc.context, bktInfo, objName)
+	require.NoError(t, err)
+	var addr oid.Address
+	addr.SetContainer(bktInfo.CID)
+	addr.SetObject(nodeVersion.OID)
+	hc.tp.SetObjectError(addr, &apistatus.ObjectNotFound{})
+
+	deleteObjects(t, hc, bktName, [][2]string{{objName, emptyVersion}})
+
+	deleteBucket(t, hc, bktName, http.StatusNoContent)
+}
 
 func TestDeleteObjectsError(t *testing.T) {
 	hc := prepareHandlerContext(t)
diff --git a/api/layer/layer.go b/api/layer/layer.go
index 24870afe..76fe496e 100644
--- a/api/layer/layer.go
+++ b/api/layer/layer.go
@@ -711,24 +711,15 @@ func (n *layer) handleNotFoundError(bkt *data.BucketInfo, obj *VersionedObject)
 }
 
 func (n *layer) handleObjectDeleteErrors(ctx context.Context, bkt *data.BucketInfo, obj *VersionedObject, nodeID uint64) *VersionedObject {
-	if client.IsErrObjectAlreadyRemoved(obj.Error) {
-		n.reqLogger(ctx).Debug(logs.ObjectAlreadyRemoved,
-			zap.Stringer("cid", bkt.CID), zap.String("oid", obj.VersionID))
-
-		obj.Error = n.treeService.RemoveVersion(ctx, bkt, nodeID)
-		if obj.Error != nil {
-			return obj
-		}
-
-		n.cache.DeleteObjectName(bkt.CID, bkt.Name, obj.Name)
+	if !client.IsErrObjectAlreadyRemoved(obj.Error) && !client.IsErrObjectNotFound(obj.Error) {
+		return obj
 	}
 
-	if client.IsErrObjectNotFound(obj.Error) {
-		n.reqLogger(ctx).Debug(logs.ObjectNotFound,
-			zap.Stringer("cid", bkt.CID), zap.String("oid", obj.VersionID))
-
-		obj.Error = nil
+	n.reqLogger(ctx).Debug(logs.CouldntDeleteObjectFromStorageContinueDeleting,
+		zap.Stringer("cid", bkt.CID), zap.String("oid", obj.VersionID), zap.Error(obj.Error))
 
+	obj.Error = n.treeService.RemoveVersion(ctx, bkt, nodeID)
+	if obj.Error == nil {
 		n.cache.DeleteObjectName(bkt.CID, bkt.Name, obj.Name)
 	}
 
diff --git a/internal/logs/logs.go b/internal/logs/logs.go
index 7aba6c34..3c460db1 100644
--- a/internal/logs/logs.go
+++ b/internal/logs/logs.go
@@ -80,8 +80,6 @@ const (
 	CouldntDeletePart                                    = "couldn't delete part"                                                                              // Warn in ../../api/layer/multipart_upload.go
 	PartDetails                                          = "part details"                                                                                      // Debug in ../../api/layer/multipart_upload.go
 	GetObject                                            = "get object"                                                                                        // Debug in ../../api/layer/layer.go
-	ObjectAlreadyRemoved                                 = "object already removed"                                                                            // Debug in ../../api/layer/layer.go
-	ObjectNotFound                                       = "object not found"                                                                                  // Debug in ../../api/layer/layer.go
 	ResolveBucket                                        = "resolve bucket"                                                                                    // Info in ../../api/layer/layer.go
 	CouldntDeleteCorsObject                              = "couldn't delete cors object"                                                                       // Error in ../../api/layer/cors.go
 	PutObject                                            = "put object"                                                                                        // Debug in ../../api/layer/object.go
@@ -140,4 +138,5 @@ const (
 	PolicyValidationFailed                               = "policy validation failed"
 	ParseTreeNode                                        = "parse tree node"
 	FailedToGetRealObjectSize                            = "failed to get real object size"
+	CouldntDeleteObjectFromStorageContinueDeleting       = "couldn't delete object from storage, continue deleting from tree"
 )