kfaraz commented on code in PR #17976:
URL: https://github.com/apache/druid/pull/17976#discussion_r2072312246


##########
server/src/main/java/org/apache/druid/server/log/FileRequestLogger.java:
##########
@@ -57,27 +57,34 @@ public class FileRequestLogger implements RequestLogger
   private final File baseDir;
   private final DateTimeFormatter filePattern;
   private final Duration durationToRetain;
+  private final Duration rollPeriod;
   private final Object lock = new Object();
 
-  private DateTime currentDay;
+  private DateTime currentPeriod;

Review Comment:
   ```suggestion
     private DateTime currentPeriodStart;
   ```



##########
server/src/main/java/org/apache/druid/server/log/FileRequestLogger.java:
##########
@@ -57,27 +57,34 @@ public class FileRequestLogger implements RequestLogger
   private final File baseDir;
   private final DateTimeFormatter filePattern;
   private final Duration durationToRetain;
+  private final Duration rollPeriod;
   private final Object lock = new Object();
 
-  private DateTime currentDay;
+  private DateTime currentPeriod;
   private OutputStreamWriter fileWriter;
 
   public FileRequestLogger(
       ObjectMapper objectMapper,
       ScheduledExecutorService exec,
       File baseDir,
       String filePattern,
-      Duration durationToRetain
+      Duration durationToRetain,
+      Duration rollPeriod
   )
   {
     this.exec = exec;
     this.objectMapper = objectMapper;
     this.baseDir = baseDir;
     this.filePattern = DateTimeFormat.forPattern(filePattern);
     this.durationToRetain = durationToRetain;
+    this.rollPeriod = rollPeriod;
     Preconditions.checkArgument(
-        this.durationToRetain == null || 
this.durationToRetain.compareTo(Duration.standardDays(1)) >= 0,
-        "request logs retention period must be atleast P1D"
+        this.durationToRetain == null || 
this.durationToRetain.compareTo(Duration.standardHours(1)) >= 0,
+        "request logs retention period must be atleast PT1H"

Review Comment:
   I think that rather than an absolute check against 1 hour, we should check 
for `durationToRetain` >= `rollPeriod`.



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