HDDS-14241. Implement Operations as objects in RDBBatchOperation #9555

swamirishi · 2025-12-24T21:17:52Z

What changes were proposed in this pull request?

Refactor RDBBatchOperation to support various Operations by abstracting out implementation of each operations.

The patch is going to create a separate Operations abstract class which is going to have 3 implementations PutByteArrayOperation, DeleteOperation, PutCodecBufferOperation each responsible for applying the operation into the rocksdb write batch.

What is the link to the Apache JIRA

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

How was this patch tested?

Just refactoring changes no additional tests required.

…by abstracting out implementation of each operation Change-Id: Idf5b9a4b9f66eae41a9d832ddbf22e43d1027344

Change-Id: I3a36aa7e4b421ec40c7a5dcb309c9a3a9e111e0a

Change-Id: I05e959b3a502156a783910ca24b679acc8025995

szetszwo · 2025-12-27T19:20:48Z

@swamirishi , When this is ready, please click "Ready for review".

swamirishi · 2025-12-27T21:19:25Z

@szetszwo this is good to be reviewed

szetszwo

Some quick comments.

szetszwo · 2025-12-26T20:24:36Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

+    private Op() {
    }


Remove this empty constructor.

@swamirishi , this is not yet addressed.

This would be useful in the next PR. I have removed it and I will add it in the next PR.

szetszwo · 2025-12-26T20:25:48Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

+  private static final String PUT_OP = "PUT";
+  private static final String DELETE_OP = "DELETE";


Use class name and remove string constants.

szetszwo · 2025-12-27T21:54:10Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

+       * existing entries when necessary.
       */
-      private final Map<Bytes, Object> ops = new HashMap<>();
+      private final Map<Bytes, Op> batchOps = new HashMap<>();


ops is better since the class is Op. batchOps should mean objects of RDBBatchOperation.

Change-Id: I961641514c699d610719c1c0b2c6d7064ff4d0dd

Change-Id: I7f35eaa783ad38f296541448aa9ebddf80f456fe

szetszwo · 2025-12-27T22:15:57Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

+    private final byte[] key;
+
+    private DeleteOp(byte[] key) {
+      super();


Remove super() here and also the other subclasses.

szetszwo · 2025-12-27T22:16:31Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

+  /**
+   * Put operation to be applied to a {@link ColumnFamily} batch using the CodecBuffer api.
+   */
+  private final class CodecBufferPutOp extends Op {


Let's call this PutOp since we are going to remove ByteArrayPutOp later on.

I would rename this in the next patch

Done renaming it

szetszwo · 2025-12-27T22:20:36Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

-        final Object overwritten = ops.put(key, val);
+      }
+
+      void overWriteOpIfExist(Bytes key, Op operation) {


To be consistent with deleteIfExist, rename it to overwriteIfExist.

Also, rename operation to op to avoid long lines.

Change-Id: I0aff2ee9911a56c4265c72819f75da268f325aaa

swamirishi · 2025-12-28T04:49:47Z

@szetszwo can you take another look. I have addressed all your review comments.

szetszwo

+1 the change looks good.

Please fix the typo before merging this.

szetszwo · 2025-12-28T11:14:18Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBBatchOperation.java

-        final Object overwritten = ops.put(key, val);
+      }
+
+      void overWriteIfExist(Bytes key, Op op) {


English: "over write" or "overwrite"? Please don't create your own words/language. For example, see https://man7.org/linux/man-pages/man1/cp.1.html

Can I do it in the PR? Don't want to waste another CI run

Change-Id: I152be40a8376823f501df6594621512368b0e7cc

swamirishi · 2025-12-28T16:41:12Z

thank you @szetszwo for reviewing the patch

swamirishi requested a review from szetszwo December 24, 2025 21:17

swamirishi changed the title ~~HDDS-14241. Refactor RDBBatchOperation to support various Operations by abstracting out implementation of each operatio~~ HDDS-14241. Refactor RDBBatchOperation to support various Operations by abstracting out implementation of each operation Dec 25, 2025

swamirishi force-pushed the HDDS-14241 branch 2 times, most recently from e4b1a99 to 792fe8a Compare December 25, 2025 05:46

adoroszlai changed the title ~~HDDS-14241. Refactor RDBBatchOperation to support various Operations by abstracting out implementation of each operation~~ HDDS-14241. Implement Operations as objects in RDBBatchOperation Dec 25, 2025

swamirishi added 3 commits December 26, 2025 21:52

HDDS-14241. Refactor RDBBatchOperation to support various Operations …

c4e2938

…by abstracting out implementation of each operation Change-Id: Idf5b9a4b9f66eae41a9d832ddbf22e43d1027344

HDDS-14241. Rename Operation class and make static

4e271fe

Change-Id: I3a36aa7e4b421ec40c7a5dcb309c9a3a9e111e0a

HDDS-14241. Remove opType

7b762ae

Change-Id: I05e959b3a502156a783910ca24b679acc8025995

swamirishi force-pushed the HDDS-14241 branch from 6a7927b to 7b762ae Compare December 27, 2025 03:00

swamirishi marked this pull request as ready for review December 27, 2025 19:22

szetszwo reviewed Dec 27, 2025

View reviewed changes

swamirishi added 2 commits December 27, 2025 17:09

HDDS-14241. Use classname for debug logs

20e24f1

Change-Id: I961641514c699d610719c1c0b2c6d7064ff4d0dd

HDDS-14241. Rename map

042d6e0

Change-Id: I7f35eaa783ad38f296541448aa9ebddf80f456fe

szetszwo reviewed Dec 27, 2025

View reviewed changes

HDDS-14241. Address review comments

4f9977c

Change-Id: I0aff2ee9911a56c4265c72819f75da268f325aaa

szetszwo approved these changes Dec 28, 2025

View reviewed changes

HDDS-14241. Fix typo

3e321b9

Change-Id: I152be40a8376823f501df6594621512368b0e7cc

swamirishi merged commit 563b9d5 into apache:master Dec 28, 2025
43 checks passed

		private static final String PUT_OP = "PUT";
		private static final String DELETE_OP = "DELETE";

HDDS-14241. Implement Operations as objects in RDBBatchOperation #9555

HDDS-14241. Implement Operations as objects in RDBBatchOperation #9555

Conversation

swamirishi commented Dec 24, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

szetszwo commented Dec 27, 2025

Uh oh!

swamirishi commented Dec 27, 2025

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Dec 28, 2025

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swamirishi commented Dec 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants