HDDS-9752. [hsync] Make Putblock performance acceptable - Client side #5663

jojochuang · 2023-11-22T20:15:56Z

What changes were proposed in this pull request?

Implement Client side change for HDDS-8047 incremental chunk list for PutBlock. See HDDS-8047 for the problem description and design doc.

Please describe your PR in detail:

This PR depends on HDDS-9750 / HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton code #5661 and HDDS-9751 / HDDS-9751. [hsync] Make Putblock performance acceptable - DataNode side #5662
Updates BlockOutputStream WriteChunk and PutBlock to product the incremental chunk list. See
PutBlock metadata optimization.pdf for implementation details.
New test cases TestBlockOutputStreamIncrementalPutBlock

What is the link to the Apache JIRA

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

How was this patch tested?

New unit tests TestBlockOutputStreamIncrementalPutBlock
Full cluster HBase ycsb workload test

ChenSammi · 2024-01-26T07:51:58Z

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

+      KeyValue.newBuilder().setKey(INCREMENTAL_CHUNK_LIST).build();
+  public static final String FULL_CHUNK = "full";
+  public static final KeyValue FULL_CHUNK_KV =
+      KeyValue.newBuilder().setKey(FULL_CHUNK).build();


Can you put INCREMENTAL_CHUNK_LIST, FULL_CHUNK in some common places. I remember there is one definition too in BlockManagerImpl.

This FULL_CHUNK_KV metadata is not welly put on chunks with full content, which cause the datanode putBlockByID process the blockData incorrectly.

ChenSammi · 2024-01-26T08:02:37Z

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java

    CONF.setBoolean(OZONE_OM_RATIS_ENABLE_KEY, false);
    CONF.set(OZONE_DEFAULT_BUCKET_LAYOUT, layout.name());
    CONF.setBoolean(OzoneConfigKeys.OZONE_FS_HSYNC_ENABLED, true);
+    CONF.setBoolean("ozone.client.incremental.chunk.list", true);


@jojochuang , we have OZONE_CHUNK_LIST_INCREMENTAL "ozone.chunk.list.incremental" to control the Datanode, and "ozone.client.incremental.chunk.list" to control the client. In long term I think the datanode side incremental chunk list shall be by default supported.

And here the "chunk.list.incremental" and "incremental.chunk.list" are used, can we just use one of them, maybe "incremental.chunk.list" ?

I had to make it false by default to pass compatibility tests.
Ideally, a new client would detect DataNode version and then fallback to full chunk list. However, due to https://issues.apache.org/jira/browse/HDDS-9884, this is currently not possible and compat test would fail.

I can revisit this after HDDS-9884 is fixed.

Filed HDDS-10252 to track this task.

ChenSammi · 2024-01-26T09:24:17Z

...t/src/test/java/org/apache/hadoop/ozone/client/TestBlockOutputStreamIncrementalPutBlock.java

+        ReplicationConfig.getDefault(config), new HashMap<>())) {
+      for (int i = 0; i < 4097; i++) {
+        out.write(byteBuffer);
+        out.hsync();


hsync is not enabled in this test. So the hsync call just fallback to flush call, which will call writeChunk and putBlock on the 4M boundary.

good catch and realized we need to enable the flag in TestHSync too.

ChenSammi · 2024-01-26T09:43:31Z

...t/src/test/java/org/apache/hadoop/ozone/client/TestBlockOutputStreamIncrementalPutBlock.java

+    try (OzoneInputStream is = bucket.readKey(keyName)) {
+      ByteBuffer readBuffer = ByteBuffer.allocate(size);
+      for (int i = 0; i < 4; i++) {
+        is.read(readBuffer);


I added one check here, it failed

int readLen = is.read(readBuffer); assertEquals(bufferSize, readLen);

Can you use a MiniOzoneCluster to test the function. I'm afraid that this MockXceiverClientFactory cannot fully test the integration of client side and DN side code.

TestHsync uses MiniOzoneCluster to test so that aspect is covered.

For this particular test file, part of the desire was to complete the unit test framework.

Change-Id: I2f5d42e02401b125694552c94541d19f2adca53c Add missing test cases. Change-Id: Ida6c78c35baa8a6665a4e7a0d99f57a9841b64aa Fix compilation error. Checkstyle Checkstyle Fix tests using the correct JUnit 5 semantics. (cherry picked from commit 2443c5f416aa173015357d148bc11b4bb11eb066) Checkstyle Fix test and checkstyle

jojochuang · 2024-01-31T18:11:59Z

@ashishkumar50 please review

...t/src/test/java/org/apache/hadoop/ozone/client/TestBlockOutputStreamIncrementalPutBlock.java

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

ChenSammi · 2024-02-05T05:13:07Z

Thanks @jojochuang for update the patch. Could you check the failed TestBlockManagerImpl? Looks relevant.

(cherry picked from commit 2938b69)

jojochuang · 2024-02-05T18:59:32Z

Yes -- turns out that the test needs to enable ozone.chunk.list.incremental

ChenSammi

LGTM +1, thanks @jojochuang .

jojochuang · 2024-02-07T04:07:26Z

Thanks @ChenSammi merged. Thanks @ashishkumar50 too!

…ent side (apache#5663)" This reverts commit 76a573a.

…apache#5663)

jojochuang requested a review from ChenSammi November 22, 2023 20:16

jojochuang changed the title ~~Hdds 9752~~ HDDS-9752. [hsync] Make Putblock performance acceptable - Client side Nov 22, 2023

aswinshakil requested review from duongkame and szetszwo November 27, 2023 17:09

jojochuang force-pushed the HDDS-9752 branch 5 times, most recently from 148b2e4 to 71285f1 Compare December 21, 2023 19:08

jojochuang force-pushed the HDDS-9752 branch 3 times, most recently from ee88612 to ee220ab Compare January 5, 2024 04:48

jojochuang marked this pull request as ready for review January 5, 2024 19:03

adoroszlai added the performance label Jan 14, 2024

adoroszlai mentioned this pull request Jan 14, 2024

HDDS-8047. [hsync] Make Putblock performance acceptable #5628

Closed

jojochuang force-pushed the HDDS-7593 branch from 074b6cc to 1c20d84 Compare January 17, 2024 14:37

jojochuang force-pushed the HDDS-9752 branch 2 times, most recently from 1859bb8 to 4d36522 Compare January 23, 2024 18:35

jojochuang added the hbase HBase on Ozone support label Jan 23, 2024

ChenSammi reviewed Jan 26, 2024

View reviewed changes

jojochuang requested a review from ChenSammi January 30, 2024 22:09

jojochuang added 3 commits January 31, 2024 08:46

Simplify block output stream.

af53756

Address review comments.

aba2f1d

jojochuang force-pushed the HDDS-9752 branch from 3819c7f to aba2f1d Compare January 31, 2024 17:05

ashishkumar50 reviewed Feb 1, 2024

View reviewed changes

smengcl mentioned this pull request Feb 2, 2024

HDDS-9884. Pass DatanodeVersion to the client #6155

Merged

2 tasks

Fix a corner case; additional test cases in TestHSync.

b478e89

jojochuang added 2 commits February 5, 2024 10:56

Fix test bug.

1d08d19

Address review comments and checkstyle.

a4c6682

(cherry picked from commit 2938b69)

jojochuang force-pushed the HDDS-9752 branch from 2938b69 to a4c6682 Compare February 5, 2024 18:58

ChenSammi approved these changes Feb 7, 2024

View reviewed changes

jojochuang merged commit 76a573a into apache:HDDS-7593 Feb 7, 2024

jojochuang added a commit to jojochuang/ozone that referenced this pull request Mar 5, 2024

Revert "HDDS-9752. [hsync] Make Putblock performance acceptable - Cli…

829097c

…ent side (apache#5663)" This reverts commit 76a573a.

chungen0126 pushed a commit to chungen0126/ozone that referenced this pull request May 3, 2024

HDDS-9752. [hsync] Make Putblock performance acceptable - Client side (…

b233091

…apache#5663)

chungen0126 pushed a commit to chungen0126/ozone that referenced this pull request May 3, 2024

HDDS-9752. [hsync] Make Putblock performance acceptable - Client side (…

0c9b87e

…apache#5663)

HDDS-9752. [hsync] Make Putblock performance acceptable - Client side #5663

HDDS-9752. [hsync] Make Putblock performance acceptable - Client side #5663

Uh oh!

Conversation

jojochuang commented Nov 22, 2023

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

ChenSammi Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

ChenSammi Jan 26, 2024 • 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!

ChenSammi Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

jojochuang Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

jojochuang Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

ChenSammi Jan 26, 2024 • 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!

jojochuang Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

ChenSammi Jan 26, 2024 • 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!

ChenSammi Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

jojochuang Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Jan 31, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChenSammi commented Feb 5, 2024

Uh oh!

jojochuang commented Feb 5, 2024

Uh oh!

ChenSammi left a comment

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Feb 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ChenSammi Jan 26, 2024 •

edited

Loading

ChenSammi Jan 26, 2024 •

edited

Loading

ChenSammi Jan 26, 2024 •

edited

Loading