peterxcli commented on code in PR #10181:
URL: https://github.com/apache/ozone/pull/10181#discussion_r3178201368


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -133,19 +138,47 @@ private OmKeyInfo(Builder b) {
     this.expectedETag = b.expectedETag;
   }
 
-  private static Codec<OmKeyInfo> newCodec(boolean ignorePipeline) {
+  /**
+   * Creates a new codec for OmKeyInfo.
+   *
+   * @param ignorePipeline whether to ignore pipeline info during serialization
+   * @param includeOpenKeyFields whether to include fields only used in 
openKeyTable

Review Comment:
   
   ```suggestion
      * @param isOpenkey whether to include fields only used in openKeyTable
   ```
   



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -133,19 +138,47 @@ private OmKeyInfo(Builder b) {
     this.expectedETag = b.expectedETag;
   }
 
-  private static Codec<OmKeyInfo> newCodec(boolean ignorePipeline) {
+  /**
+   * Creates a new codec for OmKeyInfo.
+   *
+   * @param ignorePipeline whether to ignore pipeline info during serialization
+   * @param includeOpenKeyFields whether to include fields only used in 
openKeyTable
+   *                             (expectedDataGeneration, expectedETag) in 
serialization.
+   *                             Use true for openKeyTable, false for keyTable.
+   * @return the codec
+   */
+  private static Codec<OmKeyInfo> newCodec(boolean ignorePipeline, boolean 
includeOpenKeyFields) {
     return new DelegatedCodec<>(
         Proto2Codec.get(KeyInfo.getDefaultInstance()),
         OmKeyInfo::getFromProtobuf,
-        k -> k.getProtobuf(ignorePipeline, ClientVersion.CURRENT_VERSION),
+        k -> k.getProtobuf(ignorePipeline, ClientVersion.CURRENT_VERSION, 
includeOpenKeyFields),
         OmKeyInfo.class);
   }
 
+  /**
+   * Gets the codec for openKeyTable. This codec includes 
expectedDataGeneration
+   * and expectedETag fields during serialization.
+   *
+   * @param ignorePipeline whether to ignore pipeline info
+   * @return the codec for openKeyTable
+   */
   public static Codec<OmKeyInfo> getCodec(boolean ignorePipeline) {

Review Comment:
   getopenkeyCodec?



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -755,7 +788,21 @@ public KeyInfo getNetworkProtobuf(String fullKeyName, int 
clientVersion,
    * @return KeyInfo
    */
   public KeyInfo getProtobuf(boolean ignorePipeline, int clientVersion) {
-    return getProtobuf(ignorePipeline, null, clientVersion, false);
+    return getProtobuf(ignorePipeline, null, clientVersion, false, true);
+  }
+
+  /**
+   * Gets KeyInfo for persistence with control over fields only used in 
openKeyTable.
+   *
+   * @param ignorePipeline true for persist to DB, false for network transmit.
+   * @param clientVersion the client version
+   * @param includeOpenKeyFields whether to include fields.
+   *                             Use true for openKeyTable, false for keyTable.
+   * @return KeyInfo
+   */
+  public KeyInfo getProtobuf(boolean ignorePipeline, int clientVersion,
+                             boolean includeOpenKeyFields) {

Review Comment:
   ```suggestion
                                boolean isopenkey) {
   ```
   elsewhere



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -133,19 +138,47 @@ private OmKeyInfo(Builder b) {
     this.expectedETag = b.expectedETag;
   }
 
-  private static Codec<OmKeyInfo> newCodec(boolean ignorePipeline) {
+  /**
+   * Creates a new codec for OmKeyInfo.
+   *
+   * @param ignorePipeline whether to ignore pipeline info during serialization
+   * @param includeOpenKeyFields whether to include fields only used in 
openKeyTable
+   *                             (expectedDataGeneration, expectedETag) in 
serialization.
+   *                             Use true for openKeyTable, false for keyTable.
+   * @return the codec
+   */
+  private static Codec<OmKeyInfo> newCodec(boolean ignorePipeline, boolean 
includeOpenKeyFields) {

Review Comment:
   ```suggestion
     private static Codec<OmKeyInfo> newCodec(boolean ignorePipeline, boolean 
isopenkey ) {
   ```
   



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java:
##########
@@ -37,8 +37,13 @@
  * admin wants to confirm if a given key is deleted from deletedTable metadata.
  */
 public class RepeatedOmKeyInfo implements CopyObject<RepeatedOmKeyInfo> {
-  private static final Codec<RepeatedOmKeyInfo> CODEC_TRUE = newCodec(true);
-  private static final Codec<RepeatedOmKeyInfo> CODEC_FALSE = newCodec(false);
+  // Codecs that include fields only used in openKeyTable (for backward 
compatibility)

Review Comment:
   Why would we need backward compatibility? Did this change break sth?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to