Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5fe2ff7
Add existing update / object IDs to OmKeyArgs and OmKeyInfo
Mar 12, 2024
aa3bd32
modified createKeyRequest and commitKeyRequest with tests
Mar 13, 2024
9799ba5
Push objectid / updateid through various classes
Mar 14, 2024
5040d2b
Adapt client to be able to overwrite a key
Mar 15, 2024
6f45e67
Remove debugging log messages
Mar 15, 2024
0635134
Fix find bugs
Mar 15, 2024
c151122
Ensure test only runs for non FSO for now
Mar 15, 2024
908f9cd
Fix similar commit test on FSO
Mar 15, 2024
3c9c0e2
Rename overWrite to overwrite
Mar 20, 2024
17b5ff1
Overload constructors to avoid passing nulls for the new variables
Mar 20, 2024
0fe12c5
Avoid setting acl in overwrite request. Enhance test to ensure object…
Mar 20, 2024
ae4044c
Add overwrite object / update ID to the audit if they are present
Mar 21, 2024
7bb59e9
Minor review comments
Mar 22, 2024
8890ed8
Remove overwriteObjectID and base all logic on updateID only
Mar 22, 2024
a360d45
Block non ObjectStore buckets from using optimistic overwrite for now
Mar 22, 2024
3f3a49d
Validate existing ACLs are copied over from existing to overwrite key
Mar 22, 2024
736740a
Merge branch 'master' into HDDS-10527
Apr 2, 2024
525685c
Trigger rebuild
Apr 2, 2024
112e2e1
Rename overwriteUpdateID to overwriteGeneration
Apr 30, 2024
bcd1a28
Remove updateID references from client and replace with generation
Apr 30, 2024
4e6a97c
Change client API to rewriteKey
Apr 30, 2024
247620a
Fixed logic to not copy meta data from existing key, but take it from…
Apr 30, 2024
c6bc7e7
Merge branch 'master' into HDDS-10527
Apr 30, 2024
b91a051
Review comments
May 7, 2024
9dc6d31
Change BucketLayout.DEFAULT to getBucketLayout()
May 9, 2024
80e1802
Move getGeneration to OMKeyInfo
May 9, 2024
bb7b74f
Rename any instances of overwrite to rewrite
May 9, 2024
a87e092
Remove generation from OzoneKey
May 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,9 @@ private OzoneConsts() {
public static final String BUCKET_LAYOUT = "bucketLayout";
public static final String TENANT = "tenant";
public static final String USER_PREFIX = "userPrefix";
public static final String OVERWRITE_OBJECT_ID = "overwriteObjectID";
public static final String OVERWRITE_UPDATE_ID = "overwriteUpdateID";


// For multi-tenancy
public static final String TENANT_ID_USERNAME_DELIMITER = "$";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public OzoneOutputStream overwriteKey(OzoneKeyDetails existingKey, ReplicationConfig replicationConfig)
public OzoneOutputStream atomicUpdateKey(OzoneKeyDetails existingKey, ReplicationConfig replicationConfig)

throws IOException {
return proxy.overwriteKey(existingKey, replicationConfig);
}

/**
* Creates a new key in the bucket, with default replication type RATIS and
* with replication factor THREE.
Expand Down Expand Up @@ -1779,7 +1798,7 @@ private void addKeyPrefixInfoToResultList(String keyPrefix,
keyInfo.getDataSize(), keyInfo.getCreationTime(),
keyInfo.getModificationTime(),
keyInfo.getReplicationConfig(),
keyInfo.isFile());
keyInfo.isFile(), keyInfo.getObjectID(), keyInfo.getUpdateID());
keysResultList.add(ozoneKey);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaking internal names into public APIs. For public facing API, I would model it how google APIs have documented theirs : https://cloud.google.com/storage/docs/metadata#generation-number
(We do not have the split between metadata only updates vs. whole object.)

For now, I would change this to generation and document the guarantees that an application has around the use of generation. This should apply to the entire PR, where we use updateID which is a more nebulous name.


@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) {
this.volumeName = volumeName;
this.bucketName = bucketName;
this.name = keyName;
Expand All @@ -83,6 +91,17 @@ public OzoneKey(String volumeName, String bucketName,
this.modificationTime = Instant.ofEpochMilli(modificationTime);
this.replicationConfig = replicationConfig;
this.isFile = isFile;
this.objectID = objectID;
this.updateID = updateID;
}

@SuppressWarnings("parameternumber")
public OzoneKey(String volumeName, String bucketName,
String keyName, long size, long creationTime,
long modificationTime, ReplicationConfig replicationConfig,
boolean isFile) {
this(volumeName, bucketName, keyName, size, creationTime, modificationTime, replicationConfig,
isFile, null, null);
}

@SuppressWarnings("parameternumber")
Expand All @@ -91,10 +110,19 @@ public OzoneKey(String volumeName, String bucketName,
long modificationTime, ReplicationConfig replicationConfig,
Map<String, String> metadata, boolean isFile) {
this(volumeName, bucketName, keyName, size, creationTime,
modificationTime, replicationConfig, isFile);
modificationTime, replicationConfig, isFile, null, null);
this.metadata.putAll(metadata);
}

@SuppressWarnings("parameternumber")
public OzoneKey(String volumeName, String bucketName,
String keyName, long size, long creationTime,
long modificationTime, ReplicationConfig replicationConfig,
Map<String, String> metadata, boolean isFile, Long objectID, Long updateID) {
this(volumeName, bucketName, keyName, size, creationTime,
modificationTime, replicationConfig, isFile, objectID, updateID);
this.metadata.putAll(metadata);
}
/**
* Returns Volume Name associated with the Key.
*
Expand Down Expand Up @@ -179,6 +207,16 @@ public ReplicationConfig getReplicationConfig() {
return replicationConfig;
}

@JsonIgnore
public Long getObjectID() {
return objectID;
}

@JsonIgnore
public Long getUpdateID() {
return updateID;
}

/**
* Returns indicator if key is a file.
* @return file
Expand All @@ -191,7 +229,7 @@ public static OzoneKey fromKeyInfo(OmKeyInfo keyInfo) {
return new OzoneKey(keyInfo.getVolumeName(), keyInfo.getBucketName(),
keyInfo.getKeyName(), keyInfo.getDataSize(), keyInfo.getCreationTime(),
keyInfo.getModificationTime(), keyInfo.getReplicationConfig(),
keyInfo.getMetadata(), keyInfo.isFile());
keyInfo.getMetadata(), keyInfo.isFile(), keyInfo.getObjectID(), keyInfo.getUpdateID());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,31 @@ public OzoneKeyDetails(String volumeName, String bucketName, String keyName,
Map<String, String> metadata,
FileEncryptionInfo feInfo,
CheckedSupplier<OzoneInputStream, IOException> contentSupplier,
boolean isFile) {
boolean isFile, Long objectID, Long updateID) {
super(volumeName, bucketName, keyName, size, creationTime,
modificationTime, replicationConfig, metadata, isFile);
modificationTime, replicationConfig, metadata, isFile, objectID, updateID);
this.ozoneKeyLocations = ozoneKeyLocations;
this.feInfo = feInfo;
this.contentSupplier = contentSupplier;
}

/**
* Constructs OzoneKeyDetails from OmKeyInfo.
*/
@SuppressWarnings("parameternumber")
public OzoneKeyDetails(String volumeName, String bucketName, String keyName,
long size, long creationTime, long modificationTime,
List<OzoneKeyLocation> ozoneKeyLocations,
ReplicationConfig replicationConfig,
Map<String, String> metadata,
FileEncryptionInfo feInfo,
CheckedSupplier<OzoneInputStream, IOException> contentSupplier,
boolean isFile) {
this(volumeName, bucketName, keyName, size, creationTime,
modificationTime, ozoneKeyLocations, replicationConfig, metadata, feInfo, contentSupplier,
isFile, null, null);
}

/**
* Returns the location detail information of the specific Key.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
throws IOException;

/**
* Writes a key in an existing bucket.
* @param volumeName Name of the Volume
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1378,26 +1378,7 @@ public OzoneOutputStream createKey(
ReplicationConfig replicationConfig,
Map<String, String> metadata)
throws IOException {
verifyVolumeName(volumeName);
verifyBucketName(bucketName);
if (checkKeyNameEnabled) {
HddsClientUtils.verifyKeyName(keyName);
}
HddsClientUtils.checkNotNull(keyName);
if (omVersion
.compareTo(OzoneManagerVersion.ERASURE_CODED_STORAGE_SUPPORT) < 0) {
if (replicationConfig != null &&
replicationConfig.getReplicationType()
== HddsProtos.ReplicationType.EC) {
throw new IOException("Can not set the replication of the key to"
+ " Erasure Coded replication, as OzoneManager does not support"
+ " Erasure Coded replication.");
}
}

if (replicationConfig != null) {
replicationConfigValidator.validate(replicationConfig);
}
createKeyPreChecks(volumeName, bucketName, keyName, replicationConfig);

OmKeyArgs.Builder builder = new OmKeyArgs.Builder()
.setVolumeName(volumeName)
Expand All @@ -1420,6 +1401,67 @@ 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())
.setLatestVersionLocation(getLatestVersionLocation)
.setOverwriteObjectID(keyToOverwrite.getObjectID())
.setOverwriteUpdateID(keyToOverwrite.getUpdateID());

OpenKeySession openKey = ozoneManagerClient.openKey(builder.build());
// For bucket with layout OBJECT_STORE, when create an empty file (size=0),
// OM will set DataSize to OzoneConfigKeys#OZONE_SCM_BLOCK_SIZE,
// which will cause S3G's atomic write length check to fail,
// so reset size to 0 here.
if (isS3GRequest.get() && keyToOverwrite.getDataSize() == 0) {
openKey.getKeyInfo().setDataSize(0);
}
return createOutputStream(openKey);
}

private void createKeyPreChecks(String volumeName, String bucketName, String keyName,
ReplicationConfig replicationConfig) throws IOException {
verifyVolumeName(volumeName);
verifyBucketName(bucketName);
if (checkKeyNameEnabled) {
HddsClientUtils.verifyKeyName(keyName);
}
HddsClientUtils.checkNotNull(keyName);
if (omVersion
.compareTo(OzoneManagerVersion.ERASURE_CODED_STORAGE_SUPPORT) < 0) {
if (replicationConfig != null &&
replicationConfig.getReplicationType()
== HddsProtos.ReplicationType.EC) {
throw new IOException("Can not set the replication of the key to"
+ " Erasure Coded replication, as OzoneManager does not support"
+ " Erasure Coded replication.");
}
}

if (replicationConfig != null) {
replicationConfigValidator.validate(replicationConfig);
}
}

@Override
public OzoneDataStreamOutput createStreamKey(
String volumeName, String bucketName, String keyName, long size,
Expand Down Expand Up @@ -1643,7 +1685,9 @@ public List<OzoneKey> listKeys(String volumeName, String bucketName,
key.getCreationTime(),
key.getModificationTime(),
key.getReplicationConfig(),
key.isFile()))
key.isFile(),
null,
null))
.collect(Collectors.toList());
} else {
List<OmKeyInfo> keys = ozoneManagerClient.listKeys(
Expand All @@ -1656,7 +1700,9 @@ public List<OzoneKey> listKeys(String volumeName, String bucketName,
key.getCreationTime(),
key.getModificationTime(),
key.getReplicationConfig(),
key.isFile()))
key.isFile(),
key.getObjectID(),
key.getUpdateID()))
.collect(Collectors.toList());
}
}
Expand Down Expand Up @@ -1707,7 +1753,8 @@ private OzoneKeyDetails getOzoneKeyDetails(OmKeyInfo keyInfo) {
keyInfo.getModificationTime(), ozoneKeyLocations,
keyInfo.getReplicationConfig(), keyInfo.getMetadata(),
keyInfo.getFileEncryptionInfo(),
() -> getInputStreamWithRetryFunction(keyInfo), keyInfo.isFile());
() -> getInputStreamWithRetryFunction(keyInfo), keyInfo.isFile(),
keyInfo.getObjectID(), keyInfo.getUpdateID());
}

@Override
Expand Down
Loading