HDDS-14003. EventNotification: Added protobufs and entity class #9363

gardenia · 2025-11-25T16:39:11Z

Please describe your PR in detail:

In OzoneManagerStateMachine when certain write requests complete successfully a summary of that request will be written to a new rocksdb "ledger" table named CompletedRequestInfo

The current CompletedRequestInfo schema is minimal:

/**
 * CompletedRequestInfo table entry
 */
message CompletedRequestInfo {

  optional int64 trxLogIndex = 1;
  required Type cmdType = 2; // Type of the command
  optional string volumeName = 3;
  optional string bucketName = 4;
  optional string keyName = 5;
  optional uint64 creationTime = 6;

  optional CreateKeyOperationArgs       createKeyArgs = 7;
  optional RenameKeyOperationArgs       renameKeyArgs = 8;
  optional DeleteKeyOperationArgs       deleteKeyArgs = 9;
  optional CommitKeyOperationArgs       commitKeyArgs = 10;
  optional CreateDirectoryOperationArgs createDirectoryArgs = 11;
  optional CreateFileOperationArgs      createFileArgs = 12;
}

The use of "optional" for the arguments was based on feedback from the community call where @errose28 made the point that the previous sketch of a schema (using a freeform Map<String, String>) did not jibe well with schema management and this optional pattern had been used in other places to get around that.

This change adds the protobufs and entity class to store the completed event info (with the intent that a consumer can use that data to generate event notifications about changes on the filesystem)

What is the link to the Apache JIRA

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

How was this patch tested?

unit tests

github-actions · 2025-12-23T00:05:52Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

swamirishi

Thanks for the patch @gardenia I have left some comments inline. Let me know if it makes sense.

swamirishi · 2025-12-23T01:19:59Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java

  private TypedTable<String, SnapshotInfo> snapshotInfoTable;
  private TypedTable<String, String> snapshotRenamedTable;
  private TypedTable<String, CompactionLogEntry> compactionLogTable;
+  private TypedTable<String, OmCompletedRequestInfo> completedRequestInfoTable;


Make this TypedTable<Long, OmCompletedRequestInfo> rocksdb would do correct comparison for unsigned longs. We don't have to unnecessarily store a longer string

In our case we should be good as long as the txn id is going to be positive and ratis is going to ensure that. I believe we don't have to be worried about overflows from ratis side.

@swamirishi - done. thx

swamirishi · 2025-12-23T01:24:11Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java

+  /** completedOperationnfoTable: txId :- OmCompletedRequestInfo. */
+  public static final DBColumnFamilyDefinition<String, OmCompletedRequestInfo> COMPLETED_REQUEST_INFO_TABLE_DEF
+      = new DBColumnFamilyDefinition<>(COMPLETED_REQUEST_INFO_TABLE,
+          StringCodec.get(), // txid (left zeropadded)


Suggested change

StringCodec.get(), // txid (left zeropadded)

LongCodec.get()

swamirishi · 2025-12-23T01:35:45Z

hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

  optional double keysProcessedPct = 13;
 }

+message CreateKeyOperationArgs {


Do we not need an event notify for createVolume/createBucket/createSnapshot/deleteSnapshot?

@swamirishi - I have added createVolume/deleteVolume and createBucket/deleteBucket.

I am not sure if createSnapshot/deleteSnapshot is necessary to capture for the event notification feature and it would mean some schema changes to capture this so I would be interested to hear from other reviewers if you think it is suitable to capture snapshot operations for this event ledger. @errose28, @sumitagrawl , @ivandika3

swamirishi · 2025-12-23T01:40:22Z

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmCompletedRequestInfo.java

+  /**
+   * OperationType enum
+   */
+  public enum OperationType {


I don't think we need a separate enum on java side. We can just reuse enum from proto

swamirishi · 2025-12-23T01:41:39Z

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmCompletedRequestInfo.java

+   * @param exclusiveReplicatedSize - Snapshot exclusive size w/ replication.
+   */
+  @SuppressWarnings("checkstyle:ParameterNumber")
+  private OmCompletedRequestInfo(long trxLogIndex,


use builder as argument

swamirishi · 2025-12-23T01:44:45Z

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmCompletedRequestInfo.java

+  @Override
+  public Map<String, String> toAuditMap() {
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    //auditMap.put(OzoneConsts.VOLUME, getVolumeName());


Why commented out code?

I had initially made this class Auditable because it was based on another entity class which was "Auditable" but on reflection that probably is not necessary here so I have removed it. Let me know if you disagree.

swamirishi · 2025-12-23T01:45:03Z

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmCompletedRequestInfo.java

+   * Factory for making standard instance.
+   */
+  /*
+  public static OmCompletedRequestInfo newInstance(long trxLogIndex,


Why is this commented?

again it was based on another entity class which had a similar method contract. But it was never used so I have removed it.

ChenSammi · 2026-01-28T06:48:26Z

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

+  /**
+   * DB completed request info table name. Referenced in RDBStore.
+   */
+  public static final String COMPLETED_REQUEST_INFO_TABLE = "completedRequestInfoTable";


@gardenia , thanks for updating the patch.

This is defined in OMDBDefinition already. Let's remove this unused one.

ChenSammi · 2026-01-28T07:02:57Z

hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

+ */
+message CompletedRequestInfo {
+
+  optional int64 trxLogIndex = 1;


Should trxLogIndex be required? If trxLogIndex is optional, how to detect a unique record, and check whether the record has been processed already or not.

All proto fields should be optional and enforced by the code reading it. Proto3 does not support required fields.

OmClientProtocol.proto is proto2.

The point is not the current proto version. The point is that required is generally bad practice which is why protobuf has stopped supporting it.

I have left it as optional for now but I can see both sides. @ChenSammi is correct that it doesn't make sense to insert a row without it and it might be somewhat self documenting in the protos to see which fields are required. However, there are indeed checks at the code level to enforce that it is provided.

I guess I know the difference between proto2 and proto3. If we really have spent time on code review, it will be easy to found that CompletedRequestInfo has both optional and required fields, so does the other new proto definition introduced. So we either straightforwardly comply with proto2 since OmClientProtocol.proto is proto2, or we follow the proto3 behavior, consistently using optional fields with a clear comment that proto3 behavior is used since it's proto2 definition, make sense?

ChenSammi · 2026-01-28T07:18:16Z

hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

+ * Per request type entities to hold arguments
+ * captured for CompletedRequestInfo
+ */
+message CreateVolumeOperationArgs {


Can we define a message, like OperationWithoutArgs, and use it for those operations, so we can reduce the empty message defined for each type?

Good point. The intent of that approach was to have placeholder "args" entities for each operation type we capture so that they could be filled out later with any additional fields we want to capture for those operations (e.g. hflush in the case of create file). However, having thought about your feedback it occurs to me that protobuf allows us to add them later as the need arises and therefore there is no value (unless I've misunderstood something) in having placeholder empty ones at this point.

On that basis I have removed the empty argument messages and fields in the "CompletedOperationInfo" message.

ChenSammi · 2026-01-28T07:23:24Z

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmCompletedRequestInfo.java

+        cmdType == that.cmdType &&
+        creationTime == that.creationTime &&
+        volumeName.equals(that.volumeName) &&
+        bucketName.equals(that.bucketName) &&


Could you add a new unit test for null bucketName and keyName case?

…toring the ledger of completed requests

gardenia changed the title ~~HDDS-14003. EventNotification: Added protobufs and entity class for storing the ledger of completed events~~ HDDS-14003. EventNotification: Added protobufs and entity class Nov 25, 2025

ChenSammi added the event-notification label Nov 26, 2025

gardenia force-pushed the HDDS-14003 branch from 65ca6e9 to 7c6f3fd Compare December 1, 2025 12:34

jojochuang requested review from errose28, ivandika3, sumitagrawl and swamirishi December 1, 2025 17:10

github-actions bot added the stale label Dec 23, 2025

ivandika3 removed the stale label Dec 23, 2025

swamirishi requested changes Dec 23, 2025

View reviewed changes

gardenia force-pushed the HDDS-14003 branch 3 times, most recently from 9c9a6ce to f0eb620 Compare January 14, 2026 17:08

ChenSammi reviewed Jan 28, 2026

View reviewed changes

HDDS-14003. EventNotification: Added protobufs and entity class for s…

f2056a8

…toring the ledger of completed requests

gardenia force-pushed the HDDS-14003 branch from f0eb620 to f2056a8 Compare January 29, 2026 16:02

HDDS-14003. EventNotification: Added protobufs and entity class #9363

Are you sure you want to change the base?

HDDS-14003. EventNotification: Added protobufs and entity class #9363

Conversation

gardenia commented Nov 25, 2025

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

github-actions bot commented Dec 23, 2025

Uh oh!

swamirishi 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!

ChenSammi Jan 28, 2026 • 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!

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!

ChenSammi Jan 28, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ChenSammi Jan 28, 2026 •

edited

Loading

ChenSammi Jan 28, 2026 •

edited

Loading