the-other-tim-brown commented on code in PR #10976:
URL: https://github.com/apache/hudi/pull/10976#discussion_r1556695938


##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java:
##########
@@ -140,6 +141,22 @@ protected void init(HoodieTableMetaClient metaClient, 
HoodieTimeline visibleActi
    */
   protected void refreshTimeline(HoodieTimeline visibleActiveTimeline) {
     this.visibleCommitsAndCompactionTimeline = 
visibleActiveTimeline.getWriteTimeline();
+    this.timelineHashAndPendingReplaceInstants = null;
+  }
+
+  /**
+   * Get a list of pending replace instants. Caches the result for the active 
timeline.
+   * The cache is invalidated when {@link #refreshTimeline(HoodieTimeline)} is 
called.
+   *
+   * @return list of pending replace instant timestamps
+   */
+  private List<String> getPendingReplaceInstants() {
+    HoodieActiveTimeline activeTimeline = metaClient.getActiveTimeline();

Review Comment:
   > > It seems like it may make sense long term to return the same instance 
whenever possible to benefit from this cache.
   > 
   > There should be no much difference because the map cache you use also has 
per-timeline granularity. The benefit to move to the timeline itself is for 
better maintainance.
   > 
   What do you mean by "map cache"?
   
   > And if we move the cache inside the timeline, there should not be thread 
access conflicts.
   
   Why is that? Can't multiple threads access the same timeline?
   



-- 
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-unsubscr...@hudi.apache.org

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

Reply via email to