github-advanced-security[bot] commented on code in PR #18844: URL: https://github.com/apache/druid/pull/18844#discussion_r2684774624
########## server/src/test/java/org/apache/druid/segment/metadata/CompactionTestUtils.java: ########## @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.segment.metadata; + +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import org.apache.druid.jackson.DefaultObjectMapper; + +/** + * Test utilities for compaction-related tests. + */ +public class CompactionTestUtils +{ + /** + * Creates a deterministic ObjectMapper for fingerprinting tests. + * This mapper is configured to serialize with sorted map keys and alphabetically ordered properties, + * ensuring consistent fingerprints across test runs. + * + * @return A deterministic ObjectMapper instance + */ + public static ObjectMapper createDeterministicMapper() + { + ObjectMapper mapper = new DefaultObjectMapper(); + mapper.configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true); + mapper.configure(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY, true); Review Comment: ## Deprecated method or constructor invocation Invoking [ObjectMapper.configure](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/10735) ########## server/src/test/java/org/apache/druid/server/compaction/CompactionStatusTest.java: ########## @@ -535,9 +576,267 @@ final DataSegment segment = DataSegment.builder(WIKI_SEGMENT).lastCompactionState(lastCompactionState).build(); final CompactionStatus status = CompactionStatus.compute( CompactionCandidate.from(List.of(segment), null), - compactionConfig + compactionConfig, + fingerprintMapper + ); + Assert.assertFalse(status.isComplete()); + } + + @Test + public void test_evaluate_needsCompactionWhenAllSegmentsHaveUnexpectedCompactionStateFingerprint() + { + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint("wrongFingerprint").build(), + DataSegment.builder(WIKI_SEGMENT_2).compactionStateFingerprint("wrongFingerprint").build() + ); + + final DataSourceCompactionConfig oldCompactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.HOUR, null, null)) + .build(); + CompactionState wrongState = oldCompactionConfig.toCompactionState(); + + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + CompactionState expectedState = compactionConfig.toCompactionState(); + + compactionStateStorage.upsertCompactionState(TestDataSource.WIKI, "wrongFingerprint", wrongState, DateTimes.nowUtc()); + syncCacheFromManager(); + + verifyEvaluationNeedsCompactionBecauseWithCustomSegments( + CompactionCandidate.from(segments, null), + compactionConfig, + "'segmentGranularity' mismatch: required[DAY], current[HOUR]", + compactionStateStorage, + compactionStateCache + ); + } + + @Test + public void test_evaluate_needsCompactionWhenSomeSegmentsHaveUnexpectedCompactionStateFingerprint() + { + final DataSourceCompactionConfig oldCompactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.HOUR, null, null)) + .build(); + CompactionState wrongState = oldCompactionConfig.toCompactionState(); + + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + CompactionState expectedState = compactionConfig.toCompactionState(); + + String expectedFingerprint = fingerprintMapper.generateFingerprint(TestDataSource.WIKI, expectedState); + + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint(expectedFingerprint).build(), + DataSegment.builder(WIKI_SEGMENT_2).compactionStateFingerprint("wrongFingerprint").build() + ); + + compactionStateStorage.upsertCompactionState(TestDataSource.WIKI, expectedFingerprint, expectedState, DateTimes.nowUtc()); + compactionStateStorage.upsertCompactionState(TestDataSource.WIKI, "wrongFingerprint", wrongState, DateTimes.nowUtc()); + syncCacheFromManager(); + + verifyEvaluationNeedsCompactionBecauseWithCustomSegments( + CompactionCandidate.from(segments, null), + compactionConfig, + "'segmentGranularity' mismatch: required[DAY], current[HOUR]", + compactionStateStorage, + compactionStateCache + ); + } + + @Test + public void test_evaluate_noCompacationIfUnexpectedFingerprintHasExpectedCompactionState() + { + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint("wrongFingerprint").build() + ); + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.HOUR, null, null)) + .build(); + + CompactionState expectedState = compactionConfig.toCompactionState(); + compactionStateStorage.upsertCompactionState(TestDataSource.WIKI, "wrongFingerprint", expectedState, DateTimes.nowUtc()); + syncCacheFromManager(); + + final CompactionStatus status = CompactionStatus.compute( + CompactionCandidate.from(segments, null), + compactionConfig, + fingerprintMapper + ); + Assert.assertTrue(status.isComplete()); + } + + @Test + public void test_evaluate_needsCompactionWhenUnexpectedFingerprintAndNoFingerprintInMetadataStore() + { + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint("wrongFingerprint").build() + ); + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + verifyEvaluationNeedsCompactionBecauseWithCustomSegments( + CompactionCandidate.from(segments, null), + compactionConfig, + "One or more fingerprinted segments do not have a cached compaction state", + compactionStateStorage, + compactionStateCache + ); + } + + @Test + public void test_evaluate_noCompactionWhenAllSegmentsHaveExpectedCompactionStateFingerprint() + { + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + CompactionState expectedState = compactionConfig.toCompactionState(); + + String expectedFingerprint = fingerprintMapper.generateFingerprint(TestDataSource.WIKI, expectedState); + + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint(expectedFingerprint).build(), + DataSegment.builder(WIKI_SEGMENT_2).compactionStateFingerprint(expectedFingerprint).build() + ); + + final CompactionStatus status = CompactionStatus.compute( + CompactionCandidate.from(segments, null), + compactionConfig, + fingerprintMapper + ); + Assert.assertTrue(status.isComplete()); + } + + @Test + public void test_evaluate_needsCompactionWhenNonFingerprintedSegmentsFailChecksOnLastCompactionState() + { + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + CompactionState expectedState = compactionConfig.toCompactionState(); + String expectedFingerprint = fingerprintMapper.generateFingerprint(TestDataSource.WIKI, expectedState); + + compactionStateStorage.upsertCompactionState(TestDataSource.WIKI, expectedFingerprint, expectedState, DateTimes.nowUtc()); + syncCacheFromManager(); + + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint(expectedFingerprint).build(), + DataSegment.builder(WIKI_SEGMENT_2).compactionStateFingerprint(null).lastCompactionState(createCompactionStateWithGranularity(Granularities.HOUR)).build() + ); + + + verifyEvaluationNeedsCompactionBecauseWithCustomSegments( + CompactionCandidate.from(segments, null), + compactionConfig, + "'segmentGranularity' mismatch: required[DAY], current[HOUR]", + compactionStateStorage, + compactionStateCache + ); + } + + @Test + public void test_evaluate_noCompactionWhenNonFingerprintedSegmentsPassChecksOnLastCompactionState() + { + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + CompactionState expectedState = compactionConfig.toCompactionState(); + + String expectedFingerprint = fingerprintMapper.generateFingerprint(TestDataSource.WIKI, expectedState); + + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).compactionStateFingerprint(expectedFingerprint).build(), + DataSegment.builder(WIKI_SEGMENT_2).compactionStateFingerprint(null).lastCompactionState(createCompactionStateWithGranularity(Granularities.DAY)).build() + ); + + final CompactionStatus status = CompactionStatus.compute( + CompactionCandidate.from(segments, null), + compactionConfig, + fingerprintMapper ); + Assert.assertTrue(status.isComplete()); + } + + // ============================ + // SKIPPED status tests + // ============================ + + @Test + public void test_evaluate_isSkippedWhenInputBytesExceedLimit() + { + // Two segments with 100MB each = 200MB total + // inputSegmentSizeBytes is 150MB, so should be skipped + final DataSourceCompactionConfig compactionConfig = InlineSchemaDataSourceCompactionConfig + .builder() + .forDataSource(TestDataSource.WIKI) + .withInputSegmentSizeBytes(150_000_000L) + .withGranularitySpec(new UserCompactionTaskGranularityConfig(Granularities.DAY, null, null)) + .build(); + + final CompactionState lastCompactionState = createCompactionStateWithGranularity(Granularities.HOUR); + List<DataSegment> segments = List.of( + DataSegment.builder(WIKI_SEGMENT).lastCompactionState(lastCompactionState).build(), + DataSegment.builder(WIKI_SEGMENT_2).lastCompactionState(lastCompactionState).build() + ); + + final CompactionStatus status = CompactionStatus.compute( + CompactionCandidate.from(segments, null), + compactionConfig, + fingerprintMapper + ); + Assert.assertFalse(status.isComplete()); + Assert.assertTrue(status.isSkipped()); + Assert.assertTrue(status.getReason().contains("'inputSegmentSize' exceeded")); + Assert.assertTrue(status.getReason().contains("200000000")); + Assert.assertTrue(status.getReason().contains("150000000")); + } + + /** + * Verify that the evaluation indicates compaction is needed for the expected reason. + * Allows customization of the segments in the compaction candidate. + */ + private void verifyEvaluationNeedsCompactionBecauseWithCustomSegments( + CompactionCandidate candidate, + DataSourceCompactionConfig compactionConfig, + String expectedReason, + CompactionStateStorage compactionStateStorage, + CompactionStateCache compactionStateCache + ) Review Comment: ## Useless parameter The parameter 'compactionStateStorage' is never used. [Show more details](https://github.com/apache/druid/security/code-scanning/10734) -- 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]
