[
https://issues.apache.org/jira/browse/HDDS-15193?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chu Cheng Li updated HDDS-15193:
--------------------------------
Description:
[https://github.com/apache/ozone/pull/5524]
Remove the "atomic key creation" logic from the Ozone client output stream
classes and shifts the responsibility for validating S3 object upload content
length from the client layer to the S3 gateway layer. The main effect is that
S3-specific length checks are now enforced in the S3 gateway, not in the
general Ozone client code, leading to a cleaner separation of concerns and more
maintainable code.
was:
[https://github.com/apache/ozone/pull/5524]
{code:java}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
index ee5c754875..9f94384d4d 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
@@ -74,14 +74,6 @@ public final class ECKeyOutputStream extends KeyOutputStream
private final Future<Boolean> flushFuture;
private final AtomicLong flushCheckpoint;
- /**
- * Indicates if an atomic write is required. When set to true,
- * the amount of data written must match the declared size during the commit.
- * A mismatch will prevent the commit from succeeding.
- * This is essential for operations like S3 put to ensure atomicity.
- */
- private boolean atomicKeyCreation;
-
private volatile boolean closed;
private volatile boolean closing;
// how much of data is actually written yet to underlying stream
@@ -130,7 +122,6 @@ private ECKeyOutputStream(Builder builder) {
return flushStripeFromQueue();
});
this.flushCheckpoint = new AtomicLong(0);
- this.atomicKeyCreation = builder.getAtomicKeyCreation();
}
@Override
@@ -489,12 +480,6 @@ public void close() throws IOException {
Preconditions.checkArgument(writeOffset == offset,
"Expected writeOffset= " + writeOffset
+ " Expected offset=" + offset);
- if (atomicKeyCreation) {
- long expectedSize = blockOutputStreamEntryPool.getDataSize();
- Preconditions.checkState(expectedSize == offset, String.format(
- "Expected: %d and actual %d write sizes do not match",
- expectedSize, offset));
- }
for (CheckedRunnable<IOException> preCommit : preCommits) {
preCommit.run();
}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
index ceacd624e9..afdc206ee7 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
@@ -77,14 +77,6 @@ public class KeyDataStreamOutput extends
AbstractDataStreamOutput
private long clientID;
- /**
- * Indicates if an atomic write is required. When set to true,
- * the amount of data written must match the declared size during the commit.
- * A mismatch will prevent the commit from succeeding.
- * This is essential for operations like S3 put to ensure atomicity.
- */
- private boolean atomicKeyCreation;
-
private List<CheckedRunnable<IOException>> preCommits =
Collections.emptyList();
public void setPreCommits(@Nonnull List<CheckedRunnable<IOException>>
preCommits) {
@@ -129,7 +121,6 @@ public KeyDataStreamOutput() {
this.writeOffset = 0;
this.clientID = 0L;
- this.atomicKeyCreation = false;
}
@SuppressWarnings({"parameternumber", "squid:S00107"})
@@ -140,9 +131,7 @@ public KeyDataStreamOutput(
OzoneManagerProtocol omClient, int chunkSize,
String requestId, ReplicationConfig replicationConfig,
String uploadID, int partNumber, boolean isMultipart,
- boolean unsafeByteBufferConversion,
- boolean atomicKeyCreation
- ) {
+ boolean unsafeByteBufferConversion) {
super(HddsClientUtils.getRetryPolicyByException(
config.getMaxRetryCount(), config.getRetryInterval()));
this.config = config;
@@ -162,7 +151,6 @@ public KeyDataStreamOutput(
// encrypted bucket.
this.writeOffset = 0;
this.clientID = handler.getId();
- this.atomicKeyCreation = atomicKeyCreation;
}
/**
@@ -457,12 +445,6 @@ public void close() throws IOException {
if (!isException()) {
Preconditions.checkArgument(writeOffset == offset);
}
- if (atomicKeyCreation) {
- long expectedSize = blockDataStreamOutputEntryPool.getDataSize();
- Preconditions.checkArgument(expectedSize == offset,
- String.format("Expected: %d and actual %d write sizes do not
match",
- expectedSize, offset));
- }
for (CheckedRunnable<IOException> preCommit : preCommits) {
preCommit.run();
}
@@ -501,7 +483,6 @@ public static class Builder {
private boolean unsafeByteBufferConversion;
private OzoneClientConfig clientConfig;
private ReplicationConfig replicationConfig;
- private boolean atomicKeyCreation = false;
public Builder setMultipartUploadID(String uploadID) {
this.multipartUploadID = uploadID;
@@ -553,11 +534,6 @@ public Builder setReplicationConfig(ReplicationConfig
replConfig) {
return this;
}
- public Builder setAtomicKeyCreation(boolean atomicKey) {
- this.atomicKeyCreation = atomicKey;
- return this;
- }
-
public KeyDataStreamOutput build() {
return new KeyDataStreamOutput(
clientConfig,
@@ -570,8 +546,7 @@ public KeyDataStreamOutput build() {
multipartUploadID,
multipartNumber,
isMultipartKey,
- unsafeByteBufferConversion,
- atomicKeyCreation);
+ unsafeByteBufferConversion);
}
}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
index 2f9edfa94e..791709eb52 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
@@ -98,13 +98,6 @@ public class KeyOutputStream extends OutputStream
private long clientID;
private StreamBufferArgs streamBufferArgs;
- /**
- * Indicates if an atomic write is required. When set to true,
- * the amount of data written must match the declared size during the commit.
- * A mismatch will prevent the commit from succeeding.
- * This is essential for operations like S3 put to ensure atomicity.
- */
- private boolean atomicKeyCreation;
private ContainerClientMetrics clientMetrics;
private OzoneManagerVersion ozoneManagerVersion;
private final Lock writeLock = new ReentrantLock();
@@ -186,7 +179,6 @@ public KeyOutputStream(Builder b) {
this.isException = false;
this.writeOffset = 0;
this.clientID = b.getOpenHandler().getId();
- this.atomicKeyCreation = b.getAtomicKeyCreation();
this.streamBufferArgs = b.getStreamBufferArgs();
this.clientMetrics = b.getClientMetrics();
this.ozoneManagerVersion = b.ozoneManagerVersion;
@@ -656,12 +648,6 @@ private void closeInternal() throws IOException {
if (!isException) {
Preconditions.checkArgument(writeOffset == offset);
}
- if (atomicKeyCreation) {
- long expectedSize = blockOutputStreamEntryPool.getDataSize();
- Preconditions.checkState(expectedSize == offset,
- String.format("Expected: %d and actual %d write sizes do not
match",
- expectedSize, offset));
- }
for (CheckedRunnable<IOException> preCommit : preCommits) {
preCommit.run();
}
@@ -701,7 +687,6 @@ public static class Builder {
private OzoneClientConfig clientConfig;
private ReplicationConfig replicationConfig;
private ContainerClientMetrics clientMetrics;
- private boolean atomicKeyCreation = false;
private StreamBufferArgs streamBufferArgs;
private Supplier<ExecutorService> executorServiceSupplier;
private OzoneManagerVersion ozoneManagerVersion;
@@ -800,11 +785,6 @@ public Builder setReplicationConfig(ReplicationConfig
replConfig) {
return this;
}
- public Builder setAtomicKeyCreation(boolean atomicKey) {
- this.atomicKeyCreation = atomicKey;
- return this;
- }
-
public Builder setClientMetrics(ContainerClientMetrics clientMetrics) {
this.clientMetrics = clientMetrics;
return this;
@@ -814,10 +794,6 @@ public ContainerClientMetrics getClientMetrics() {
return clientMetrics;
}
- public boolean getAtomicKeyCreation() {
- return atomicKeyCreation;
- }
-
public Builder setExecutorServiceSupplier(Supplier<ExecutorService>
executorServiceSupplier) {
this.executorServiceSupplier = executorServiceSupplier;
return this;
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
index c52ac5d188..007f7fdc64 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -2579,7 +2579,6 @@ private OzoneDataStreamOutput
createDataStreamOutput(OpenKeySession openKey,
KeyDataStreamOutput keyOutputStream = newKeyOutputStreamBuilder()
.setHandler(openKey)
.setReplicationConfig(replicationConfig)
- .setAtomicKeyCreation(isS3GRequest.get() && hasKnownLength)
.build();
keyOutputStream.addPreallocateBlocks(
openKey.getKeyInfo().getLatestVersionLocations(),
@@ -2594,15 +2593,12 @@ private OzoneDataStreamOutput
createDataStreamOutput(OpenKeySession openKey,
}
private KeyDataStreamOutput.Builder newKeyOutputStreamBuilder() {
- // Amazon S3 never adds partial objects, So for S3 requests we need to
- // set atomicKeyCreation to true
// refer:
https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html
return new KeyDataStreamOutput.Builder()
.setXceiverClientManager(xceiverClientManager)
.setOmClient(ozoneManagerClient)
.enableUnsafeByteBufferConversion(unsafeByteBufferConversion)
- .setConfig(clientConfig)
- .setAtomicKeyCreation(isS3GRequest.get());
+ .setConfig(clientConfig);
}
private OzoneOutputStream createOutputStream(OpenKeySession openKey)
@@ -2687,7 +2683,6 @@ private KeyOutputStream.Builder createKeyOutputStream(
.setOmClient(ozoneManagerClient)
.enableUnsafeByteBufferConversion(unsafeByteBufferConversion)
.setConfig(clientConfig)
- .setAtomicKeyCreation(isS3GRequest.get() && hasKnownLength)
.setClientMetrics(clientMetrics)
.setExecutorServiceSupplier(writeExecutor)
.setStreamBufferArgs(streamBufferArgs)
{code}
> Move the "atomic key creation" logic from output stream to S3 endpoints
> -----------------------------------------------------------------------
>
> Key: HDDS-15193
> URL: https://issues.apache.org/jira/browse/HDDS-15193
> Project: Apache Ozone
> Issue Type: Sub-task
> Reporter: Chu Cheng Li
> Assignee: Chu Cheng Li
> Priority: Major
> Labels: pull-request-available
>
> [https://github.com/apache/ozone/pull/5524]
>
> Remove the "atomic key creation" logic from the Ozone client output stream
> classes and shifts the responsibility for validating S3 object upload content
> length from the client layer to the S3 gateway layer. The main effect is that
> S3-specific length checks are now enforced in the S3 gateway, not in the
> general Ozone client code, leading to a cleaner separation of concerns and
> more maintainable code.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]