HDDS-10010. Support snapshot rename operation

[Cyrill]

What changes were proposed in this pull request?

HDDS-10010. Support snapshot rename operation

Ozone implements org.apache.hadoop.fs.FileSystem, which has three methods to work with snapshots - create, rename and delete. Only create and delete are implemented at the moment.
Implementing snapshot rename will improve Ozone compatibility with other software (that uses snapshots functionality).

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10010

How was this patch tested?

Tested with unit tests an manual testing.

[hemantk-12]

Thanks for the patch @Cyrill.
I looked at briefly and the only two major concerns I've are

1. Snapshot chain manager needs to be updated.
2. SnapshotCache: currently snapshotCache used /volName/bucketName/snapshotName as key for the cache. When we do a rename, we may need to update the cache as well. Or we can think if we should use snapshotId for it.

Afair, SnapshotDiff uses SnapshotId and snapshot rename should not be a problem there but it would be great if you can double check that part as well.

[hemantk-12]

My main concern with snapshot renaming is SnapshotCache. Currently SnapshotName is used as key in SnapshotCache.

Invalidating cache for snapshot rename doesn't make sense to me. Invalidate should only be used when snapshot is purged. Also invalidating cache for active snapshot will impact background service (KeyDeletingService, SSTFilteringService, SnapshotDeletingService, and SnapDiff).

To me better approach would be to use SnapshotId as key in SnapshotCache. This change could be a follow up task. I'm to other suggestion as well.

[hemantk-12]

1. I don't think it is good idea to invalidate the cache because of snapshot rename. I guess better approach would be to use snapshotId as key for snapshotCache.
2. Unit test is failing due to NPE in this line. Please take a look of that.

Error:  Tests run: 13, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 4.973 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.request.snapshot.TestOMSnapshotRenameRequest
Error:  org.apache.hadoop.ozone.om.request.snapshot.TestOMSnapshotRenameRequest.testValidateAndUpdateCache -- Time elapsed: 0.178 s <<< ERROR!
java.lang.NullPointerException
	at org.apache.hadoop.ozone.om.request.snapshot.OMSnapshotRenameRequest.validateAndUpdateCache(OMSnapshotRenameRequest.java:192)
	at org.apache.hadoop.ozone.om.request.OMClientRequest.validateAndUpdateCache(OMClientRequest.java:145)
	at org.apache.hadoop.ozone.om.request.snapshot.TestOMSnapshotRenameRequest.testValidateAndUpdateCache(TestOMSnapshotRenameRequest.java:217)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at java.util.ArrayList.forEach(ArrayList.java:1259)
	at java.util.ArrayList.forEach(ArrayList.java:1259

[Cyrill]

2. done

[Cyrill]

1. Removed the cache invalidate call here

[aswinshakil]

I took a quick look at the patch. My main concern is similar to what @hemantk-12 suggested, There is KeyDeletingService, SnapshotDirectoryCleanupService and the OM requests used by this services pass the snapshot name in the request to OM to identify the snapshot. If a rename happened in between these requests this may cause undefined behaviour.

I would suggest changing these other classes as well to use snapshotId instead of snapshot name is the way to go forward.

[Cyrill]

Hi @hemantk-12, are there any more changes required for this PR to move forward?

[Cyrill]

@hemantk-12 @aswinshakil Is there any chance to help me with moving this PR forward?

[hemantk-12]

@Cyrill I was busy with other stuff last week and didn't get chance to look into this.

LGTM.