adoroszlai commented on code in PR #6385:
URL: https://github.com/apache/ozone/pull/6385#discussion_r1530868087
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneKey.java:
##########
@@ -70,11 +70,19 @@ public class OzoneKey {
* Constructs OzoneKey from OmKeyInfo.
*
*/
+
+ /**
+ * The object and update ID of an existing key. These will be null
+ * if the key is not yet created on OM and read from OM.
+ */
+ private final Long objectID;
+ private final Long updateID;
+
@SuppressWarnings("parameternumber")
public OzoneKey(String volumeName, String bucketName,
String keyName, long size, long creationTime,
long modificationTime, ReplicationConfig replicationConfig,
- boolean isFile) {
+ boolean isFile, Long objectID, Long updateID) {
Review Comment:
Should we add the 2 extra parameters in an overloaded constructor? (to avoid
breaking existing code and having to add `null, null` in various places) We
can switch to using a builder separately.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java:
##########
@@ -353,6 +353,19 @@ OzoneOutputStream createKey(String volumeName, String
bucketName,
Map<String, String> metadata)
throws IOException;
+ /**
+ * Overwrite an existing key using optimistic locking. The OzoneKeyDetails
passed must contain
+ * the objectID and updateID of the key to be overwritten. The existing key
must also exist on
+ * OM and have the same ObjectID and UpdateID as the one passed in or the
key create / commit
+ * will fail.
+ * @param keyToOverwrite Existing key to overwrite
+ * @param replicationConfig The replication configuration for the new key
+ * @return {@link OzoneOutputStream}
+ * @throws IOException
+ */
+ OzoneOutputStream overWriteKey(OzoneKeyDetails keyToOverwrite,
ReplicationConfig replicationConfig)
Review Comment:
```suggestion
OzoneOutputStream overwriteKey(OzoneKeyDetails keyToOverwrite,
ReplicationConfig replicationConfig)
```
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java:
##########
@@ -469,6 +469,25 @@ public OzoneOutputStream createKey(String key, long size,
.createKey(volumeName, name, key, size, replicationConfig,
keyMetadata);
}
+ /**
+ * Overwrite an existing key using optimistic locking. The existingKey must
exist in Ozone to allow
+ * the new key to be created with the same name. Additionally, the
existingKey must not have been
+ * modified since the time its details were read. This is controlled by the
objectID and updateID
+ * fields in the existingKey. If the key is replaced the objectID will
change. If it has been updated
+ * (eg appended) the updateID will change. If either of these have changed
since the existingKey
+ * was read, either the initial key create will fail, or the key will fail
to commit after the data
+ * has been written.
+ *
+ * @param existingKey Name of the key to be created.
+ * @param replicationConfig Replication configuration.
+ * @return OzoneOutputStream to which the data has to be written.
+ * @throws IOException
+ */
+ public OzoneOutputStream overWriteKey(OzoneKeyDetails existingKey,
ReplicationConfig replicationConfig)
Review Comment:
nit: "overwrite" is used as one word in comments and some variable names, so
I think method name should match that.
```suggestion
public OzoneOutputStream overwriteKey(OzoneKeyDetails existingKey,
ReplicationConfig replicationConfig)
```
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1420,6 +1401,70 @@ public OzoneOutputStream createKey(
return createOutputStream(openKey);
}
+ @Override
+ public OzoneOutputStream overWriteKey(OzoneKeyDetails keyToOverwrite,
ReplicationConfig replicationConfig)
+ throws IOException {
+ if (keyToOverwrite == null) {
+ throw new IllegalArgumentException("KeyToOverwrite cannot be null");
+ }
+ if (keyToOverwrite.getObjectID() == null) {
+ throw new IllegalArgumentException("KeyToOverwrite ObjectID cannot be
null");
+ }
+ if (keyToOverwrite.getUpdateID() == null) {
+ throw new IllegalArgumentException("KeyToOverwrite UpdateID cannot be
null");
+ }
+ createKeyPreChecks(keyToOverwrite.getVolumeName(),
keyToOverwrite.getBucketName(),
+ keyToOverwrite.getName(), replicationConfig);
+
+ OmKeyArgs.Builder builder = new OmKeyArgs.Builder()
+ .setVolumeName(keyToOverwrite.getVolumeName())
+ .setBucketName(keyToOverwrite.getBucketName())
+ .setKeyName(keyToOverwrite.getName())
+ .setDataSize(keyToOverwrite.getDataSize())
+ .setReplicationConfig(replicationConfig)
+ .addAllMetadataGdpr(keyToOverwrite.getMetadata())
+ // TODO - if we are effectively cloning a key, probably the ACLs should
+ // be copied over server side. I am not too sure how this works.
+ .setAcls(getAclList())
Review Comment:
Good point. I think we should omit metadata and ACLs, if possible, and let
OM copy them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
