HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton code #5661

jojochuang · 2023-11-22T19:55:59Z

What changes were proposed in this pull request?

Skeleton code for supporting incremental chunk list in PutBlock. See HDDS-8047 for the problem description and design doc.

Please describe your PR in detail:

Add a new rocksdb table lastChunkInfoTable to store the last, in-progress, partial chunk of blocks. (Question: does adding a new table require a new schema version v4? do we need additional code to support upgrade?)
Add a Client configuration switch incremental.chunk.list for clients to send incremental chunk list in PutBlock.
Add a new client version INCREMENTAL_CHUNK_LIST_SUPPORT (this version change may not be required)

What is the link to the Apache JIRA

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

How was this patch tested?

Skeleton code. No functional change.

...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

ChenSammi · 2023-11-27T08:16:19Z

There is one HDDSLayoutFeature.java to record any new features on DN side. @jojochuang , I think we should add one new item in HDDSLayoutFeature too since we add a new CF for every container.

ChenSammi · 2023-11-27T09:04:58Z

cc @ashishkumar50 , could you help to take a review too?

ashishkumar50

@jojochuang, Thanks for the patch.
When new version client connects with the old version DN, in that case chunklist will be only the last chunks which new client will send. Does this case is considered?

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

ashishkumar50 · 2023-11-28T09:15:08Z

...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

+  @Override
+  public long putBlock(Container container, BlockData data, boolean endOfBlock)
+      throws IOException {
+    return putBlock(container, data, endOfBlock, true);


By default can we keep incremental as false so that current behaviour remains same?

i'm going to refactor this one to make it clear, but basically the client side will have a flag in PutBlock request to hint it carries incremental chunk list. If the flag is false it falls back to the original behavior.

jojochuang · 2023-12-06T23:51:02Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/ClientVersion.java

      "This client version has support for Object Store and File " +
          "System Optimized Bucket Layouts."),
+  INCREMENTAL_CHUNK_LIST_SUPPORT(4,
+      "This client version has support for incremental chunk list."),


this is not needed. will remove.

jojochuang · 2023-12-07T00:21:54Z

When new version client connects with the old version DN, in that case chunklist will be only the last chunks which new client will send. Does this case is considered?

Good question. Checking the code we don't seem to have a framework to detect DataNode version from client side? If so I'd leave it later to implement it in HDDS-9753.

Another guardrail to implement is reject new clients from sending incremental chunk list in PutBlock for blocks in schema v1~v3.

jojochuang · 2023-12-07T00:41:55Z

There is one HDDSLayoutFeature.java to record any new features on DN side. @jojochuang , I think we should add one new item in HDDSLayoutFeature too since we add a new CF for every container.

My understanding, according to https://github.com/facebook/rocksdb/wiki/Column-Families is that adding/droping RocksDB CF doesn't require any extra work. So perhaps no need for a schema v4? Just make sure that this feature (use the new CF) is activated after upgrade is finalized.

jojochuang · 2023-12-08T19:01:30Z

When new version client connects with the old version DN, in that case chunklist will be only the last chunks which new client will send. Does this case is considered?

We need to fix HDDS-9884 first for clients to be able to tell DataNode version.

Change-Id: I2a92d9137bcb1ed7a91f465cf26046ab1d8b1857 Fix checkstyle Change-Id: Ifdd3b0f088d7d60adb50063bdc5e2e67392c5bd9 Refactor code and address review comments. Add a layout feature and reject incremental chunk list requests if the layout version is not finalized. Add config ozone.chunk.list.incremental to ozone-default.xml.

ashishkumar50

@jojochuang Thanks for updating patch. Overall LGTM, just a minor suggestion.

ashishkumar50 · 2024-01-10T03:59:23Z

...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

      throws IOException {
+    boolean incrementalEnabled =
+            config.getBoolean(OZONE_CHUNK_LIST_INCREMENTAL,
+                    OZONE_CHUNK_LIST_INCREMENTAL_DEFAULT);


Getting OZONE_CHUNK_LIST_INCREMENTAL can move to BlockManagerImpl constructor to avoid reading in every putBlock call.

Yeah I understand but this is a static method, and it uses less than 0.1% of CPU time

ChenSammi · 2024-01-10T04:22:35Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/upgrade/HDDSLayoutFeature.java

  HADOOP_PRC_PORTS_IN_DATANODEDETAILS(7, "Adding Hadoop RPC ports " +
-                                     "to DatanodeDetails.");
+                                     "to DatanodeDetails."),
+  LAST_CHUNK_TABLE(8, "Datanode RocksDB Schema Version 3 has an extra table " +


How about change the feature name to HBASE_SUPPORT? For file recovery, a new table finalize_blocks is also introduced. So for HBASE, as of now, two new tables are introduced for it. So if use the name HBASE_SUPPORT, we can cover these two new tables, and also the potential new changes in later.

We should support both Schema V3 and Schema V2. For an existing Ozone cluster with older version, it's likely there will be open Schema V2 containers there.

ChenSammi · 2024-01-10T04:28:27Z

...tainer-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java

    // Create DB store for a newly formatted volume
    if (VersionedDatanodeFeatures.isFinalized(
-        HDDSLayoutFeature.DATANODE_SCHEMA_V3)) {
+        HDDSLayoutFeature.LAST_CHUNK_TABLE)) {


This change is not necessary. Let's keep the current code. DATANODE_SCHEMA_V3 will be easy to understand this part logic.

ChenSammi · 2024-01-10T04:30:02Z

...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

      BlockData data, ConfigurationSource config, boolean endOfBlock)
      throws IOException {
+    boolean incrementalEnabled =
+            config.getBoolean(OZONE_CHUNK_LIST_INCREMENTAL,


The indent here looks like 8. It's expected to be 4. It's odd that there is no checkstyle warning.

ChenSammi

The last patch looks good to me.

…de (apache#5661)

jojochuang requested a review from errose28 November 22, 2023 19:56

jojochuang changed the title ~~Hdds 9750~~ HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton code Nov 22, 2023

This was referenced Nov 22, 2023

HDDS-9751. [hsync] Make Putblock performance acceptable - DataNode side #5662

Merged

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

Merged

ChenSammi reviewed Nov 27, 2023

View reviewed changes

...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java Outdated Show resolved Hide resolved

ChenSammi reviewed Nov 27, 2023

View reviewed changes

...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java Outdated Show resolved Hide resolved

ashishkumar50 reviewed Nov 28, 2023

View reviewed changes

jojochuang commented Dec 6, 2023

View reviewed changes

jojochuang force-pushed the HDDS-9750 branch from 7fb06c5 to e5ae92e Compare December 19, 2023 01:03

jojochuang force-pushed the HDDS-9750 branch from e5ae92e to 3ddbcf8 Compare January 4, 2024 16:46

jojochuang force-pushed the HDDS-9750 branch from 3ddbcf8 to 852e8a7 Compare January 5, 2024 01:02

ashishkumar50 reviewed Jan 10, 2024

View reviewed changes

ChenSammi reviewed Jan 10, 2024

View reviewed changes

jojochuang added 2 commits January 10, 2024 19:58

Address review comments.

83cd844

Add table definition for schema v2.

981692c

ChenSammi approved these changes Jan 11, 2024

View reviewed changes

jojochuang merged commit 5d91635 into apache:HDDS-7593 Jan 11, 2024

jojochuang added a commit to jojochuang/ozone that referenced this pull request Jan 17, 2024

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton co…

3a8863b

…de (apache#5661)

jojochuang added a commit to jojochuang/ozone that referenced this pull request Jan 17, 2024

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton co…

ada3325

…de (apache#5661)

jojochuang added a commit to jojochuang/ozone that referenced this pull request Jan 17, 2024

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton co…

7224393

…de (apache#5661)

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

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

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton co…

42b4753

…de (apache#5661)

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

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton co…

ee1d877

…de (apache#5661)

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton code #5661

HDDS-9750. [hsync] Make Putblock performance acceptable - Skeleton code #5661

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!

Uh oh!

Uh oh!

ChenSammi commented Nov 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChenSammi commented Nov 27, 2023

Uh oh!

ashishkumar50 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashishkumar50 Nov 28, 2023

Choose a reason for hiding this comment

Uh oh!

jojochuang Dec 6, 2023

Choose a reason for hiding this comment

Uh oh!

jojochuang Dec 6, 2023

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jojochuang commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jojochuang commented Dec 8, 2023

Uh oh!

ashishkumar50 left a comment

Choose a reason for hiding this comment

Uh oh!

ashishkumar50 Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

jojochuang Jan 11, 2024

Choose a reason for hiding this comment

Uh oh!

ChenSammi Jan 10, 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 11, 2024

Choose a reason for hiding this comment

Uh oh!

ChenSammi Jan 10, 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 10, 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 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ChenSammi commented Nov 27, 2023 •

edited

Loading

jojochuang commented Dec 7, 2023 •

edited

Loading

jojochuang commented Dec 7, 2023 •

edited

Loading

ChenSammi Jan 10, 2024 •

edited

Loading

ChenSammi Jan 10, 2024 •

edited

Loading

ChenSammi Jan 10, 2024 •

edited

Loading