This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch segment_metadata_refresh in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit f398d4576163b7644c306d6cebd7563b4c6209a0 Author: Jackie (Xiaotian) Jiang <[email protected]> AuthorDate: Fri Apr 12 16:33:03 2019 -0700 When refreshing the segment with identical segment, still update creation time and refresh time in ZK metadata We check crc to identify whether two segments are identical. Segment creation time is not included in crc, so the segment creation time might be different. This make sense since we might generate segment with same data but at different time, and we don't need to refresh the segment. In such case we should still update creation time and refresh time. --- .../pinot/controller/api/upload/ZKOperator.java | 8 ++++-- .../tests/OfflineClusterIntegrationTest.java | 29 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java index 1bf5e8e..f836aae 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java @@ -152,8 +152,12 @@ public class ZKOperator { long newCrc = Long.valueOf(segmentMetadata.getCrc()); if (newCrc == existingCrc) { LOGGER.info( - "New segment crc {} is same as existing segment crc {} for segment {}. Updating ZK metadata without refreshing the segment {}", - newCrc, existingCrc, segmentName); + "New segment crc '{}' is the same as existing segment crc for segment '{}'. Updating ZK metadata without refreshing the segment.", + newCrc, segmentName); + // NOTE: even though we don't need to refresh the segment, we should still update creation time and refresh time + // (creation time is not included in the crc) + existingSegmentZKMetadata.setCreationTime(segmentMetadata.getIndexCreationTime()); + existingSegmentZKMetadata.setRefreshTime(System.currentTimeMillis()); if (!_pinotHelixResourceManager.updateZkMetadata(existingSegmentZKMetadata)) { throw new RuntimeException( "Failed to update ZK metadata for segment: " + segmentName + " of table: " + offlineTableName); diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java index 14b694f..2dc48c5 100644 --- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java +++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java @@ -33,6 +33,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import org.apache.commons.io.FileUtils; import org.apache.pinot.common.config.TableConfig; +import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata; import org.apache.pinot.common.utils.CommonConstants; import org.apache.pinot.common.utils.JsonUtils; import org.apache.pinot.common.utils.ServiceStatus; @@ -165,6 +166,34 @@ public class OfflineClusterIntegrationTest extends BaseClusterIntegrationTestSet } @Test + public void testUploadSameSegments() + throws Exception { + OfflineSegmentZKMetadata segmentZKMetadata = _helixResourceManager.getOfflineSegmentMetadata(getTableName()).get(0); + String segmentName = segmentZKMetadata.getSegmentName(); + long crc = segmentZKMetadata.getCrc(); + // Creation time is when the segment gets created + long creationTime = segmentZKMetadata.getCreationTime(); + // Push time is when the segment gets first pushed (new segment) + long pushTime = segmentZKMetadata.getPushTime(); + // Refresh time is when the segment gets refreshed (existing segment) + long refreshTime = segmentZKMetadata.getRefreshTime(); + + uploadSegments(_tarDir); + for (OfflineSegmentZKMetadata segmentZKMetadataAfterUpload : _helixResourceManager + .getOfflineSegmentMetadata(getTableName())) { + // Only check one segment + if (segmentZKMetadataAfterUpload.getSegmentName().equals(segmentName)) { + assertEquals(segmentZKMetadataAfterUpload.getCrc(), crc); + assertEquals(segmentZKMetadataAfterUpload.getCreationTime(), creationTime); + assertEquals(segmentZKMetadataAfterUpload.getPushTime(), pushTime); + // Refresh time should change + assertTrue(segmentZKMetadataAfterUpload.getRefreshTime() > refreshTime); + return; + } + } + } + + @Test public void testInvertedIndexTriggering() throws Exception { final long numTotalDocs = getCountStarResult(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
