saurabhd336 commented on code in PR #9356:
URL: https://github.com/apache/pinot/pull/9356#discussion_r985385484


##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/timeboundary/TimeBoundaryManager.java:
##########
@@ -142,17 +153,42 @@ private long 
extractEndTimeMsFromSegmentZKMetadataZNRecord(String segment, @Null
     return endTimeMs;
   }
 
-  private void updateTimeBoundaryInfo(long maxEndTimeMs) {
-    if (maxEndTimeMs > 0) {
-      String timeBoundary = _timeFormatSpec.fromMillisToFormat(maxEndTimeMs - 
_timeOffsetMs);
-      TimeBoundaryInfo currentTimeBoundaryInfo = _timeBoundaryInfo;
-      if (currentTimeBoundaryInfo == null || 
!currentTimeBoundaryInfo.getTimeValue().equals(timeBoundary)) {
-        _timeBoundaryInfo = new TimeBoundaryInfo(_timeColumn, timeBoundary);
-        LOGGER.info("Updated time boundary to: {} for table: {}", 
timeBoundary, _offlineTableName);
+  private synchronized void updateTimeBoundaryInfo(Long enforcedTimeBoundary, 
long maxEndTimeMs,
+      boolean idealStateReffered) {

Review Comment:
   Ack



##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/timeboundary/TimeBoundaryManager.java:
##########
@@ -142,17 +153,42 @@ private long 
extractEndTimeMsFromSegmentZKMetadataZNRecord(String segment, @Null
     return endTimeMs;
   }
 
-  private void updateTimeBoundaryInfo(long maxEndTimeMs) {
-    if (maxEndTimeMs > 0) {
-      String timeBoundary = _timeFormatSpec.fromMillisToFormat(maxEndTimeMs - 
_timeOffsetMs);
-      TimeBoundaryInfo currentTimeBoundaryInfo = _timeBoundaryInfo;
-      if (currentTimeBoundaryInfo == null || 
!currentTimeBoundaryInfo.getTimeValue().equals(timeBoundary)) {
-        _timeBoundaryInfo = new TimeBoundaryInfo(_timeColumn, timeBoundary);
-        LOGGER.info("Updated time boundary to: {} for table: {}", 
timeBoundary, _offlineTableName);
+  private synchronized void updateTimeBoundaryInfo(Long enforcedTimeBoundary, 
long maxEndTimeMs,

Review Comment:
   Ack



##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/timeboundary/TimeBoundaryManager.java:
##########
@@ -142,17 +153,42 @@ private long 
extractEndTimeMsFromSegmentZKMetadataZNRecord(String segment, @Null
     return endTimeMs;
   }
 
-  private void updateTimeBoundaryInfo(long maxEndTimeMs) {
-    if (maxEndTimeMs > 0) {
-      String timeBoundary = _timeFormatSpec.fromMillisToFormat(maxEndTimeMs - 
_timeOffsetMs);
-      TimeBoundaryInfo currentTimeBoundaryInfo = _timeBoundaryInfo;
-      if (currentTimeBoundaryInfo == null || 
!currentTimeBoundaryInfo.getTimeValue().equals(timeBoundary)) {
-        _timeBoundaryInfo = new TimeBoundaryInfo(_timeColumn, timeBoundary);
-        LOGGER.info("Updated time boundary to: {} for table: {}", 
timeBoundary, _offlineTableName);
+  private synchronized void updateTimeBoundaryInfo(Long enforcedTimeBoundary, 
long maxEndTimeMs,

Review Comment:
   Ack



##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/timeboundary/TimeBoundaryManager.java:
##########
@@ -142,17 +153,42 @@ private long 
extractEndTimeMsFromSegmentZKMetadataZNRecord(String segment, @Null
     return endTimeMs;
   }
 
-  private void updateTimeBoundaryInfo(long maxEndTimeMs) {
-    if (maxEndTimeMs > 0) {
-      String timeBoundary = _timeFormatSpec.fromMillisToFormat(maxEndTimeMs - 
_timeOffsetMs);
-      TimeBoundaryInfo currentTimeBoundaryInfo = _timeBoundaryInfo;
-      if (currentTimeBoundaryInfo == null || 
!currentTimeBoundaryInfo.getTimeValue().equals(timeBoundary)) {
-        _timeBoundaryInfo = new TimeBoundaryInfo(_timeColumn, timeBoundary);
-        LOGGER.info("Updated time boundary to: {} for table: {}", 
timeBoundary, _offlineTableName);
+  private synchronized void updateTimeBoundaryInfo(Long enforcedTimeBoundary, 
long maxEndTimeMs,
+      boolean idealStateReffered) {
+    TimeBoundaryInfo currentTimeBoundaryInfo = _timeBoundaryInfo;
+    Long finalTimeBoundaryMs = null;
+    boolean isEnforced = false;
+    boolean validTimeBoundaryFound = false;
+
+    if (enforcedTimeBoundary != null) {

Review Comment:
   I couldn't seem to find a way to simplify this without using 2 different 
TimeBoundaryInfo instances for explicitly set and derived time boundaries. Have 
added comments for now.



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