clintropolis commented on code in PR #13852:
URL: https://github.com/apache/druid/pull/13852#discussion_r1119621427


##########
server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicy.java:
##########
@@ -20,34 +20,78 @@
 package org.apache.druid.server.coordinator.duty;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Preconditions;
 import com.google.inject.Inject;
+import org.apache.druid.java.util.common.Pair;
 import org.apache.druid.server.coordinator.DataSourceCompactionConfig;
+import org.apache.druid.server.coordinator.DruidCoordinatorConfig;
 import org.apache.druid.timeline.SegmentTimeline;
 import org.joda.time.Interval;
 
+import java.time.Clock;
 import java.util.List;
 import java.util.Map;
 
 /**
  * This policy searches segments for compaction from the newest one to oldest 
one.
+ * The {@link #resetIfNeeded} functionality is inspired by {@link 
com.google.common.base.Suppliers.ExpiringMemoizingSupplier}.
  */
 public class NewestSegmentFirstPolicy implements CompactionSegmentSearchPolicy
 {
   private final ObjectMapper objectMapper;
+  private final long durationMillis;
+  private transient volatile NewestSegmentFirstIterator iterator;

Review Comment:
   I guess I agree that `transient` is technically harmless since we don't 
participate in `Serializable`, I was more thinking that it being there makes 
the reader wonder if we do or why its there (like i did), and since we don't 
use `transient` anywhere else in the code.
   
   Guava has it because
   ```
   static class MemoizingSupplier<T> implements Supplier<T>, Serializable
   ```
   so it needs these things because it implements `Serializable`, but we do not.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to