You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/10/28 07:32:56 UTC
[GitHub] [ozone] szetszwo opened a new pull request #2782: HDDS-5763. Set raft.server.data-stream.async.write.thread.pool.size conf in datanode.
szetszwo opened a new pull request #2782:
URL: https://github.com/apache/ozone/pull/2782
## What changes were proposed in this pull request?
The Ratis conf raft.server.data-stream.async.write.thread.pool.size default is only 16. In Ozone, it should be configured to a similar value as dfs.container.ratis.num.write.chunk.threads.per.volume.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5763
## How was this patch tested?
No new tests are needed.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc commented on pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#issuecomment-956338966
> BTW, filed https://issues.apache.org/jira/browse/RATIS-1419 for changing the fixed thread pools in DataStreamManagement to cached thread pools in order to reduce the resources usage when the executors are idle.
Good idea, Currently ContainerStateMachine also uses [newFixedThreadPool](https://github.com/apache/ozone/blob/HDDS-4454/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java#L199). Perhaps we can replace it with newCachedThreadPool in RATIS-1419.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #2782: HDDS-5763. Set raft.server.data-stream.async.write.thread.pool.size conf in datanode.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#discussion_r738895554
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -249,6 +249,11 @@ private void setUpRatisStream(RaftProperties properties) {
.getClientPoolSize();
RaftServerConfigKeys.DataStream.setClientPoolSize(properties,
dataStreamClientPoolSize);
+ final int asyncWriteThreadPoolPoolSize =
+ conf.getObject(DatanodeRatisServerConfig.class)
+ .getAsyncWriteThreadPoolPoolSize();
+ RaftServerConfigKeys.DataStream.setAsyncWriteThreadPoolSize(properties,
+ asyncWriteThreadPoolPoolSize);
Review comment:
This property is already set on line 245, so it not need set here.
##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/DatanodeRatisServerConfig.java
##########
@@ -175,6 +175,22 @@ public void setClientPoolSize(int clientPoolSize) {
this.clientPoolSize = clientPoolSize;
}
+ @Config(key = "datastream.async.write.thread.pool.size",
Review comment:
In the previous implementation we added datastream.write.threads, so we just need to reuse the previous key.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc merged pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc merged pull request #2782:
URL: https://github.com/apache/ozone/pull/2782
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on a change in pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#discussion_r740066232
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
##########
@@ -515,8 +515,11 @@ private ContainerCommandResponseProto runCommand(
ContainerCommandResponseProto response = runCommand(
requestProto, context);
- String path = response.getMessage();
- return new LocalStream(new StreamDataChannel(Paths.get(path)));
+ final StreamDataChannel channel = new StreamDataChannel(
+ Paths.get(response.getMessage()));
+ final ExecutorService chunkExecutor = requestProto.hasWriteChunk() ?
+ getChunkExecutor(requestProto.getWriteChunk()) : null;
+ return new LocalStream(channel, chunkExecutor);
Review comment:
@captainzmc , you are right. Let's remove it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#issuecomment-956085823
@captainzmc , just have pushed a new commit. Please take a look.
BTW, filed https://issues.apache.org/jira/browse/RATIS-1419 for changing the fixed thread pools in DataStreamManagement to cached thread pools in order to reduce the resources usage when the executors are idle.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc merged pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc merged pull request #2782:
URL: https://github.com/apache/ozone/pull/2782
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#issuecomment-954406450
@captainzmc , thanks a lot for reviewing this and pointing out that raft.server.data-stream.async.write.thread.pool.size has already been set in XceiverServerRatis.
Just have updated the JIRA, this pull request and pushed a new commit.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#discussion_r739940860
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
##########
@@ -515,8 +515,11 @@ private ContainerCommandResponseProto runCommand(
ContainerCommandResponseProto response = runCommand(
requestProto, context);
- String path = response.getMessage();
- return new LocalStream(new StreamDataChannel(Paths.get(path)));
+ final StreamDataChannel channel = new StreamDataChannel(
+ Paths.get(response.getMessage()));
+ final ExecutorService chunkExecutor = requestProto.hasWriteChunk() ?
+ getChunkExecutor(requestProto.getWriteChunk()) : null;
+ return new LocalStream(channel, chunkExecutor);
Review comment:
Hi @szetszwo, The size of the chunkExecutor is determined by dfs.container.ratis.num.write.chunk.threads.per.volume. So I'm wondering if we need delete [datastream.write.threads in DatanodeRatisServerConfig](https://github.com/apache/ozone/blob/HDDS-4454/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/DatanodeRatisServerConfig.java#L144)? Now that we're passing in chunkExecutor every time, this configuration should be useless.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#discussion_r739940860
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
##########
@@ -515,8 +515,11 @@ private ContainerCommandResponseProto runCommand(
ContainerCommandResponseProto response = runCommand(
requestProto, context);
- String path = response.getMessage();
- return new LocalStream(new StreamDataChannel(Paths.get(path)));
+ final StreamDataChannel channel = new StreamDataChannel(
+ Paths.get(response.getMessage()));
+ final ExecutorService chunkExecutor = requestProto.hasWriteChunk() ?
+ getChunkExecutor(requestProto.getWriteChunk()) : null;
+ return new LocalStream(channel, chunkExecutor);
Review comment:
Hi @szetszwo, The size of the chunkExecutors is determined by dfs.container.ratis.num.write.chunk.threads.per.volume. So I'm wondering if we need delete [datastream.write.threads in DatanodeRatisServerConfig](https://github.com/apache/ozone/blob/HDDS-4454/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/DatanodeRatisServerConfig.java#L144)? Now that we're passing in chunkExecutor every time, this configuration should be useless?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on a change in pull request #2782: HDDS-5763. Set raft.server.data-stream.async.write.thread.pool.size conf in datanode.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#discussion_r738908789
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -249,6 +249,11 @@ private void setUpRatisStream(RaftProperties properties) {
.getClientPoolSize();
RaftServerConfigKeys.DataStream.setClientPoolSize(properties,
dataStreamClientPoolSize);
+ final int asyncWriteThreadPoolPoolSize =
+ conf.getObject(DatanodeRatisServerConfig.class)
+ .getAsyncWriteThreadPoolPoolSize();
+ RaftServerConfigKeys.DataStream.setAsyncWriteThreadPoolSize(properties,
+ asyncWriteThreadPoolPoolSize);
Review comment:
Oops, you are right. We already have set it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc commented on pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#issuecomment-956338966
> BTW, filed https://issues.apache.org/jira/browse/RATIS-1419 for changing the fixed thread pools in DataStreamManagement to cached thread pools in order to reduce the resources usage when the executors are idle.
Good idea, Currently ContainerStateMachine also uses [newFixedThreadPool](https://github.com/apache/ozone/blob/HDDS-4454/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java#L199). Perhaps we can replace it with newCachedThreadPool in RATIS-1419.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #2782: HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2782:
URL: https://github.com/apache/ozone/pull/2782#discussion_r739940860
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
##########
@@ -515,8 +515,11 @@ private ContainerCommandResponseProto runCommand(
ContainerCommandResponseProto response = runCommand(
requestProto, context);
- String path = response.getMessage();
- return new LocalStream(new StreamDataChannel(Paths.get(path)));
+ final StreamDataChannel channel = new StreamDataChannel(
+ Paths.get(response.getMessage()));
+ final ExecutorService chunkExecutor = requestProto.hasWriteChunk() ?
+ getChunkExecutor(requestProto.getWriteChunk()) : null;
+ return new LocalStream(channel, chunkExecutor);
Review comment:
Hi @szetszwo, The size of the chunkExecutors is determined by dfs.container.ratis.num.write.chunk.threads.per.volume. So I'm wondering if we need delete [datastream.write.threads in DatanodeRatisServerConfig](https://github.com/apache/ozone/blob/HDDS-4454/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/DatanodeRatisServerConfig.java#L144)? Now that we're passing in chunkExecutor every time, this configuration should be useless.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org