snleee commented on a change in pull request #6239: URL: https://github.com/apache/incubator-pinot/pull/6239#discussion_r518946846
########## File path: pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadata.java ########## @@ -105,54 +97,40 @@ public void setSegmentName(String segmentName) { _segmentName = segmentName; } - @Deprecated - public String getTableName() { - return _tableName; + public SegmentType getSegmentType() { + return _segmentType; } - @Deprecated - public void setTableName(String tableName) { - _tableName = tableName; + public void setSegmentType(SegmentType segmentType) { + _segmentType = segmentType; } - public long getStartTime() { - return _startTime; + public long getStartTimeMs() { + if (_startTime > 0 && _timeUnit != null) { + return _timeUnit.toMillis(_startTime); + } else { + return -1; + } } public void setStartTime(long startTime) { _startTime = startTime; } - public long getEndTime() { - return _endTime; + public long getEndTimeMs() { + if (_endTime > 0 && _timeUnit != null) { + return _timeUnit.toMillis(_endTime); + } else { + return -1; + } } public void setEndTime(long endTime) { _endTime = endTime; } - public TimeUnit getTimeUnit() { - return _timeUnit; - } - - /** - * NOTE: should be called after setting start and end time. - */ - public void setTimeUnit(@Nonnull TimeUnit timeUnit) { + public void setTimeUnit(TimeUnit timeUnit) { _timeUnit = timeUnit; - _timeGranularity = new Duration(_timeUnit.toMillis(1)); - // For consuming segment, end time might not be set - if (_startTime >= 0 && _startTime <= _endTime) { - _timeInterval = new Interval(_timeUnit.toMillis(_startTime), _timeUnit.toMillis(_endTime)); - } - } - - public Duration getTimeGranularity() { - return _timeGranularity; - } - - public Interval getTimeInterval() { Review comment: Can we put `@Deprecated` annotation instead of removing the function? ########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/RealtimeToOfflineSegmentsTaskGenerator.java ########## @@ -287,21 +284,14 @@ private long getWatermarkMs(String realtimeTableName, List<LLCRealtimeSegmentZKM long watermarkMs; // Find the smallest time from all segments - RealtimeSegmentZKMetadata minSegmentZkMetadata = null; + long minStartTimeMs = Long.MAX_VALUE; for (LLCRealtimeSegmentZKMetadata realtimeSegmentZKMetadata : completedSegmentsMetadata) { - if (minSegmentZkMetadata == null || realtimeSegmentZKMetadata.getStartTime() < minSegmentZkMetadata - .getStartTime()) { - minSegmentZkMetadata = realtimeSegmentZKMetadata; - } + minStartTimeMs = Math.min(minStartTimeMs, realtimeSegmentZKMetadata.getStartTimeMs()); } Review comment: Do we want to add ``` Preconditions.checkState(minStartTimeMs != Long.MAX_VALUE); ``` to throw the exception at the same condition? ########## File path: pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadata.java ########## @@ -105,54 +97,40 @@ public void setSegmentName(String segmentName) { _segmentName = segmentName; } - @Deprecated - public String getTableName() { - return _tableName; + public SegmentType getSegmentType() { + return _segmentType; } - @Deprecated - public void setTableName(String tableName) { - _tableName = tableName; + public void setSegmentType(SegmentType segmentType) { + _segmentType = segmentType; } - public long getStartTime() { - return _startTime; + public long getStartTimeMs() { + if (_startTime > 0 && _timeUnit != null) { + return _timeUnit.toMillis(_startTime); + } else { + return -1; + } } public void setStartTime(long startTime) { _startTime = startTime; } - public long getEndTime() { - return _endTime; + public long getEndTimeMs() { + if (_endTime > 0 && _timeUnit != null) { + return _timeUnit.toMillis(_endTime); + } else { + return -1; + } } public void setEndTime(long endTime) { _endTime = endTime; } - public TimeUnit getTimeUnit() { - return _timeUnit; - } - - /** - * NOTE: should be called after setting start and end time. - */ - public void setTimeUnit(@Nonnull TimeUnit timeUnit) { + public void setTimeUnit(TimeUnit timeUnit) { _timeUnit = timeUnit; - _timeGranularity = new Duration(_timeUnit.toMillis(1)); - // For consuming segment, end time might not be set - if (_startTime >= 0 && _startTime <= _endTime) { - _timeInterval = new Interval(_timeUnit.toMillis(_startTime), _timeUnit.toMillis(_endTime)); - } - } - - public Duration getTimeGranularity() { Review comment: Can we put `@Deprecated ` annotation instead of removing this? ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org