Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -541,29 +541,26 @@ public static File createOMDir(String dirPath) {
*/
public static RepeatedOmKeyInfo prepareKeyForDelete(long bucketId, OmKeyInfo keyInfo,
long trxnLogIndex) {
OmKeyInfo sanitizedKeyInfo = keyInfo;
OmKeyInfo.Builder builder = keyInfo.toBuilder();
// If this key is in a GDPR enforced bucket, then before moving
// KeyInfo to deletedTable, remove the GDPR related metadata and
// FileEncryptionInfo from KeyInfo.
if (Boolean.parseBoolean(
keyInfo.getMetadata().get(OzoneConsts.GDPR_FLAG))
) {
sanitizedKeyInfo = sanitizedKeyInfo.withMetadataMutations(metadata -> {
metadata.remove(OzoneConsts.GDPR_FLAG);
metadata.remove(OzoneConsts.GDPR_ALGORITHM);
metadata.remove(OzoneConsts.GDPR_SECRET);
});
sanitizedKeyInfo.clearFileEncryptionInfo();
Map<String, String> metadata = builder.getMetadata();
metadata.remove(OzoneConsts.GDPR_FLAG);
metadata.remove(OzoneConsts.GDPR_ALGORITHM);
metadata.remove(OzoneConsts.GDPR_SECRET);

builder.setFileEncryptionInfo(null);
}

// Set the updateID
sanitizedKeyInfo.setUpdateID(trxnLogIndex);
if (sanitizedKeyInfo != keyInfo) {
keyInfo.setUpdateID(trxnLogIndex);
}
builder.withUpdateID(trxnLogIndex);

//The key doesn't exist in deletedTable, so create a new instance.
return new RepeatedOmKeyInfo(sanitizedKeyInfo, bucketId);
return new RepeatedOmKeyInfo(builder.build(), bucketId);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,12 +509,24 @@ public Builder setObjectID(long obId) {
return this;
}

@Override
public Builder withObjectID(long obId) {
super.withObjectID(obId);
return this;
}

@Override
public Builder setUpdateID(long id) {
super.setUpdateID(id);
return this;
}

@Override
public Builder withUpdateID(long newValue) {
super.withUpdateID(newValue);
return this;
}

@Override
public Builder addMetadata(String key, String value) {
super.addMetadata(key, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public Builder addAllMetadata(Map<String, String> additionalMetadata) {
return this;
}

@Override
public OmDirectoryInfo build() {
return new OmDirectoryInfo(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,18 @@ public Builder setMetadata(Map<String, String> map) {
return this;
}

@Override
public Builder withObjectID(long obId) {
super.withObjectID(obId);
return this;
}

@Override
public Builder withUpdateID(long newValue) {
super.withUpdateID(newValue);
return this;
}

public Builder setFileEncryptionInfo(FileEncryptionInfo feInfo) {
this.encInfo = feInfo;
return this;
Expand Down Expand Up @@ -676,6 +688,12 @@ public Builder setFile(boolean isAFile) {
return this;
}

public Builder setTags(Map<String, String> tags) {
this.tags.clear();
addAllTags(tags);
return this;
}

public Builder addTag(String key, String value) {
tags.put(key, value);
return this;
Expand All @@ -691,6 +709,7 @@ public Builder setExpectedDataGeneration(Long existingGeneration) {
return this;
}

@Override
public OmKeyInfo build() {
return new OmKeyInfo(this);
}
Expand Down Expand Up @@ -942,6 +961,7 @@ public int hashCode() {
/**
* Return a new copy of the object.
*/
@Override
public Builder toBuilder() {
return new Builder(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ public ReplicationConfig getReplicationConfig() {
return replicationConfig;
}

public Builder toBuilder() {
return new Builder(this);
}

/**
* Builder of OmMultipartKeyInfo.
*/
Expand All @@ -236,6 +240,18 @@ public Builder() {
this.partKeyInfoList = new TreeMap<>();
}

public Builder(OmMultipartKeyInfo multipartKeyInfo) {
super(multipartKeyInfo);
this.uploadID = multipartKeyInfo.uploadID;
this.creationTime = multipartKeyInfo.creationTime;
this.replicationConfig = multipartKeyInfo.replicationConfig;
this.partKeyInfoList = new TreeMap<>();
for (PartKeyInfo partKeyInfo : multipartKeyInfo.partKeyInfoMap) {
this.partKeyInfoList.put(partKeyInfo.getPartNumber(), partKeyInfo);
}
this.parentID = multipartKeyInfo.parentID;
}

public Builder setUploadID(String uploadId) {
this.uploadID = uploadId;
return this;
Expand Down Expand Up @@ -277,6 +293,12 @@ public Builder setUpdateID(long id) {
return this;
}

@Override
public Builder withUpdateID(long newValue) {
super.withUpdateID(newValue);
return this;
}

public Builder setParentID(long parentObjId) {
this.parentID = parentObjId;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ public long getUsedNamespace() {
return usedNamespace;
}

public Builder toBuilder() {
return new Builder(this);
}

/**
* Returns new builder class that builds a OmVolumeArgs.
*
Expand Down Expand Up @@ -297,6 +301,18 @@ public Builder setUpdateID(long id) {
return this;
}

@Override
public Builder withObjectID(long obId) {
super.withObjectID(obId);
return this;
}

@Override
public Builder withUpdateID(long newValue) {
super.withUpdateID(newValue);
return this;
}

/**
* Constructs a builder.
*/
Expand All @@ -310,6 +326,20 @@ private Builder(List<OzoneAcl> acls) {
quotaInNamespace = OzoneConsts.QUOTA_RESET;
}

private Builder(OmVolumeArgs omVolumeArgs) {
super(omVolumeArgs);
this.acls = omVolumeArgs.acls;
this.adminName = omVolumeArgs.adminName;
this.ownerName = omVolumeArgs.ownerName;
this.volume = omVolumeArgs.volume;
this.creationTime = omVolumeArgs.creationTime;
this.modificationTime = omVolumeArgs.modificationTime;
this.quotaInBytes = omVolumeArgs.quotaInBytes;
this.quotaInNamespace = omVolumeArgs.quotaInNamespace;
this.usedNamespace = omVolumeArgs.usedNamespace;
this.refCount = omVolumeArgs.refCount;
}

public Builder setAdminName(String admin) {
this.adminName = admin;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public Builder setMetadata(Map<String, String> map) {
return this;
}

protected Map<String, String> getMetadata() {
public Map<String, String> getMetadata() {
return metadata;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,21 @@

import static org.apache.hadoop.ozone.OzoneConsts.OBJECT_ID_RECLAIM_BLOCKS;

import net.jcip.annotations.Immutable;

/**
* Mixin class to handle ObjectID and UpdateID.
*/
@Immutable
public abstract class WithObjectID extends WithMetadata {

private long objectID;
private long updateID;
private final long objectID;
private final long updateID;

protected WithObjectID() {
super();
objectID = 0;
updateID = 0;
}

protected WithObjectID(Builder b) {
Expand Down Expand Up @@ -59,65 +64,6 @@ public final long getUpdateID() {
return updateID;
}

/**
* Set the Object ID.
* There is a reason why we cannot use the final here. The object
* ({@link OmVolumeArgs}/ {@link OmBucketInfo}/ {@link OmKeyInfo}) is
* deserialized from the protobuf in many places in code. We need to set
* this object ID, after it is deserialized.
*
* @param obId - long
*/
public final void setObjectID(long obId) {
if (this.objectID != 0 && obId != OBJECT_ID_RECLAIM_BLOCKS) {
throw new UnsupportedOperationException("Attempt to modify object ID " +
"which is not zero. Current Object ID is " + this.objectID);
}
this.objectID = obId;
}

/**
* Sets the update ID. For each modification of this object, we will set
* this to a value greater than the current value.
*/
public final void setUpdateID(long newValue) {

// Because in non-HA, we have multiple rpc handler threads and
// transactionID is generated in OzoneManagerServerSideTranslatorPB.

// Lets take T1 -> Set Bucket Property
// T2 -> Set Bucket Acl

// Now T2 got lock first, so updateID will be set to 2. Now when T1 gets
// executed we will hit the precondition exception. So for OM non-HA with
// out ratis we should not have this check.

// Same can happen after OM restart also.

// OM Start
// T1 -> Create Bucket
// T2 -> Set Bucket Property

// OM restart
// T1 -> Set Bucket Acl

// So when T1 is executing, Bucket will have updateID 2 which is set by T2
// execution before restart.

// Main reason, in non-HA transaction Index after restart starts from 0.
// And also because of this same reason we don't do replay checks in non-HA.

final long currentValue = getUpdateID();
if (newValue < currentValue) {
throw new IllegalArgumentException(String.format(
"Trying to set updateID to %d which is not greater than the " +
"current value of %d for %s", newValue, currentValue,
getObjectInfo()));
}

updateID = newValue;
}

/** Hook method, customized in subclasses. */
public String getObjectInfo() {
return this.toString();
Expand Down Expand Up @@ -148,6 +94,63 @@ public Builder setObjectID(long obId) {
return this;
}

/**
* Set the Object ID.
* The object ({@link OmVolumeArgs}/ {@link OmBucketInfo}/ {@link OmKeyInfo}) is
* deserialized from the protobuf in many places in code. We need to set
* this object ID, after it is deserialized.
*
* @param obId - long
*/
public Builder withObjectID(long obId) {
if (this.objectID != 0 && obId != OBJECT_ID_RECLAIM_BLOCKS) {
throw new UnsupportedOperationException("Attempt to modify object ID " +
"which is not zero. Current Object ID is " + this.objectID);
}
this.objectID = obId;
return this;
}

/**
* Sets the update ID. For each modification of this object, we will set
* this to a value greater than the current value.
*/
public Builder withUpdateID(long newValue) {
// Because in non-HA, we have multiple rpc handler threads and
// transactionID is generated in OzoneManagerServerSideTranslatorPB.

// Lets take T1 -> Set Bucket Property
// T2 -> Set Bucket Acl

// Now T2 got lock first, so updateID will be set to 2. Now when T1 gets
// executed we will hit the precondition exception. So for OM non-HA with
// out ratis we should not have this check.

// Same can happen after OM restart also.

// OM Start
// T1 -> Create Bucket
// T2 -> Set Bucket Property

// OM restart
// T1 -> Set Bucket Acl

// So when T1 is executing, Bucket will have updateID 2 which is set by T2
// execution before restart.

// Main reason, in non-HA transaction Index after restart starts from 0.
// And also because of this same reason we don't do replay checks in non-HA.
final long currentValue = getUpdateID();
if (newValue < currentValue) {
throw new IllegalArgumentException(String.format(
"Trying to set updateID to %d which is not greater than the " +
"current value of %d for %s", newValue, currentValue,
getObjectInfo()));
}
this.updateID = newValue;
return this;
}

/**
* Sets the update ID for this Object. Update IDs are monotonically
* increasing values which are updated each time there is an update.
Expand All @@ -164,5 +167,10 @@ public long getObjectID() {
public long getUpdateID() {
return updateID;
}

/** Hook method, customized in subclasses. */
public String getObjectInfo() {
Comment on lines +171 to +172
Copy link
Contributor

Choose a reason for hiding this comment

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

getObjectInfo is overridden in value subclasses, but not in builders. What's more, value classes override toString(), but builders do not.

Probably worth a separate sub-task to keep patches from growing huge.

Copy link
Contributor Author

@echonesis echonesis Nov 20, 2025

Choose a reason for hiding this comment

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

Thanks @adoroszlai
I agree with you.
Could I create a subtask under HDDS-10519 since HDDS-13937 has already been a sub-task?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, subtask of HDDS-10519.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've created one sub-task, HDDS-13964, to handle this.

return this.toString();
}
}
}
Loading