HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine #2782

szetszwo · 2021-10-28T07:32:49Z

What changes were proposed in this pull request?

RATIS-1406 added a new API for providing an Executor to StateMachine.DataStream. In this JIRA, we use this new API to provide an Executor for each LocalStream in ContainerStateMachine.

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.

captainzmc

Thanks @szetszwo. Overall LGTM, only a few minor comments. Also there is a problem with checkstyle.

captainzmc · 2021-10-29T02:40:15Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/DatanodeRatisServerConfig.java

    this.clientPoolSize = clientPoolSize;
  }

+  @Config(key = "datastream.async.write.thread.pool.size",


In the previous implementation we added datastream.write.threads, so we just need to reuse the previous key.

captainzmc · 2021-10-29T02:41:07Z

...java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java

+        conf.getObject(DatanodeRatisServerConfig.class)
+            .getAsyncWriteThreadPoolPoolSize();
+    RaftServerConfigKeys.DataStream.setAsyncWriteThreadPoolSize(properties,
+        asyncWriteThreadPoolPoolSize);


This property is already set on line 245, so it not need set here.

Oops, you are right. We already have set it.

…Machine.

szetszwo · 2021-10-29T03:46:21Z

@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.

captainzmc · 2021-11-01T03:48:32Z

...a/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java

+            Paths.get(response.getMessage()));
+        final ExecutorService chunkExecutor = requestProto.hasWriteChunk() ?
+            getChunkExecutor(requestProto.getWriteChunk()) : null;
+        return new LocalStream(channel, chunkExecutor);


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? Now that we're passing in chunkExecutor every time, this configuration should be useless?

@captainzmc , you are right. Let's remove it.

szetszwo · 2021-11-01T09:51:01Z

@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.

captainzmc · 2021-11-01T15:35:42Z

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. Perhaps we can replace it with newCachedThreadPool in RATIS-1419.

captainzmc

+1, change looks good.
The CI of Ozone is very unstable recently. I looked at the failed CI and it had nothing to do with this PR. Let's merge this PR first.

…Machine (#2782)

…Machine (apache#2782)

…Machine (#2782)

…Machine (apache#2782)

…Machine (#2782) (cherry picked from commit e186bfc) (cherry picked from commit 311b36652697155b052cd9781fd1590d22256660)

…Machine (#2782) (cherry picked from commit e186bfc) (cherry picked from commit 311b36652697155b052cd9781fd1590d22256660) (cherry picked from commit 7cbb3fa)

…Machine (#2782) (cherry picked from commit e4a0f00)

…Machine (apache#2782) (cherry picked from commit e4a0f00)

captainzmc self-requested a review October 29, 2021 02:34

captainzmc reviewed Oct 29, 2021

View reviewed changes

szetszwo changed the title ~~HDDS-5763. Set raft.server.data-stream.async.write.thread.pool.size conf in datanode.~~ HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine Oct 29, 2021

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

a1793b4

…Machine.

szetszwo force-pushed the HDDS-5763 branch from 8aeb983 to a1793b4 Compare October 29, 2021 03:44

Fix checkstyle.

ad1f30d

captainzmc reviewed Nov 1, 2021

View reviewed changes

Remove datastream.write.threads from DatanodeRatisServerConfig.

c7628e5

captainzmc approved these changes Nov 1, 2021

View reviewed changes

captainzmc merged commit 871f2cd into HDDS-4454 Nov 1, 2021

szetszwo added a commit that referenced this pull request Nov 15, 2021

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

1d9f870

…Machine (#2782)

szetszwo added a commit that referenced this pull request Nov 30, 2021

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

64c9933

…Machine (#2782)

captainzmc pushed a commit that referenced this pull request Dec 20, 2021

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

38757c7

…Machine (#2782)

szetszwo added a commit that referenced this pull request Dec 30, 2021

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

0872bd8

…Machine (#2782)

szetszwo added a commit that referenced this pull request Jan 20, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

9be29ee

…Machine (#2782)

captainzmc pushed a commit that referenced this pull request Feb 9, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

a4ade67

…Machine (#2782)

szetszwo added a commit that referenced this pull request Feb 16, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

26be29c

…Machine (#2782)

szetszwo added a commit that referenced this pull request Mar 15, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

253ee38

…Machine (#2782)

szetszwo added a commit that referenced this pull request Mar 24, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

e5a281d

…Machine (#2782)

szetszwo added a commit to szetszwo/ozone that referenced this pull request May 6, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

0f5a55a

…Machine (apache#2782)

szetszwo added a commit that referenced this pull request May 13, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

888f948

…Machine (#2782)

szetszwo added a commit that referenced this pull request May 24, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

659397a

…Machine (#2782)

szetszwo added a commit that referenced this pull request Jun 9, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

12cd5d1

…Machine (#2782)

captainzmc pushed a commit to captainzmc/hadoop-ozone that referenced this pull request Jul 4, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

e186bfc

…Machine (apache#2782)

szetszwo added a commit that referenced this pull request Oct 25, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

7cbb3fa

…Machine (#2782) (cherry picked from commit e186bfc) (cherry picked from commit 311b36652697155b052cd9781fd1590d22256660)

szetszwo added a commit that referenced this pull request Dec 1, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

f317382

…Machine (#2782) (cherry picked from commit e4a0f00)

szetszwo added a commit that referenced this pull request Dec 16, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

75efaae

…Machine (#2782) (cherry picked from commit e4a0f00)

nishitpatira pushed a commit to nishitpatira/ozone that referenced this pull request Dec 16, 2022

HDDS-5763. Provide an Executor for each LocalStream in ContainerState…

117d9bb

…Machine (apache#2782) (cherry picked from commit e4a0f00)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine #2782

HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine #2782

Uh oh!

szetszwo commented Oct 28, 2021 •

edited

Loading

Uh oh!

captainzmc left a comment

Uh oh!

captainzmc Oct 29, 2021

Uh oh!

captainzmc Oct 29, 2021

Uh oh!

szetszwo Oct 29, 2021

Uh oh!

szetszwo commented Oct 29, 2021

Uh oh!

captainzmc Nov 1, 2021 •

edited

Loading

Uh oh!

szetszwo Nov 1, 2021

Uh oh!

szetszwo commented Nov 1, 2021

Uh oh!

captainzmc commented Nov 1, 2021

Uh oh!

captainzmc left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine #2782

HDDS-5763. Provide an Executor for each LocalStream in ContainerStateMachine #2782

Uh oh!

Conversation

szetszwo commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

captainzmc left a comment

Choose a reason for hiding this comment

Uh oh!

captainzmc Oct 29, 2021

Choose a reason for hiding this comment

Uh oh!

captainzmc Oct 29, 2021

Choose a reason for hiding this comment

Uh oh!

szetszwo Oct 29, 2021

Choose a reason for hiding this comment

Uh oh!

szetszwo commented Oct 29, 2021

Uh oh!

captainzmc Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szetszwo Nov 1, 2021

Choose a reason for hiding this comment

Uh oh!

szetszwo commented Nov 1, 2021

Uh oh!

captainzmc commented Nov 1, 2021

Uh oh!

captainzmc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

szetszwo commented Oct 28, 2021 •

edited

Loading

captainzmc Nov 1, 2021 •

edited

Loading