[I] Revisit the design of HoodieRecord metadata [hudi]

["hudi-bot (via GitHub)" <gi...@apache.org>]

hudi-bot opened a new issue, #17111:
URL: https://github.com/apache/hudi/issues/17111

   Currently, each HoodieRecord takes an optional Map as a metadata, which is used for function like event_time metadata tracing, but it is not as efficient.
   
   Before write a record in each write handle, we initialize a new map to keep the metadata if the HoodieRecord is empty(now it is always empty), and pass it to the WriteStatus.markSuccess, the write status then fetchs the meta from the map, this happens for every record.
   
   Suggestion, remove the metadata from HoodieRecord, and add API on WriteStauts to collect the whatever metadata we like directly, for e.g, 
   
   {code:java}
   WriteStatus.collectEventTimeMetadata(Object eventTime).
   {code}
   
   
   ## JIRA info
   
   - Link: https://issues.apache.org/jira/browse/HUDI-9659
   - Type: Improvement
   - Fix version(s):
     - 1.2.0


-- 
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: commits-unsubscribe@hudi.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Revisit the design of HoodieRecord metadata [hudi]

["rangareddy (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org