You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "abhishekrb19 (via GitHub)" <gi...@apache.org> on 2026/06/22 19:45:48 UTC
[PR] fix: preserve `DimensionValueSetShardSpec` on upgraded append segments (druid)
abhishekrb19 opened a new pull request, #19615:
URL: https://github.com/apache/druid/pull/19615
With https://github.com/apache/druid/pull/19571, when a concurrent replace task
upgrades a still-appending streaming task, the upgraded (new-version) copy of
each append segment previously adopted the pending segment's plain NumberedShardSpec,
dropping the DimensionValueSetShardSpec stamped at publish time. This made upgraded
segments unprunable by the broker.
The upgraded copy now takes its partition number and core-partition count
from the pending segment while carrying forward the original segment's
partitionDimensionValues when it is a DimensionValueSetShardSpec. The
value-set guarantee holds because the upgraded copy serves the same rows
as the original append segment.
This does not change the fact that pending segments are always allocated as numbered
shard specs, since tasks still advertise them as such — the value-set shard spec
currently applies only to recently-published segments.
This PR has:
- [x] been self-reviewed.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [x] been tested in a test Druid cluster.
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: preserve `DimensionValueSetShardSpec` on upgraded append segments (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 merged PR #19615:
URL: https://github.com/apache/druid/pull/19615
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: preserve `DimensionValueSetShardSpec` on upgraded append segments (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #19615:
URL: https://github.com/apache/druid/pull/19615#discussion_r3455147948
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -632,6 +738,82 @@
);
}
+ /**
+ * When a REPLACE commits over an interval with already-published APPEND segments held under a REPLACE lock, the
+ * upgraded (re-versioned) copies must preserve their {@link DimensionValueSetShardSpec} so they remain prunable by
+ * the broker.
+ */
+ @Test
+ public void testCommitReplaceSegments_upgradedPublishedSegmentPreservesDimensionValueSetShardSpec()
+ {
+ final ReplaceTaskLock replaceLock = new ReplaceTaskLock("g1", Intervals.of("2023-01-01/2023-02-01"), "2023-02-01");
+
+ final Map<String, List<String>> partitionDimensionValues = ImmutableMap.of("tenant_id", ImmutableList.of("tenant_a"));
+ // A published APPEND segment carrying a DimensionValueSetShardSpec (as stamped at publish time by the streaming task).
+ final DataSegment appendSegment = new DataSegment(
+ "foo",
+ Intervals.of("2023-01-01/2023-01-02"),
+ "2023-01-01",
+ ImmutableMap.of("path", "a-0"),
+ ImmutableList.of("dim1"),
+ ImmutableList.of("m1"),
+ new DimensionValueSetShardSpec(0, 1, partitionDimensionValues),
+ 9,
+ 100
+ );
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [DataSegment.DataSegment](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11314)
##########
server/src/test/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinatorTest.java:
##########
@@ -632,6 +738,82 @@
);
}
+ /**
+ * When a REPLACE commits over an interval with already-published APPEND segments held under a REPLACE lock, the
+ * upgraded (re-versioned) copies must preserve their {@link DimensionValueSetShardSpec} so they remain prunable by
+ * the broker.
+ */
+ @Test
+ public void testCommitReplaceSegments_upgradedPublishedSegmentPreservesDimensionValueSetShardSpec()
+ {
+ final ReplaceTaskLock replaceLock = new ReplaceTaskLock("g1", Intervals.of("2023-01-01/2023-02-01"), "2023-02-01");
+
+ final Map<String, List<String>> partitionDimensionValues = ImmutableMap.of("tenant_id", ImmutableList.of("tenant_a"));
+ // A published APPEND segment carrying a DimensionValueSetShardSpec (as stamped at publish time by the streaming task).
+ final DataSegment appendSegment = new DataSegment(
+ "foo",
+ Intervals.of("2023-01-01/2023-01-02"),
+ "2023-01-01",
+ ImmutableMap.of("path", "a-0"),
+ ImmutableList.of("dim1"),
+ ImmutableList.of("m1"),
+ new DimensionValueSetShardSpec(0, 1, partitionDimensionValues),
+ 9,
+ 100
+ );
+ segmentSchemaTestUtils.insertUsedSegments(Set.of(appendSegment), Collections.emptyMap());
+ insertIntoUpgradeSegmentsTable(
+ Map.of(appendSegment, replaceLock),
+ derbyConnectorRule.metadataTablesConfigSupplier().get()
+ );
+
+ final Set<DataSegment> replacingSegments = new HashSet<>();
+ for (int i = 0; i < 4; i++) {
+ replacingSegments.add(
+ new DataSegment(
+ "foo",
+ Intervals.of("2023-01-01/2023-02-01"),
+ "2023-02-01",
+ ImmutableMap.of("path", "b-" + i),
+ ImmutableList.of("dim1"),
+ ImmutableList.of("m1"),
+ new NumberedShardSpec(i, 4),
+ 9,
+ 100
+ )
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [DataSegment.DataSegment](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11315)
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org