HDDS-9323. Apply expiry of excluded datanodes to writing Ratis keys by DaveTeng0 · Pull Request #5530 · apache/ozone

DaveTeng0 · 2023-11-01T21:12:00Z

What changes were proposed in this pull request?

Currently it is possible that a long lived client can add most or all nodes of a small cluster to its exclude list, and further writes using that client instance will fail. This PR add a timeout for RATIS key to remove datanodes from the exclude list so that they can be retried later.
(For EC key, this mechanism exists and is configured to 10 minutes by default.)

This improvement is especially relevant for S3 gateway, which uses a persistent Ozone client to connect to the cluster.

And there is another part of improvement that allows the write to fall back to datanodes in the exclude list if that is all available. This would be implemented as a retry from the client based on the server's initial error response.

This part of work is separated into another PR: #5514

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-9323

How was this patch tested?

unit test

sumitagrawl

@DaveTeng0 Thanks for working over this, have minor comments, please check

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java

…ethod in BlockOutputStreamEntryPool to use constructor of ExcludeList with clock

sumitagrawl

@DaveTeng0 Give few review comments

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/OzoneClientConfig.java

hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestKeyOutputStream.java

adoroszlai · 2023-11-10T16:53:12Z

@DaveTeng0 please check integration test failures:

org.apache.hadoop.ozone.client.rpc.TestFailureHandlingByClient
org.apache.hadoop.ozone.client.rpc.TestFailureHandlingByClientFlushDelay

https://github.com/DaveTeng0/ozone/actions/runs/6818659142/job/18545006173

DaveTeng0 · 2023-11-10T17:00:52Z

@DaveTeng0 please check integration test failures:
org.apache.hadoop.ozone.client.rpc.TestFailureHandlingByClient
org.apache.hadoop.ozone.client.rpc.TestFailureHandlingByClientFlushDelay
https://github.com/DaveTeng0/ozone/actions/runs/6818659142/job/18545006173

yup thanks attila! These two tests failure are on my radar!

…lush()

hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestKeyOutputStream.java

… exclude list on client side

DaveTeng0 · 2023-11-18T00:09:25Z

note: there are unit test for ExcludeList.java already in https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/container/common/helpers/TestExcludeList.java#L32

adoroszlai

I think we should apply the same in streaming write:

ozone/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockDataStreamOutputEntryPool.java

Line 86 in 2085ebe

this.excludeList = new ExcludeList();

sumitagrawl

@DaveTeng0 Have minor comment

sumitagrawl · 2023-11-20T13:21:31Z

...ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntryPool.java

  ExcludeList createExcludeList() {
-    return new ExcludeList();
+    return new ExcludeList(getConfig().getExcludeNodesExpiryTime(),
+        Clock.system(ZoneOffset.UTC));


new use createExcludeList() for constructor BlockOutputStreamEntryPool() at line 140 as used.
https://github.com/apache/ozone/pull/5530/files#diff-db8e4914806b73e8ec37c15ffd146fc70bd1fcba631df9a314c7389af64a895dL140

yeah updated.

DaveTeng0 · 2023-11-20T18:19:20Z

I think we should apply the same in streaming write:

ozone/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockDataStreamOutputEntryPool.java

Line 86 in 2085ebe

this.excludeList = new ExcludeList();

oh yes you're right! updated.

adoroszlai · 2023-11-20T19:43:44Z

@DaveTeng0 please merge master into your branch, it's 106 commits behind, and there is a conflict

DaveTeng0 · 2023-11-20T23:48:21Z

@DaveTeng0 please merge master into your branch, it's 106 commits behind, and there is a conflict

oh sure! working on it!

adoroszlai

Thanks @DaveTeng0 for updating the patch, LGTM.

adoroszlai · 2023-11-21T16:23:13Z

Thanks @DaveTeng0 for the patch, @sumitagrawl for the review.

…Ratis keys (apache#5530) (cherry picked from commit cddc571) Change-Id: I4a2e1d12c1bfb6593a43850869a9b24dabcb1d2a

DaveTeng0 added 2 commits November 1, 2023 13:59

HDDS-9323. Better datanode exclude list handling for long-lived clients

a82ac65

fix checkstyle error

64f92d4

aswinshakil requested review from duongkame, errose28, kerneltime and swamirishi November 6, 2023 17:27

fix findbugs/rat error

77b726d

sumitagrawl reviewed Nov 7, 2023

View reviewed changes

DaveTeng0 added 2 commits November 7, 2023 21:06

fix findbugs error; add VisibleForTesting; change createExcludeList m…

9102016

…ethod in BlockOutputStreamEntryPool to use constructor of ExcludeList with clock

fix checkstyle error

1f43950

sumitagrawl reviewed Nov 9, 2023

View reviewed changes

removed unused method

da929b2

remove adding failedServers of DN in method BlockOutputStream.handleF…

ffe80d9

…lush()

adoroszlai reviewed Nov 15, 2023

View reviewed changes

hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestKeyOutputStream.java Outdated Show resolved Hide resolved

DaveTeng0 added 2 commits November 17, 2023 11:09

update TestFailureHandlingByClient test with asserting expire time of…

3734b45

… exclude list on client side

remove empty line

583087a

adoroszlai changed the title ~~HDDS-9323. Better datanode exclude list handling for long-lived clients~~ HDDS-9323. Apply expiry of excluded datanodes to writing Ratis keys Nov 18, 2023

adoroszlai reviewed Nov 18, 2023

View reviewed changes

sumitagrawl reviewed Nov 20, 2023

View reviewed changes

Update BlockDataStreamOutputEntryPool

9224785

Merge branch 'master' into HDDS-9323

a0acdc2

adoroszlai reviewed Nov 21, 2023

View reviewed changes

adoroszlai requested a review from sumitagrawl November 21, 2023 08:13

adoroszlai merged commit cddc571 into apache:master Nov 21, 2023

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 1, 2024

CDPD-61539. HDDS-9323. Apply expiry of excluded datanodes to writing …

81e646a

…Ratis keys (apache#5530) (cherry picked from commit cddc571) Change-Id: I4a2e1d12c1bfb6593a43850869a9b24dabcb1d2a

Comments

Conversation

DaveTeng0 commented Nov 1, 2023

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adoroszlai commented Nov 10, 2023

Uh oh!

DaveTeng0 commented Nov 10, 2023

Uh oh!

Uh oh!

DaveTeng0 commented Nov 18, 2023

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

sumitagrawl Nov 20, 2023

Choose a reason for hiding this comment

Uh oh!

DaveTeng0 Nov 20, 2023

Choose a reason for hiding this comment

Uh oh!

DaveTeng0 commented Nov 20, 2023

Uh oh!

adoroszlai commented Nov 20, 2023

Uh oh!

DaveTeng0 commented Nov 20, 2023

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

adoroszlai commented Nov 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants