You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/15 22:05:12 UTC
[GitHub] [pinot] jackjlli commented on a diff in pull request #9403: Handle segments outside retention time gracefully during creation
jackjlli commented on code in PR #9403:
URL: https://github.com/apache/pinot/pull/9403#discussion_r972452979
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -765,6 +766,28 @@ private void writeMetadata()
"Invalid segment start/end time: %s (in millis: %s/%s) for time column: %s, must be between: %s",
timeInterval, timeInterval.getStartMillis(), timeInterval.getEndMillis(), timeColumnName,
TimeUtils.VALID_TIME_INTERVAL);
+ } else {
+ Interval timeInterval =
+ new Interval(timeUnit.toMillis(startTime), timeUnit.toMillis(endTime), DateTimeZone.UTC);
+ long now = System.currentTimeMillis();
+ if (!TimeUtils.isValidTimeInterval(timeInterval)) {
+ startTime = now;
+ endTime = now;
+ }
+ SegmentsValidationAndRetentionConfig validationConfig = _config.getTableConfig().getValidationConfig();
+ String retentionTimeUnitConfig = validationConfig.getRetentionTimeUnit();
+ String retentionTimeValueConfig = validationConfig.getRetentionTimeValue();
+ try {
+ TimeUnit retentionTimeUnit = TimeUnit.valueOf(retentionTimeUnitConfig.toUpperCase());
+ long retentionTimeValue = Long.parseLong(retentionTimeValueConfig);
+ long retentionTimeMs = retentionTimeUnit.toMillis(retentionTimeValue);
+ if (endTime <= (now - retentionTimeMs)) {
Review Comment:
nit: put some comments to explicitly explain the rationale of doing this here.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -765,6 +766,28 @@ private void writeMetadata()
"Invalid segment start/end time: %s (in millis: %s/%s) for time column: %s, must be between: %s",
timeInterval, timeInterval.getStartMillis(), timeInterval.getEndMillis(), timeColumnName,
TimeUtils.VALID_TIME_INTERVAL);
+ } else {
+ Interval timeInterval =
+ new Interval(timeUnit.toMillis(startTime), timeUnit.toMillis(endTime), DateTimeZone.UTC);
+ long now = System.currentTimeMillis();
+ if (!TimeUtils.isValidTimeInterval(timeInterval)) {
+ startTime = now;
+ endTime = now;
+ }
+ SegmentsValidationAndRetentionConfig validationConfig = _config.getTableConfig().getValidationConfig();
+ String retentionTimeUnitConfig = validationConfig.getRetentionTimeUnit();
+ String retentionTimeValueConfig = validationConfig.getRetentionTimeValue();
+ try {
+ TimeUnit retentionTimeUnit = TimeUnit.valueOf(retentionTimeUnitConfig.toUpperCase());
+ long retentionTimeValue = Long.parseLong(retentionTimeValueConfig);
+ long retentionTimeMs = retentionTimeUnit.toMillis(retentionTimeValue);
+ if (endTime <= (now - retentionTimeMs)) {
+ endTime = now;
+ }
+ } catch (Exception e) {
+ LOGGER.warn("Invalid retention time: {} {} for table: {}, skip", retentionTimeUnitConfig,
Review Comment:
nit: it'd be good to state what step is skipped here.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org