From 84149d88c75d75ff6376b8f175f341dbe4685f2c Mon Sep 17 00:00:00 2001 From: Vishesh Garg Date: Fri, 8 Mar 2024 11:59:50 +0530 Subject: [PATCH 1/5] Correct gcs object last update time granularity and update var names --- .../druid/storage/google/GoogleStorage.java | 4 ++-- .../google/GoogleStorageObjectMetadata.java | 21 ++++++++++++------- .../druid/storage/google/GoogleTaskLogs.java | 6 +++--- .../GoogleTimestampVersionedDataFinder.java | 2 +- ...oogleTimestampVersionedDataFinderTest.java | 16 +++++++------- .../overlord/duty/TaskLogAutoCleaner.java | 6 +++--- .../duty/TaskLogAutoCleanerConfig.java | 16 +++++++------- .../duty/TaskLogAutoCleanerConfigTest.java | 4 ++-- 8 files changed, 40 insertions(+), 35 deletions(-) diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java index 91d290b17856..301f848339f4 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java @@ -138,7 +138,7 @@ public GoogleStorageObjectMetadata getMetadata( blob.getName(), blob.getSize(), blob.getUpdateTimeOffsetDateTime() - .toEpochSecond() + .toEpochSecond() * 1000 ); } @@ -234,7 +234,7 @@ public GoogleStorageObjectPage list( blob.getName(), blob.getSize(), blob.getUpdateTimeOffsetDateTime() - .toEpochSecond() + .toEpochSecond() * 1000 )) .collect(Collectors.toList()); diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java index 87feb774a5d8..0101beca7bc7 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java @@ -26,19 +26,24 @@ public class GoogleStorageObjectMetadata final String bucket; final String name; final Long size; - Long lastUpdateTime; + Long lastUpdateTimeMs; - public GoogleStorageObjectMetadata(final String bucket, final String name, final Long size, final Long lastUpdateTime) + public GoogleStorageObjectMetadata( + final String bucket, + final String name, + final Long size, + final Long lastUpdateTimeMs + ) { this.bucket = bucket; this.name = name; this.size = size; - this.lastUpdateTime = lastUpdateTime; + this.lastUpdateTimeMs = lastUpdateTimeMs; } - public void setLastUpdateTime(Long lastUpdateTime) + public void setLastUpdateTimeMs(Long lastUpdateTimeMs) { - this.lastUpdateTime = lastUpdateTime; + this.lastUpdateTimeMs = lastUpdateTimeMs; } @@ -57,9 +62,9 @@ public Long getSize() return size; } - public Long getLastUpdateTime() + public Long getLastUpdateTimeMs() { - return lastUpdateTime; + return lastUpdateTimeMs; } @Override @@ -90,7 +95,7 @@ public String toString() "bucket='" + bucket + '\'' + ", name='" + name + '\'' + ", size=" + size + - ", lastUpdateTime=" + lastUpdateTime + + ", lastUpdateTime=" + lastUpdateTimeMs + '}'; } } diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java index 4f7444f8ea9e..5ff89a40d150 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java @@ -190,13 +190,13 @@ public void killAll() throws IOException } @Override - public void killOlderThan(long timestamp) throws IOException + public void killOlderThan(long timestampMs) throws IOException { LOG.info( "Deleting all task logs from gs location [bucket: '%s' prefix: '%s'] older than %s.", config.getBucket(), config.getPrefix(), - new Date(timestamp) + new Date(timestampMs) ); try { GoogleUtils.deleteObjectsInPath( @@ -204,7 +204,7 @@ public void killOlderThan(long timestamp) throws IOException inputDataConfig, config.getBucket(), config.getPrefix(), - (object) -> object.getLastUpdateTime() < timestamp + (object) -> object.getLastUpdateTimeMs() < timestampMs ); } catch (Exception e) { diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java index b93128cc2fa4..aebcd5e37886 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java @@ -67,7 +67,7 @@ public URI getLatestVersion(URI descriptorBase, @Nullable Pattern pattern) if (pattern != null && !pattern.matcher(keyString).matches()) { continue; } - final long latestModified = objectMetadata.getLastUpdateTime(); + final long latestModified = objectMetadata.getLastUpdateTimeMs(); if (latestModified >= mostRecent) { mostRecent = latestModified; latest = objectLocation.toUri(GoogleStorageDruidModule.SCHEME_GS); diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java index b9417b7f7f0e..3201418c67e2 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java @@ -37,13 +37,13 @@ public void getLatestVersion() // object for directory prefix/dir/0/ final GoogleStorageObjectMetadata storageObject1 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "//", 0); - storageObject1.setLastUpdateTime(System.currentTimeMillis()); + storageObject1.setLastUpdateTimeMs(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject2 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/v1", 1); - storageObject2.setLastUpdateTime(System.currentTimeMillis()); + storageObject2.setLastUpdateTimeMs(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject3 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/v2", 1); - storageObject3.setLastUpdateTime(System.currentTimeMillis() + 100); + storageObject3.setLastUpdateTimeMs(System.currentTimeMillis() + 100); final GoogleStorageObjectMetadata storageObject4 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/other", 4); - storageObject4.setLastUpdateTime(System.currentTimeMillis() + 100); + storageObject4.setLastUpdateTimeMs(System.currentTimeMillis() + 100); final GoogleStorage storage = ObjectStorageIteratorTest.makeMockClient(ImmutableList.of(storageObject1, storageObject2, storageObject3, storageObject4)); final GoogleTimestampVersionedDataFinder finder = new GoogleTimestampVersionedDataFinder(storage); @@ -61,13 +61,13 @@ public void getLatestVersionTrailingSlashKeyPrefix() // object for directory prefix/dir/0/ final GoogleStorageObjectMetadata storageObject1 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/", 0); - storageObject1.setLastUpdateTime(System.currentTimeMillis()); + storageObject1.setLastUpdateTimeMs(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject2 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "v1", 1); - storageObject2.setLastUpdateTime(System.currentTimeMillis()); + storageObject2.setLastUpdateTimeMs(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject3 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "v2", 1); - storageObject3.setLastUpdateTime(System.currentTimeMillis() + 100); + storageObject3.setLastUpdateTimeMs(System.currentTimeMillis() + 100); final GoogleStorageObjectMetadata storageObject4 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "other", 4); - storageObject4.setLastUpdateTime(System.currentTimeMillis() + 100); + storageObject4.setLastUpdateTimeMs(System.currentTimeMillis() + 100); final GoogleStorage storage = ObjectStorageIteratorTest.makeMockClient(ImmutableList.of(storageObject1, storageObject2, storageObject3, storageObject4)); final GoogleTimestampVersionedDataFinder finder = new GoogleTimestampVersionedDataFinder(storage); diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java index 5f8f3c1c1669..f738856a311a 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java @@ -58,9 +58,9 @@ public boolean isEnabled() @Override public void run() throws Exception { - long timestamp = System.currentTimeMillis() - config.getDurationToRetain(); - taskLogKiller.killOlderThan(timestamp); - taskStorage.removeTasksOlderThan(timestamp); + long timestampMs = System.currentTimeMillis() - config.getDurationToRetainMs(); + taskLogKiller.killOlderThan(timestampMs); + taskStorage.removeTasksOlderThan(timestampMs); } @Override diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java index 71945ad92301..04f9c5099ca0 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java @@ -43,18 +43,18 @@ public class TaskLogAutoCleanerConfig private final long delay; @JsonProperty - private final long durationToRetain; + private final long durationToRetainMs; @JsonCreator public TaskLogAutoCleanerConfig( @JsonProperty("enabled") boolean enabled, @JsonProperty("initialDelay") Long initialDelay, @JsonProperty("delay") Long delay, - @JsonProperty("durationToRetain") Long durationToRetain + @JsonProperty("durationToRetain") Long durationToRetainMs ) { if (enabled) { - Preconditions.checkNotNull(durationToRetain, "'durationToRetain' must be provided."); + Preconditions.checkNotNull(durationToRetainMs, "'durationToRetain' must be provided."); } this.enabled = enabled; @@ -63,11 +63,11 @@ public TaskLogAutoCleanerConfig( 60000 + ThreadLocalRandom.current().nextInt(4 * 60000) ); this.delay = Configs.valueOrDefault(delay, TimeUnit.HOURS.toMillis(6)); - this.durationToRetain = Configs.valueOrDefault(durationToRetain, Long.MAX_VALUE); + this.durationToRetainMs = Configs.valueOrDefault(durationToRetainMs, Long.MAX_VALUE); Preconditions.checkArgument(this.initialDelay > 0, "'initialDelay' must be greater than 0."); Preconditions.checkArgument(this.delay > 0, "'delay' must be greater than 0."); - Preconditions.checkArgument(this.durationToRetain > 0, "'durationToRetain' must be greater than 0."); + Preconditions.checkArgument(this.durationToRetainMs > 0, "'durationToRetain' must be greater than 0."); } public boolean isEnabled() @@ -85,9 +85,9 @@ public long getDelay() return delay; } - public long getDurationToRetain() + public long getDurationToRetainMs() { - return durationToRetain; + return durationToRetainMs; } @Override @@ -97,7 +97,7 @@ public String toString() "enabled=" + enabled + ", initialDelay=" + initialDelay + ", delay=" + delay + - ", durationToRetain=" + durationToRetain + + ", durationToRetain=" + durationToRetainMs + '}'; } } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java index 1c78a98851e3..0880cbdc04a6 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java @@ -52,7 +52,7 @@ public void testSerde() throws Exception Assert.assertTrue(config.isEnabled()); Assert.assertEquals(10, config.getInitialDelay()); Assert.assertEquals(40, config.getDelay()); - Assert.assertEquals(30, config.getDurationToRetain()); + Assert.assertEquals(30, config.getDurationToRetainMs()); } @Test @@ -74,6 +74,6 @@ public void testSerdeWithDefaults() throws Exception Assert.assertFalse(config.isEnabled()); Assert.assertTrue(config.getInitialDelay() >= 60000 && config.getInitialDelay() <= 300000); Assert.assertEquals(6 * 60 * 60 * 1000, config.getDelay()); - Assert.assertEquals(Long.MAX_VALUE, config.getDurationToRetain()); + Assert.assertEquals(Long.MAX_VALUE, config.getDurationToRetainMs()); } } From 1f19139b423175fe0c092db62885e6a44061ea9c Mon Sep 17 00:00:00 2001 From: Vishesh Garg Date: Fri, 8 Mar 2024 14:51:06 +0530 Subject: [PATCH 2/5] Add tests and other minor changes --- .../google/GoogleStorageObjectMetadata.java | 7 ++-- .../druid/storage/google/GoogleTaskLogs.java | 2 +- .../GoogleTimestampVersionedDataFinder.java | 2 +- .../storage/google/GoogleStorageTest.java | 35 ++++++++++++++++--- .../overlord/duty/TaskLogAutoCleaner.java | 6 ++-- .../duty/TaskLogAutoCleanerConfig.java | 20 ++++++----- .../duty/TaskLogAutoCleanerConfigTest.java | 4 +-- .../apache/druid/tasklogs/TaskLogKiller.java | 6 ++++ 8 files changed, 60 insertions(+), 22 deletions(-) diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java index 0101beca7bc7..8e02709d50dc 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java @@ -62,7 +62,7 @@ public Long getSize() return size; } - public Long getLastUpdateTimeMs() + public Long getLastUpdateTimeMillis() { return lastUpdateTimeMs; } @@ -79,13 +79,14 @@ public boolean equals(Object o) GoogleStorageObjectMetadata that = (GoogleStorageObjectMetadata) o; return Objects.equals(bucket, that.bucket) && Objects.equals(name, that.name) - && Objects.equals(size, that.size); + && Objects.equals(size, that.size) + && Objects.equals(lastUpdateTimeMs, that.getLastUpdateTimeMillis()); } @Override public int hashCode() { - return Objects.hash(bucket, name, size); + return Objects.hash(bucket, name, size, lastUpdateTimeMs); } @Override diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java index 5ff89a40d150..a11694f4a2f6 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java @@ -204,7 +204,7 @@ public void killOlderThan(long timestampMs) throws IOException inputDataConfig, config.getBucket(), config.getPrefix(), - (object) -> object.getLastUpdateTimeMs() < timestampMs + (object) -> object.getLastUpdateTimeMillis() < timestampMs ); } catch (Exception e) { diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java index aebcd5e37886..01ae094912cf 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinder.java @@ -67,7 +67,7 @@ public URI getLatestVersion(URI descriptorBase, @Nullable Pattern pattern) if (pattern != null && !pattern.matcher(keyString).matches()) { continue; } - final long latestModified = objectMetadata.getLastUpdateTimeMs(); + final long latestModified = objectMetadata.getLastUpdateTimeMillis(); if (latestModified >= mostRecent) { mostRecent = latestModified; latest = objectLocation.toUri(GoogleStorageDruidModule.SCHEME_GS); diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java index d92339f53c79..63161ea90d1a 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java @@ -37,6 +37,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; public class GoogleStorageTest @@ -124,7 +125,7 @@ public void testBatchDeleteFailure() } @Test - public void testGetMetadata() throws IOException + public void testGetMetadataMatch() throws IOException { EasyMock.expect(mockStorage.get( EasyMock.eq(BUCKET), @@ -140,7 +141,33 @@ public void testGetMetadata() throws IOException EasyMock.replay(mockStorage, blob); GoogleStorageObjectMetadata objectMetadata = googleStorage.getMetadata(BUCKET, PATH); - assertEquals(objectMetadata, new GoogleStorageObjectMetadata(BUCKET, PATH, SIZE, UPDATE_TIME.toEpochSecond())); + assertEquals( + objectMetadata, + new GoogleStorageObjectMetadata(BUCKET, PATH, SIZE, UPDATE_TIME.toEpochSecond() * 1000) + ); + + } + @Test + public void testGetMetadataMismatch() throws IOException + { + EasyMock.expect(mockStorage.get( + EasyMock.eq(BUCKET), + EasyMock.eq(PATH), + EasyMock.anyObject(Storage.BlobGetOption.class) + )).andReturn(blob); + + EasyMock.expect(blob.getBucket()).andReturn(BUCKET); + EasyMock.expect(blob.getName()).andReturn(PATH); + EasyMock.expect(blob.getSize()).andReturn(SIZE); + EasyMock.expect(blob.getUpdateTimeOffsetDateTime()).andReturn(UPDATE_TIME); + + EasyMock.replay(mockStorage, blob); + + GoogleStorageObjectMetadata objectMetadata = googleStorage.getMetadata(BUCKET, PATH); + assertNotEquals( + objectMetadata, + new GoogleStorageObjectMetadata(BUCKET, PATH, SIZE, UPDATE_TIME.toEpochSecond()) + ); } @@ -243,13 +270,13 @@ public void testList() throws IOException bucket1, path1, size1, - updateTime1.toEpochSecond() + updateTime1.toEpochSecond() * 1000 ); GoogleStorageObjectMetadata objectMetadata2 = new GoogleStorageObjectMetadata( bucket2, path2, size2, - updateTime2.toEpochSecond() + updateTime2.toEpochSecond() * 1000 ); GoogleStorageObjectPage objectPage = googleStorage.list(BUCKET, PATH, null, null); diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java index f738856a311a..5f8f3c1c1669 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleaner.java @@ -58,9 +58,9 @@ public boolean isEnabled() @Override public void run() throws Exception { - long timestampMs = System.currentTimeMillis() - config.getDurationToRetainMs(); - taskLogKiller.killOlderThan(timestampMs); - taskStorage.removeTasksOlderThan(timestampMs); + long timestamp = System.currentTimeMillis() - config.getDurationToRetain(); + taskLogKiller.killOlderThan(timestamp); + taskStorage.removeTasksOlderThan(timestamp); } @Override diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java index 04f9c5099ca0..26c2a9362a4a 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfig.java @@ -43,18 +43,22 @@ public class TaskLogAutoCleanerConfig private final long delay; @JsonProperty - private final long durationToRetainMs; + private final long durationToRetain; + /** + * Config for Task logs auto-cleaner. + * All time-related parameters should be in milliseconds. + */ @JsonCreator public TaskLogAutoCleanerConfig( @JsonProperty("enabled") boolean enabled, @JsonProperty("initialDelay") Long initialDelay, @JsonProperty("delay") Long delay, - @JsonProperty("durationToRetain") Long durationToRetainMs + @JsonProperty("durationToRetain") Long durationToRetain ) { if (enabled) { - Preconditions.checkNotNull(durationToRetainMs, "'durationToRetain' must be provided."); + Preconditions.checkNotNull(durationToRetain, "'durationToRetain' must be provided."); } this.enabled = enabled; @@ -63,11 +67,11 @@ public TaskLogAutoCleanerConfig( 60000 + ThreadLocalRandom.current().nextInt(4 * 60000) ); this.delay = Configs.valueOrDefault(delay, TimeUnit.HOURS.toMillis(6)); - this.durationToRetainMs = Configs.valueOrDefault(durationToRetainMs, Long.MAX_VALUE); + this.durationToRetain = Configs.valueOrDefault(durationToRetain, Long.MAX_VALUE); Preconditions.checkArgument(this.initialDelay > 0, "'initialDelay' must be greater than 0."); Preconditions.checkArgument(this.delay > 0, "'delay' must be greater than 0."); - Preconditions.checkArgument(this.durationToRetainMs > 0, "'durationToRetain' must be greater than 0."); + Preconditions.checkArgument(this.durationToRetain > 0, "'durationToRetain' must be greater than 0."); } public boolean isEnabled() @@ -85,9 +89,9 @@ public long getDelay() return delay; } - public long getDurationToRetainMs() + public long getDurationToRetain() { - return durationToRetainMs; + return durationToRetain; } @Override @@ -97,7 +101,7 @@ public String toString() "enabled=" + enabled + ", initialDelay=" + initialDelay + ", delay=" + delay + - ", durationToRetain=" + durationToRetainMs + + ", durationToRetain=" + durationToRetain + '}'; } } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java index 0880cbdc04a6..1c78a98851e3 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/duty/TaskLogAutoCleanerConfigTest.java @@ -52,7 +52,7 @@ public void testSerde() throws Exception Assert.assertTrue(config.isEnabled()); Assert.assertEquals(10, config.getInitialDelay()); Assert.assertEquals(40, config.getDelay()); - Assert.assertEquals(30, config.getDurationToRetainMs()); + Assert.assertEquals(30, config.getDurationToRetain()); } @Test @@ -74,6 +74,6 @@ public void testSerdeWithDefaults() throws Exception Assert.assertFalse(config.isEnabled()); Assert.assertTrue(config.getInitialDelay() >= 60000 && config.getInitialDelay() <= 300000); Assert.assertEquals(6 * 60 * 60 * 1000, config.getDelay()); - Assert.assertEquals(Long.MAX_VALUE, config.getDurationToRetainMs()); + Assert.assertEquals(Long.MAX_VALUE, config.getDurationToRetain()); } } diff --git a/processing/src/main/java/org/apache/druid/tasklogs/TaskLogKiller.java b/processing/src/main/java/org/apache/druid/tasklogs/TaskLogKiller.java index d2a3d0e92fd2..76dc7b8e92f3 100644 --- a/processing/src/main/java/org/apache/druid/tasklogs/TaskLogKiller.java +++ b/processing/src/main/java/org/apache/druid/tasklogs/TaskLogKiller.java @@ -30,5 +30,11 @@ public interface TaskLogKiller { void killAll() throws IOException; + + /** + * Removes logs older than the provided timestamp + * @param timestamp Timestamp in milliseconds + * @throws IOException + */ void killOlderThan(long timestamp) throws IOException; } From a53d6edcef098c8a83a1fb35524d0414108c8bf9 Mon Sep 17 00:00:00 2001 From: Vishesh Garg Date: Fri, 8 Mar 2024 14:52:58 +0530 Subject: [PATCH 3/5] var name update --- .../google/GoogleStorageObjectMetadata.java | 18 +++++++++--------- ...GoogleTimestampVersionedDataFinderTest.java | 16 ++++++++-------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java index 8e02709d50dc..d55654a3fa37 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java @@ -26,24 +26,24 @@ public class GoogleStorageObjectMetadata final String bucket; final String name; final Long size; - Long lastUpdateTimeMs; + Long lastUpdateTimeMillis; public GoogleStorageObjectMetadata( final String bucket, final String name, final Long size, - final Long lastUpdateTimeMs + final Long lastUpdateTimeMillis ) { this.bucket = bucket; this.name = name; this.size = size; - this.lastUpdateTimeMs = lastUpdateTimeMs; + this.lastUpdateTimeMillis = lastUpdateTimeMillis; } - public void setLastUpdateTimeMs(Long lastUpdateTimeMs) + public void setLastUpdateTimeMillis(Long lastUpdateTimeMillis) { - this.lastUpdateTimeMs = lastUpdateTimeMs; + this.lastUpdateTimeMillis = lastUpdateTimeMillis; } @@ -64,7 +64,7 @@ public Long getSize() public Long getLastUpdateTimeMillis() { - return lastUpdateTimeMs; + return lastUpdateTimeMillis; } @Override @@ -80,13 +80,13 @@ public boolean equals(Object o) return Objects.equals(bucket, that.bucket) && Objects.equals(name, that.name) && Objects.equals(size, that.size) - && Objects.equals(lastUpdateTimeMs, that.getLastUpdateTimeMillis()); + && Objects.equals(lastUpdateTimeMillis, that.getLastUpdateTimeMillis()); } @Override public int hashCode() { - return Objects.hash(bucket, name, size, lastUpdateTimeMs); + return Objects.hash(bucket, name, size, lastUpdateTimeMillis); } @Override @@ -96,7 +96,7 @@ public String toString() "bucket='" + bucket + '\'' + ", name='" + name + '\'' + ", size=" + size + - ", lastUpdateTime=" + lastUpdateTimeMs + + ", lastUpdateTime=" + lastUpdateTimeMillis + '}'; } } diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java index 3201418c67e2..2c65d3863793 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTimestampVersionedDataFinderTest.java @@ -37,13 +37,13 @@ public void getLatestVersion() // object for directory prefix/dir/0/ final GoogleStorageObjectMetadata storageObject1 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "//", 0); - storageObject1.setLastUpdateTimeMs(System.currentTimeMillis()); + storageObject1.setLastUpdateTimeMillis(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject2 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/v1", 1); - storageObject2.setLastUpdateTimeMs(System.currentTimeMillis()); + storageObject2.setLastUpdateTimeMillis(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject3 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/v2", 1); - storageObject3.setLastUpdateTimeMs(System.currentTimeMillis() + 100); + storageObject3.setLastUpdateTimeMillis(System.currentTimeMillis() + 100); final GoogleStorageObjectMetadata storageObject4 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/other", 4); - storageObject4.setLastUpdateTimeMs(System.currentTimeMillis() + 100); + storageObject4.setLastUpdateTimeMillis(System.currentTimeMillis() + 100); final GoogleStorage storage = ObjectStorageIteratorTest.makeMockClient(ImmutableList.of(storageObject1, storageObject2, storageObject3, storageObject4)); final GoogleTimestampVersionedDataFinder finder = new GoogleTimestampVersionedDataFinder(storage); @@ -61,13 +61,13 @@ public void getLatestVersionTrailingSlashKeyPrefix() // object for directory prefix/dir/0/ final GoogleStorageObjectMetadata storageObject1 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "/", 0); - storageObject1.setLastUpdateTimeMs(System.currentTimeMillis()); + storageObject1.setLastUpdateTimeMillis(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject2 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "v1", 1); - storageObject2.setLastUpdateTimeMs(System.currentTimeMillis()); + storageObject2.setLastUpdateTimeMillis(System.currentTimeMillis()); final GoogleStorageObjectMetadata storageObject3 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "v2", 1); - storageObject3.setLastUpdateTimeMs(System.currentTimeMillis() + 100); + storageObject3.setLastUpdateTimeMillis(System.currentTimeMillis() + 100); final GoogleStorageObjectMetadata storageObject4 = ObjectStorageIteratorTest.makeStorageObject(bucket, keyPrefix + "other", 4); - storageObject4.setLastUpdateTimeMs(System.currentTimeMillis() + 100); + storageObject4.setLastUpdateTimeMillis(System.currentTimeMillis() + 100); final GoogleStorage storage = ObjectStorageIteratorTest.makeMockClient(ImmutableList.of(storageObject1, storageObject2, storageObject3, storageObject4)); final GoogleTimestampVersionedDataFinder finder = new GoogleTimestampVersionedDataFinder(storage); From 52d3b2abc6babf0759eae45c9afd8f3b89086476 Mon Sep 17 00:00:00 2001 From: Vishesh Garg Date: Fri, 8 Mar 2024 16:30:09 +0530 Subject: [PATCH 4/5] Remove test --- .../google/GoogleStorageObjectMetadata.java | 2 +- .../storage/google/GoogleStorageTest.java | 23 ------------------- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java index d55654a3fa37..ce4b4ca3064b 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorageObjectMetadata.java @@ -96,7 +96,7 @@ public String toString() "bucket='" + bucket + '\'' + ", name='" + name + '\'' + ", size=" + size + - ", lastUpdateTime=" + lastUpdateTimeMillis + + ", lastUpdateTimeMillis=" + lastUpdateTimeMillis + '}'; } } diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java index 63161ea90d1a..ec4925bdbc32 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java @@ -146,29 +146,6 @@ public void testGetMetadataMatch() throws IOException new GoogleStorageObjectMetadata(BUCKET, PATH, SIZE, UPDATE_TIME.toEpochSecond() * 1000) ); - } - @Test - public void testGetMetadataMismatch() throws IOException - { - EasyMock.expect(mockStorage.get( - EasyMock.eq(BUCKET), - EasyMock.eq(PATH), - EasyMock.anyObject(Storage.BlobGetOption.class) - )).andReturn(blob); - - EasyMock.expect(blob.getBucket()).andReturn(BUCKET); - EasyMock.expect(blob.getName()).andReturn(PATH); - EasyMock.expect(blob.getSize()).andReturn(SIZE); - EasyMock.expect(blob.getUpdateTimeOffsetDateTime()).andReturn(UPDATE_TIME); - - EasyMock.replay(mockStorage, blob); - - GoogleStorageObjectMetadata objectMetadata = googleStorage.getMetadata(BUCKET, PATH); - assertNotEquals( - objectMetadata, - new GoogleStorageObjectMetadata(BUCKET, PATH, SIZE, UPDATE_TIME.toEpochSecond()) - ); - } @Test From c8fefbe2cf7661e6dbb783c041fcdb217c1e4ae3 Mon Sep 17 00:00:00 2001 From: Vishesh Garg Date: Fri, 8 Mar 2024 16:44:29 +0530 Subject: [PATCH 5/5] Remove unused import --- .../java/org/apache/druid/storage/google/GoogleStorageTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java index ec4925bdbc32..715628022072 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java @@ -37,7 +37,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; public class GoogleStorageTest