rangareddy commented on issue #17111:
URL: https://github.com/apache/hudi/issues/17111#issuecomment-5366096688

   This issue was reviewed as part of the JIRA-migrated backlog triage 
(HUDI-9659).
   
   **Findings: confirmed, and the cost extends to serialization as well.**
   
   The per-record map is intact in 
`hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecord.java`:
   
   ```java
   :163  protected Option<Map<String, String>> metaData;
   :173  public HoodieRecord(HoodieKey key, T data, HoodieOperation operation, 
Option<Map<String, String>> metaData)
   :184  public HoodieRecord(HoodieKey key, T data, HoodieOperation operation, 
Boolean isDelete, Option<Map<String, String>> metaData)
   :521  public abstract Option<Map<String, String>> getMetadata();
   ```
   
   Worth adding to the case in your description: the field is also on the Kryo 
path, per record:
   
   ```java
   :410  kryo.writeClassAndObject(output, metaData == null ? null : 
metaData.orElse(null));
   :429  this.metaData = Option.ofNullable((Map<String, String>) 
kryo.readClassAndObject(input));
   ```
   
   `writeClassAndObject` / `readClassAndObject` are the expensive Kryo variants 
because they carry class information. So every shuffled record pays for a field 
that - as you note - is always empty in practice today. That is on top of the 
per-record map allocation in the write handle that you already identified.
   
   Your proposed direction (drop it from `HoodieRecord` and collect what is 
needed directly on `WriteStatus`, e.g. `collectEventTimeMetadata`) removes both 
costs. The compatibility question to settle is whether `getMetadata()` is 
relied on by any external `HoodieRecord` subclass, since it is a public 
abstract method.
   
   Keeping this open.
   


-- 
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]

Reply via email to