[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11123: Avoid memory mapping hydrants after they are persisted & after they are merged for native batch ingestion

[GitBox <gi...@apache.org>]

abhishekagarwal87 commented on a change in pull request #11123:
URL: https://github.com/apache/druid/pull/11123#discussion_r625030507



##########
File path: server/src/main/java/org/apache/druid/segment/realtime/appenderator/AppenderatorImpl.java
##########
@@ -161,6 +164,13 @@
 
   private volatile Throwable persistError;
 
+  private final boolean isRealTime;
+  // Use next Map to store metadata (File, SegmentId) for a hydrant for batch appenderator
+  // in order to facilitate the mapping of the QueryableIndex associated with a given hydrant
+  // at merge time. This is necessary since batch appenderator will not map the QueryableIndex
+  // at persist time in order to minimize its memory footprint.
+  private final Map<FireHydrant, Pair<File, SegmentId>> persistedHydrantMetadata = new HashMap<>();

Review comment:
       does `FireHydrant` has a `hashCode` implementation? Maybe using identityHashMap makes more sense here? Also, do we ever need to clear this map? like when `org.apache.druid.segment.realtime.appenderator.Appenderator#clear` or `org.apache.druid.segment.realtime.appenderator.Appenderator#drop` is called. 




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org