Uh oh!
There was an error while loading. Please reload this page.
HDDS-10383. Introduce a Provider for client-side thread resources passing - #6222
Conversation
| * Provides resources for BlockOutputStream, including executor service, | ||
| * and client metrics. | ||
| */ | ||
| public final class BlockOutputStreamResourceProvider { |
There was a problem hiding this comment.
It seems adding this new BlockOutputStreamResourceProvider is for parameter passing instead of add a new executorServiceSupplier parameter. However, executorServiceSupplier and clientMetrics are not related.
Let's pass the builders; filed HDDS-10387.
There was a problem hiding this comment.
This class only has executorServiceSupplier. Let's pass it directly.
| public ExecutorService getWriteThreadPool() { | ||
| ExecutorService localRef = writeExecutor; | ||
| if (localRef == null) { | ||
| synchronized (this) { | ||
| localRef = writeExecutor; | ||
| if (localRef == null) { | ||
| localRef = createThreadPoolExecutor(WRITE_POOL_MIN_SIZE, | ||
| Integer.MAX_VALUE, | ||
| "client-write-TID-%d"); | ||
| writeExecutor = localRef; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Use MemoizedSupplier:
diff --gita/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.javab/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.javaindex94d6ae9769..abb981caa9100644
--- 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@@ -145,6 +145,7@@importorg.apache.hadoop.security.UserGroupInformation;
importorg.apache.hadoop.security.token.Token;
importorg.apache.ratis.protocol.ClientId;
+importorg.apache.ratis.util.MemoizedSupplier;
importorg.slf4j.Logger;
importorg.slf4j.LoggerFactory;
@@ -195,6 +196,9@@publicclassRpcClientimplementsClientProtocol {
// for reconstruction.privatestaticfinalintEC_RECONSTRUCT_STRIPE_READ_POOL_MIN_SIZE = 3;
+ // TODO: Adjusts to the appropriate value when the writeThreadPool is used.
+ privatestaticfinal int WRITE_POOL_MIN_SIZE = 0;
+
privatefinalConfigurationSourceconf;
privatefinalOzoneManagerClientProtocolozoneManagerClient;
privatefinalXceiverClientFactoryxceiverClientManager;
@@ -213,7 +217,8@@publicclassRpcClientimplementsClientProtocol {
privatefinalByteBufferPoolbyteBufferPool;
privatefinalBlockInputStreamFactoryblockInputStreamFactory;
privatefinalOzoneManagerVersionomVersion;
- privatevolatileExecutorServiceecReconstructExecutor;
+ privatefinalMemoizedSupplier<ExecutorService> ecReconstructExecutor;
+ privatefinalMemoizedSupplier<ExecutorService> writeExecutor;
privatefinalContainerClientMetricsclientMetrics;
privatefinalAtomicBooleanisS3GRequest = newAtomicBoolean(false);
@@ -237,6 +242,12@@publicRpcClient(ConfigurationSourceconf, StringomServiceId)
this.groupRights = aclConfig.getGroupDefaultRights();
this.clientConfig = conf.getObject(OzoneClientConfig.class);
+ this.ecReconstructExecutor = MemoizedSupplier.valueOf(() -> createThreadPoolExecutor(
+ EC_RECONSTRUCT_STRIPE_READ_POOL_MIN_SIZE,
+ clientConfig.getEcReconstructStripeReadPoolLimit(),
+ "ec-reconstruct-reader-TID-%d"));
+ this.writeExecutor = MemoizedSupplier.valueOf(() -> createThreadPoolExecutor(
+ WRITE_POOL_MIN_SIZE, Integer.MAX_VALUE, "client-write-TID-%d"));
OmTransportomTransport = createOmTransport(omServiceId);
OzoneManagerProtocolClientSideTranslatorPB@@ -311,7 +322,7@@publicvoidonRemoval(
}).build();
this.byteBufferPool = newElasticByteBufferPool();
this.blockInputStreamFactory = BlockInputStreamFactoryImpl
- .getInstance(byteBufferPool, this::getECReconstructExecutor);
+ .getInstance(byteBufferPool, ecReconstructExecutor);
this.clientMetrics = ContainerClientMetrics.acquire();
}
@@ -1752,9 +1763,11@@privateOmKeyInfogetKeyInfo(OmKeyArgskeyArgs) throwsIOException {
@Overridepublicvoidclose() throwsIOException {
- if (ecReconstructExecutor != null) {
- ecReconstructExecutor.shutdownNow();
- ecReconstructExecutor = null;
+ if (ecReconstructExecutor.isInitialized()) {
+ ecReconstructExecutor.get().shutdownNow();
+ }
+ if (writeExecutor.isInitialized()) {
+ writeExecutor.get().shutdownNow();
}
IOUtils.cleanupWithLogger(LOG, ozoneManagerClient, xceiverClientManager);
keyProviderCache.invalidateAll();
@@ -2496,26 +2509,11@@publicvoidsetTimes(OzoneObjobj, StringkeyName, longmtime, longatime)
ozoneManagerClient.setTimes(builder.build(), mtime, atime);
}
- publicExecutorServicegetECReconstructExecutor() {
- // local ref to a volatile to ensure access
- // to a completed initialized object
- ExecutorServiceexecutor = ecReconstructExecutor;
- if (executor == null) {
- synchronized (this) {
- executor = ecReconstructExecutor;
- if (executor == null) {
- ecReconstructExecutor = newThreadPoolExecutor(
- EC_RECONSTRUCT_STRIPE_READ_POOL_MIN_SIZE,
- clientConfig.getEcReconstructStripeReadPoolLimit(),
- 60, TimeUnit.SECONDS, newSynchronousQueue<>(),
- newThreadFactoryBuilder()
- .setNameFormat("ec-reconstruct-reader-TID-%d")
- .build(),
- newThreadPoolExecutor.CallerRunsPolicy());
- executor = ecReconstructExecutor;
- }
- }
- }
- returnexecutor;
+ privatestaticExecutorServicecreateThreadPoolExecutor(
+ intcorePoolSize, intmaximumPoolSize, StringthreadNameFormat) {
+ returnnewThreadPoolExecutor(corePoolSize, maximumPoolSize,
+ 60, TimeUnit.SECONDS, newSynchronousQueue<>(),
+ newThreadFactoryBuilder().setNameFormat(threadNameFormat).build(),
+ newThreadPoolExecutor.CallerRunsPolicy());
}
}# Conflicts: # hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java # hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntryPool.java # hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntry.java # hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntryPool.java # hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java # hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java # hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneOutputStreamStub.java
szetszwo
left a comment
There was a problem hiding this comment.
@xichen01 , thanks for the update! Please see the comment inlined and also https://issues.apache.org/jira/secure/attachment/13066945/6222_review.patch
| * Provides resources for BlockOutputStream, including executor service, | ||
| * and client metrics. | ||
| */ | ||
| public final class BlockOutputStreamResourceProvider { |
There was a problem hiding this comment.
This class only has executorServiceSupplier. Let's pass it directly.
xichen01
commented
Feb 22, 2024
Thanks for your comment. I have applied it. |
szetszwo
left a comment
There was a problem hiding this comment.
+1 the change looks good.
…sing (apache#6222) (cherry picked from commit f0b75b7)
What changes were proposed in this pull request?
Introduce a Provider for client-side thread resources passing.
Detail: HDDS-9912
The changes here are split from HDDS-9912, which just introduces
BlockOutputStreamResourceProviderand implements its parameter passing.The changes doesn't actually use
BlockOutputStreamResourceProvider's thread resources. So the logic of creating and using threads inBlockOutputStreamremains unchanged.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10383
How was this patch tested?
Existing Test.