[GitHub] [hudi] nsivabalan commented on a diff in pull request #8480: [HUDI-6090] Optimise payload size for list of FileGroupDTO

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

nsivabalan commented on code in PR #8480:
URL: https://github.com/apache/hudi/pull/8480#discussion_r1174041178


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/dto/FileGroupDTO.java:
##########
@@ -46,17 +49,50 @@ public class FileGroupDTO {
   TimelineDTO timeline;
 
   public static FileGroupDTO fromFileGroup(HoodieFileGroup fileGroup) {
+    return fromFileGroup(fileGroup, true);
+  }
+
+  public static List<FileGroupDTO> fromFileGroup(List<HoodieFileGroup> fileGroups) {

Review Comment:
   this is a POJO class. lets not add static utils methods here. can we move it to elsewhere. something like DTOUtils may be. 



##########
hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/RequestHandler.java:
##########
@@ -160,6 +160,8 @@ private boolean isLocalViewBehind(Context ctx) {
     if (LOG.isDebugEnabled()) {
       LOG.debug("Client [ LastTs=" + lastKnownInstantFromClient + ", TimelineHash=" + timelineHashFromClient
           + "], localTimeline=" + localTimeline.getInstants());
+    } else {
+      LOG.info("Client [ LastTs=" + lastKnownInstantFromClient + ", TimelineHash=" + timelineHashFromClient + "]");

Review Comment:
   this gets logged for every request reaching timeline server. so, better to avoid this



##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/dto/FileGroupDTO.java:
##########
@@ -46,17 +49,50 @@ public class FileGroupDTO {
   TimelineDTO timeline;
 
   public static FileGroupDTO fromFileGroup(HoodieFileGroup fileGroup) {
+    return fromFileGroup(fileGroup, true);
+  }
+
+  public static List<FileGroupDTO> fromFileGroup(List<HoodieFileGroup> fileGroups) {
+    if (fileGroups.isEmpty()) {
+      return Collections.emptyList();
+    }
+
+    List<FileGroupDTO> fileGroupDTOs = fileGroups.stream()
+        .map(fg -> FileGroupDTO.fromFileGroup(fg, false)).collect(Collectors.toList());
+    // Timeline exists only in the first file group DTO. Optimisation to reduce payload size.
+    fileGroupDTOs.set(0, FileGroupDTO.fromFileGroup(fileGroups.get(0), true));
+    return fileGroupDTOs;
+  }
+
+  public static HoodieFileGroup toFileGroup(FileGroupDTO dto, HoodieTableMetaClient metaClient) {
+    return toFileGroup(dto, metaClient, null);

Review Comment:
   +1 



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