Repository: minio/minio (language: go)

Issue:
after deleting the replica of an object, a folder with a size of zero remains
I am using active-active bucket replication. I made two cluster of minio, and both have the same configurations. Each cluster has 3 nodes and 2 drivers per node(thus 6 drivers total).
I am trying to test deleting objects.
I am copying a 50kb text file to a replicated bucket, set tags on the object. I check that the object appeared on the second cluster. After that, i delete the object on the first cluster. 
But on the other cluster, an empty folder with the name of the deleted object remains. This directory cannot be deleted.
**If I copy a file less than 50 kilobytes or don't set the tags**, then deleting the replicated object works correctly, no empty folder is created.
## Expected Behavior
Object replica is deleted without leaving an empty folder.

## Current Behavior
The object replica is deleted, but a directory with a size of zero is created.

## Possible Solution
???

## Steps to Reproduce (for bugs)

1. creating clusters.
`mcli admin info webtest1`

> ●  192.0.2.170:9000
>    Uptime: 2 weeks 
>    Version: 2022-02-01T18:00:14Z
>    Network: 3/3 OK 
>    Drives: 2/2 OK 
> 
> ●  192.0.2.171:9000
>    Uptime: 2 weeks 
>    Version: 2022-02-01T18:00:14Z
>    Network: 3/3 OK 
>    Drives: 2/2 OK 
> 
> ●  192.0.2.172:9000
>    Uptime: 2 weeks 
>    Version: 2022-02-01T18:00:14Z
>    Network: 3/3 OK 
>    Drives: 2/2 OK 

`mcli admin info webtest2`

> ●  192.0.2.177:9000
>    Uptime: 2 weeks 
>    Version: 2022-02-01T18:00:14Z
>    Network: 3/3 OK 
>    Drives: 2/2 OK 
> 
> ●  192.0.2.178:9000
>    Uptime: 2 weeks 
>    Version: 2022-02-01T18:00:14Z
>    Network: 3/3 OK 
>    Drives: 2/2 OK 
> 
> ●  192.0.2.179:9000
>    Uptime: 2 weeks 
>    Version: 2022-02-01T18:00:14Z
>    Network: 3/3 OK 
>    Drives: 2/2 OK 

2. Сreate a buckets and set up replication.
`mcli stat webtest1/test-replication-bucket`

> Name      : test-replication-bucket/
> Size      : 0 B    
> Type      : folder 
> Metadata  :
>   Versioning: Enabled
>   Replication: Enabled
>   Location: us-east-1
>   Policy: none

`mcli admin bucket remote ls --json webtest1/test-replication-bucket`

> {
>  "status": "success",
>  "accessKey": "minio",
>  "sourceBucket": "test-replication-bucket",
>  "TargetURL": "http://minio.remote.net:9000",
>  "TargetBucket": "test-replication-bucket",
>  "RemoteARN": "arn:minio:replication::9c47a610-1135-48c3-9688-2ef6f935c1d7:test-replication-bucket",
>  "service": "replication",
>  "bandwidth": 0,
>  "replicationSync": false,
>  "proxy": true,
>  "healthcheckDuration": 0,
>  "resetID": "",
>  "resetBeforeDate": "0001-01-01T00:00:00Z"
> }

`mcli replicate ls --json webtest1/test-replication-bucket`

> {
>  "op": "",
>  "status": "success",
>  "url": "",
>  "rule": {
>   "ID": "c9clf6mgdp4t7h2bv0e0",
>   "Status": "Enabled",
>   "Priority": 0,
>   "DeleteMarkerReplication": {
>    "Status": "Enabled"
>   },
>   "DeleteReplication": {
>    "Status": "Enabled"
>   },
>   "Destination": {
>    "Bucket": "arn:aws:s3:::test-replication-bucket"
>   },
>   "Filter": {
>    "And": {},
>    "Tag": {}
>   },
>   "SourceSelectionCriteria": {
>    "ReplicaModifications": {
>     "Status": "Enabled"
>    }
>   },
>   "ExistingObjectReplication": {
>    "Status": "Enabled"
>   }
>  }
> }


on the second cluster similarly

3. create a text file with a size of 50 kilobytes
`base64 /dev/urandom | head -c 50000 > test-obj.txt`

4. copy the file to Minio. Set the tag.
`mcli cp test-obj.txt webtest1/test-replication-bucket/`
`mcli tag set --versions webtest1/test-replication-bucket/test-obj.txt "mytag=testtag"`

5. check the object on both clusters.
`mcli stat --versions --json webtest1/test-replication-bucket/test-obj.txt`

> {
>  "status": "success",
>  "name": "test-obj.txt",
>  "lastModified": "2022-04-18T11:13:40+03:00",
>  "size": 50000,
>  "etag": "17c6b3d0621fb71dc30fc084179d52f2",
>  "type": "file",
>  "replicationStatus": "COMPLETED",
>  "metadata": {
>   "Content-Type": "text/plain",
>   "X-Amz-Tagging-Count": "1"
>  },
>  "versionID": "ce491af5-d4dc-464f-9528-ec130111dcbd"
> }

`mcli stat --json --versions webtest2/test-replication-bucket/test-obj.txt`

> {
>  "status": "success",
>  "name": "test-obj.txt",
>  "lastModified": "2022-04-18T11:13:40+03:00",
>  "size": 50000,
>  "etag": "17c6b3d0621fb71dc30fc084179d52f2",
>  "type": "file",
>  "replicationStatus": "REPLICA",
>  "metadata": {
>   "Content-Type": "text/plain",
>   "X-Amz-Tagging-Count": "1"
>  },
>  "versionID": "ce491af5-d4dc-464f-9528-ec130111dcbd"
> }

Object replication successful.

6. I delete the object on the first cluster and check on the second.
`mcli rm --versions webtest1/test-replication-bucket/test-obj.txt`

> Removing `webtest1/test-obj.txt` (versionId=ce491af5-d4dc-464f-9528-ec130111dcbd).

On the second cluster there is a folder with zero size
`mcli ls --versions --json webtest2/test-replication-bucket/`
 
> {
>  "status": "success",
>  "type": "folder",
>  "lastModified": "2022-04-18T11:32:53.148126348+03:00",
>  "size": 0,
>  "key": "test-obj.txt/",
>  "etag": "",
>  "url": "http://192.0.2.179:9000/test-replication-bucket/",
>  "versionOrdinal": 1
> }
> 

`mcli ls --versions --json webtest2/test-replication-bucket/test-obj.txt/`

> {
>  "status": "success",
>  "type": "folder",
>  "lastModified": "2022-04-18T11:35:15.114666549+03:00",
>  "size": 0,
>  "key": "bc6dd250-8120-448a-ae53-6fb13f197961/",
>  "etag": "",
>  "url": "http://192.0.2.179:9000/test-replication-bucket/test-obj.txt/",
>  "versionOrdinal": 1
> }

`mcli ls --versions --json webtest2/test-replication-bucket/test-obj.txt/bc6dd250-8120-448a-ae53-6fb13f197961`

> Nothing


## Context
These directories are not needed.


## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Version used (`minio --version`): minio version RELEASE.2022-02-01T18-00-14Z
* Server setup and configuration:
* Operating System and version (`uname -a`): Linux mn-a2p-webtest1.vimpelcom.ru 4.18.0-348.2.1.el8_5.x86_64 #1 SMP Mon Nov 15 09:17:08 EST 2021 x86_64 x86_64 x86_64 GNU/Linux



Discussion / hints:
updated Minio to latest version

`minio --version`

> minio version RELEASE.2022-04-16T04-26-02Z

Problem still exists
Please describe your setup details such as command line etc 
> Please describe your setup details such as command line etc

Start minio in the first cluster(webtest1):
`/usr/local/bin/minio server --address 192.0.2.172:9000 http://192.0.2.170/data/minio11 http://192.0.2.170/data/minio12 http://192.0.2.171/data/minio11 http://192.0.2.171/data/minio12 http://192.0.2.172/data/minio11 http://192.0.2.172/data/minio12`

Start minio in the second cluster(webtest2):
`/usr/local/bin/minio server --address 192.0.2.179:9000 http://192.0.2.177/data/minio11 http://192.0.2.177/data/minio12 http://192.0.2.178/data/minio11 http://192.0.2.178/data/minio12 http://192.0.2.179/data/minio11 http://192.0.2.179/data/minio12`

I hope I understood your question correctly.
Found these objects in the directory(on second cluster)

`tree /data/minio11/test-replication-bucket/`

/data/minio11/test-replication-bucket/
├── test-obj1.txt
│   └── bc8e8925-4be4-485d-8043-36c115d064e5
│       └── part.1
├── test-obj2.txt
│   └── 871194a2-3af0-4c8a-b99f-ac8289ab8c95
│       └── part.1
├── test-obj4.txt
│   ├── 1e639b3c-ed0e-44cc-9e3d-ccd3efc56f32
│   │   └── part.1
│   ├── 4ee01d81-873f-4577-9b66-a179757f7cdc
│   │   └── part.1
│   └── xl.meta
├── test-obj5.txt
│   └── xl.meta
└── test-obj.txt
    └── bc6dd250-8120-448a-ae53-6fb13f197961
        └── part.1

Objects "test-obj.txt", "test-obj1.txt", "test-obj2.txt" do not exist, in minio they are represented by an empty directory. But in the filesystem the file has the content:
`stat /data/minio11/test-replication-bucket/test-obj.txt/bc6dd250-8120-448a-ae53-6fb13f197961/part.1`
> File: /data/minio11/test-replication-bucket/test-obj.txt/bc6dd250-8120-448a-ae53-6fb13f197961/part.1
>   Size: 16699           Blocks: 40         IO Block: 4096   regular file

Object "test-obj4.txt" exists. Its size is more than 50 kilobytes.
Object "test-obj5.txt" exists. Its size is less than 50 kilobytes. After deletion replicate it will disappear.

`mcli ls --versions webtest2/test-replication-bucket/`

> [2022-04-19 05:34:52 UTC]  59KiB STANDARD e63e95c8-b22e-4e38-9624-5ca13d9aa0b2 v1 PUT test-obj4.txt
> [2022-04-19 05:38:22 UTC]  29KiB STANDARD d3a91cc9-e0da-4caf-a6a0-a721559b9d0c v1 PUT test-obj5.txt
> [2022-04-19 05:59:51 UTC]     0B test-obj.txt/
> [2022-04-19 05:59:51 UTC]     0B test-obj1.txt/
> [2022-04-19 05:59:51 UTC]     0B test-obj2.txt/

The same directory on the first cluster:

`tree /data/minio11/test-replication-bucket/`

/data/minio11/test-replication-bucket/
├── test-obj4.txt
│   ├── 58c2ad7c-4b97-46e1-997a-4dfe86a2cd3b
│   │   └── part.1
│   └── xl.meta
└── test-obj5.txt
    └── xl.meta


I hope this helps.



this is good we will get back @tifling85 
> `/usr/local/bin/minio server --address 192.0.2.172:9000 http://192.0.2.170/data/minio11 http://192.0.2.170/data/minio12 http://192.0.2.171/data/minio11 http://192.0.2.171/data/minio12 http://192.0.2.172/data/minio11 http://192.0.2.172/data/minio12`

This is an incorrect way to setup MinIO you should set it up like this

```
minio server http://192.0.2.17{0...2}/data/minio{11...12}
```


> `/usr/local/bin/minio server --address 192.0.2.179:9000 http://192.0.2.177/data/minio11 http://192.0.2.177/data/minio12 http://192.0.2.178/data/minio11 http://192.0.2.178/data/minio12 http://192.0.2.179/data/minio11 http://192.0.2.179/data/minio12`


```
minio server http://192.0.2.17{7...9}/data/minio{11...12}
```

However I can reproduce the problem   
Okay, I re-created the cluster the way you said. Problem still exists.

A new one has also been added: when deleting an object with the --version flag, a deletion marker remains. This happens in a bucket with replication enabled.
Take a basket with versioning, but without replication:
`mcli stat webtest1/test-bucket`

> Name      : test-bucket/
> Size      : 0 B    
> Type      : folder 
> Metadata  :
>   Versioning: Enabled

`mcli cp test-obj1.txt webtest1/test-bucket/object`
`mcli ls --versions webtest1/test-bucket`

> [2022-04-20 05:41:50 UTC]  59KiB STANDARD cbe7ec47-6783-4408-90a8-253bb27f1c86 v1 PUT object

`mcli rm --versions --force webtest1/test-bucket/object`

> Removing `webtest1/test-bucket/object` (versionId=cbe7ec47-6783-4408-90a8-253bb27f1c86)

`mcli ls --versions webtest1/test-bucket`

> Nothing

Everything is fine here.

Now we repeat with the bucket on which replication is configured:

`mcli cp test-obj1.txt webtest1/test-replication-bucket/object`

`mcli ls --versions webtest1/test-replication-bucket/`

> [2022-04-20 05:47:01 UTC]  59KiB STANDARD f6c82093-bb5d-4c03-8d22-bdcfdbdbef82 v1 PUT object

`mcli rm --force --versions webtest1/test-replication-bucket/object`

> Removing `webtest1/test-replication-bucket/object` (versionId=f6c82093-bb5d-4c03-8d22-bdcfdbdbef82)

`mcli ls --versions webtest1/test-replication-bucket/`

> [2022-04-20 05:48:30 UTC]     0B STANDARD f6c82093-bb5d-4c03-8d22-bdcfdbdbef82 v1 DEL object

The delete marker remains!
It is correct? This was not the case with the previous(incorrect way) setup.
Thanks.

Yes this is a bug, there are two bugs here.

- one is errant delete marker left over on source cluster, but this is removed in the next scanner cycle (already fixed it locally will send a PR)
- another is tags is creating an incorrect state on the destination cluster (on replica) - I have fixed one corner case here, but there is one more problem to be fixed will send separate fixes for both.
> one is errant delete marker left over on source cluster, but this is removed in the next scanner cycle (already fixed it locally will send a PR)

Yes, the delete marker was removed after some time.

> another is tags is creating an incorrect state on the destination cluster (on replica) - I have fixed one corner case here, but there is one more problem to be fixed will send separate fixes for both.

Ok, i'm waiting for the release with the fix. Thanks.

Fix patch:
diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go
index f565e8f60d7c5..062b8459303c5 100644
--- a/cmd/erasure-object.go
+++ b/cmd/erasure-object.go
@@ -112,9 +112,20 @@ func (er erasureObjects) CopyObject(ctx context.Context, srcBucket, srcObject, d
 		// preserve destination versionId if specified.
 		if versionID == "" {
 			versionID = mustGetUUID()
+			fi.IsLatest = true // we are creating a new version so this is latest.
 		}
 		modTime = UTCNow()
 	}
+
+	// If the data is not inlined, we may end up incorrectly
+	// inlining the data here, that leads to an inconsistent
+	// situation where some objects are were not inlined
+	// were now inlined, make sure to `nil` the Data such
+	// that xl.meta is written as expected.
+	if !fi.InlineData() {
+		fi.Data = nil
+	}
+
 	fi.VersionID = versionID // set any new versionID we might have created
 	fi.ModTime = modTime     // set modTime for the new versionID
 	if !dstOpts.MTime.IsZero() {
@@ -130,6 +141,14 @@ func (er erasureObjects) CopyObject(ctx context.Context, srcBucket, srcObject, d
 			metaArr[index].ModTime = modTime
 			metaArr[index].VersionID = versionID
 			metaArr[index].Metadata = srcInfo.UserDefined
+			if !metaArr[index].InlineData() {
+				// If the data is not inlined, we may end up incorrectly
+				// inlining the data here, that leads to an inconsistent
+				// situation where some objects are were not inlined
+				// were now inlined, make sure to `nil` the Data such
+				// that xl.meta is written as expected.
+				metaArr[index].Data = nil
+			}
 		}
 	}
 
@@ -138,8 +157,6 @@ func (er erasureObjects) CopyObject(ctx context.Context, srcBucket, srcObject, d
 		return oi, toObjectErr(err, srcBucket, srcObject)
 	}
 
-	// we are adding a new version to this object under the namespace lock, so this is the latest version.
-	fi.IsLatest = true
 	return fi.ToObjectInfo(srcBucket, srcObject), nil
 }
 
@@ -1319,10 +1336,21 @@ func (er erasureObjects) DeleteObject(ctx context.Context, bucket, object string
 		if opts.VersionPurgeStatus() == Complete {
 			markDelete = false
 		}
-		// determine if the version represents an object delete
-		// deleteMarker = true
-		if versionFound && !goi.DeleteMarker { // implies a versioned delete of object
-			deleteMarker = false
+
+		// Version is found but we do not wish to create more delete markers
+		// now, since VersionPurgeStatus() is already set, we can let the
+		// lower layers decide this. This fixes a regression that was introduced
+		// in PR #14555 where !VersionPurgeStatus.Empty() is automatically
+		// considered as Delete marker true to avoid listing such objects by
+		// regular ListObjects() calls. However for delete replication this
+		// ends up being a problem because "upon" a successful delete this
+		// ends up creating a new delete marker that is spurious and unnecessary.
+		if versionFound {
+			if !goi.VersionPurgeStatus.Empty() {
+				deleteMarker = false
+			} else if !goi.DeleteMarker { // implies a versioned delete of object
+				deleteMarker = false
+			}
 		}
 	}
 
diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go
index 4635a0fac82da..2c4e73375d20e 100644
--- a/cmd/object-handlers.go
+++ b/cmd/object-handlers.go
@@ -1118,10 +1118,10 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 	}
 
 	var chStorageClass bool
-	if dstSc != "" {
+	if dstSc != "" && dstSc != srcInfo.StorageClass {
 		chStorageClass = true
 		srcInfo.metadataOnly = false
-	}
+	} // no changes in storage-class expected so its a metadataonly operation.
 
 	var reader io.Reader = gr
 
diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go
index 6eb1f1da8f79d..6a6e1ba1e63f3 100644
--- a/cmd/xl-storage.go
+++ b/cmd/xl-storage.go
@@ -901,19 +901,24 @@ func (s *xlStorage) deleteVersions(ctx context.Context, volume, path string, fis
 			if versionID == "" {
 				versionID = nullVersionID
 			}
+
 			// PR #11758 used DataDir, preserve it
 			// for users who might have used master
 			// branch
-			if !xlMeta.data.remove(versionID, dataDir) {
-				filePath := pathJoin(volumeDir, path, dataDir)
-				if err = checkPathLength(filePath); err != nil {
+			xlMeta.data.remove(versionID, dataDir)
+
+			// We need to attempt delete "dataDir" on the disk
+			// due to a CopyObject() bug where it might have
+			// inlined the data incorrectly, to avoid a situation
+			// where we potentially leave "DataDir"
+			filePath := pathJoin(volumeDir, path, dataDir)
+			if err = checkPathLength(filePath); err != nil {
+				return err
+			}
+			if err = s.moveToTrash(filePath, true); err != nil {
+				if err != errFileNotFound {
 					return err
 				}
-				if err = s.moveToTrash(filePath, true); err != nil {
-					if err != errFileNotFound {
-						return err
-					}
-				}
 			}
 		}
 	}
@@ -1026,16 +1031,20 @@ func (s *xlStorage) DeleteVersion(ctx context.Context, volume, path string, fi F
 		// PR #11758 used DataDir, preserve it
 		// for users who might have used master
 		// branch
-		if !xlMeta.data.remove(versionID, dataDir) {
-			filePath := pathJoin(volumeDir, path, dataDir)
-			if err = checkPathLength(filePath); err != nil {
+		xlMeta.data.remove(versionID, dataDir)
+
+		// We need to attempt delete "dataDir" on the disk
+		// due to a CopyObject() bug where it might have
+		// inlined the data incorrectly, to avoid a situation
+		// where we potentially leave "DataDir"
+		filePath := pathJoin(volumeDir, path, dataDir)
+		if err = checkPathLength(filePath); err != nil {
+			return err
+		}
+		if err = s.moveToTrash(filePath, true); err != nil {
+			if err != errFileNotFound {
 				return err
 			}
-			if err = s.moveToTrash(filePath, true); err != nil {
-				if err != errFileNotFound {
-					return err
-				}
-			}
 		}
 	}
 


Test patch: