github-advanced-security[bot] commented on code in PR #19409:
URL: https://github.com/apache/druid/pull/19409#discussion_r3190905396
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/SegmentActionHandler.java:
##########
@@ -36,6 +37,26 @@
*/
void replicateSegment(DataSegment segment, Map<String, Integer>
tierToReplicaCount);
+ /**
+ * Like {@link #replicateSegment(DataSegment, Map)} but for a partial-load
rule. The given {@code profile} carries
+ * the partial load spec and rule fingerprint that the historicals are being
asked to load and that the
+ * coordinator uses to reconcile already-loaded replicas against the rule's
request.
+ * <p>
+ * Default implementation throws {@link UnsupportedOperationException}:
callers that don't intend to support partial
+ * load rules (e.g., minimal mock handlers used in unit tests) can leave it
unimplemented. The production handler
+ * {@code StrategicSegmentAssigner} overrides this to do fingerprint-aware
replica counting.
+ */
+ default void replicateSegmentPartially(
+ DataSegment segment,
Review Comment:
## CodeQL / Useless parameter
The parameter 'segment' is never used.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11174)
##########
server/src/test/java/org/apache/druid/server/coordination/SegmentChangeRequestLoadTest.java:
##########
@@ -76,4 +77,79 @@
Assert.assertEquals(IndexIO.CURRENT_VERSION_ID,
objectMap.get("binaryVersion"));
Assert.assertEquals(1, objectMap.get("size"));
}
+
+ @Test
+ public void testPartialLoadFieldsOmittedWhenNull() throws Exception
+ {
+ // Coordinator-issued load requests leave the partial-load fields null;
the wire payload should not include them.
+ ObjectMapper mapper = new DefaultObjectMapper();
+ DataSegment segment = new DataSegment(
+ "ds",
+ Intervals.of("2024-01-01/2024-02-01"),
+ "v1",
+ Map.of("type", "local"),
+ List.of("d"),
+ List.of("m"),
+ NoneShardSpec.instance(),
+ IndexIO.CURRENT_VERSION_ID,
+ 1
+ );
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/11171)
##########
server/src/test/java/org/apache/druid/server/coordination/SegmentChangeRequestLoadTest.java:
##########
@@ -76,4 +77,79 @@
Assert.assertEquals(IndexIO.CURRENT_VERSION_ID,
objectMap.get("binaryVersion"));
Assert.assertEquals(1, objectMap.get("size"));
}
+
+ @Test
+ public void testPartialLoadFieldsOmittedWhenNull() throws Exception
+ {
+ // Coordinator-issued load requests leave the partial-load fields null;
the wire payload should not include them.
+ ObjectMapper mapper = new DefaultObjectMapper();
+ DataSegment segment = new DataSegment(
+ "ds",
+ Intervals.of("2024-01-01/2024-02-01"),
+ "v1",
+ Map.of("type", "local"),
+ List.of("d"),
+ List.of("m"),
+ NoneShardSpec.instance(),
+ IndexIO.CURRENT_VERSION_ID,
+ 1
+ );
+ SegmentChangeRequestLoad load = new SegmentChangeRequestLoad(segment);
+ Map<String, Object> objectMap = mapper.readValue(
+ mapper.writeValueAsString(load),
+ JacksonUtils.TYPE_REFERENCE_MAP_STRING_OBJECT
+ );
+ Assert.assertFalse(objectMap.containsKey("fingerprint"));
+ Assert.assertFalse(objectMap.containsKey("loadedBytes"));
+ }
+
+ @Test
+ public void testPartialLoadFieldsRoundTrip() throws Exception
+ {
+ // Historical announcement with partial-load metadata: round-trip
preserves both fields.
+ ObjectMapper mapper = new DefaultObjectMapper();
+ DataSegment segment = new DataSegment(
+ "ds",
+ Intervals.of("2024-01-01/2024-02-01"),
+ "v1",
+ Map.of("type", "local"),
+ List.of("d"),
+ List.of("m"),
+ NoneShardSpec.instance(),
+ IndexIO.CURRENT_VERSION_ID,
+ 1
+ );
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/11172)
##########
server/src/test/java/org/apache/druid/server/coordination/SegmentChangeRequestLoadTest.java:
##########
@@ -76,4 +77,79 @@
Assert.assertEquals(IndexIO.CURRENT_VERSION_ID,
objectMap.get("binaryVersion"));
Assert.assertEquals(1, objectMap.get("size"));
}
+
+ @Test
+ public void testPartialLoadFieldsOmittedWhenNull() throws Exception
+ {
+ // Coordinator-issued load requests leave the partial-load fields null;
the wire payload should not include them.
+ ObjectMapper mapper = new DefaultObjectMapper();
+ DataSegment segment = new DataSegment(
+ "ds",
+ Intervals.of("2024-01-01/2024-02-01"),
+ "v1",
+ Map.of("type", "local"),
+ List.of("d"),
+ List.of("m"),
+ NoneShardSpec.instance(),
+ IndexIO.CURRENT_VERSION_ID,
+ 1
+ );
+ SegmentChangeRequestLoad load = new SegmentChangeRequestLoad(segment);
+ Map<String, Object> objectMap = mapper.readValue(
+ mapper.writeValueAsString(load),
+ JacksonUtils.TYPE_REFERENCE_MAP_STRING_OBJECT
+ );
+ Assert.assertFalse(objectMap.containsKey("fingerprint"));
+ Assert.assertFalse(objectMap.containsKey("loadedBytes"));
+ }
+
+ @Test
+ public void testPartialLoadFieldsRoundTrip() throws Exception
+ {
+ // Historical announcement with partial-load metadata: round-trip
preserves both fields.
+ ObjectMapper mapper = new DefaultObjectMapper();
+ DataSegment segment = new DataSegment(
+ "ds",
+ Intervals.of("2024-01-01/2024-02-01"),
+ "v1",
+ Map.of("type", "local"),
+ List.of("d"),
+ List.of("m"),
+ NoneShardSpec.instance(),
+ IndexIO.CURRENT_VERSION_ID,
+ 1
+ );
+ SegmentChangeRequestLoad load = new SegmentChangeRequestLoad(
+ segment,
+ "v1:abcdef0123456789",
+ 12345L
+ );
+ String json = mapper.writeValueAsString(load);
+ SegmentChangeRequestLoad reread = mapper.readValue(json,
SegmentChangeRequestLoad.class);
+ Assert.assertEquals(load, reread);
+ Assert.assertEquals("v1:abcdef0123456789", reread.getFingerprint());
+ Assert.assertEquals(Long.valueOf(12345L), reread.getLoadedBytes());
+ }
+
+ @Test
+ public void testOldPayloadDeserializesWithoutPartialFields() throws Exception
+ {
+ // An old-version payload with no partial-load fields should deserialize
cleanly; the partial fields are null.
+ ObjectMapper mapper = new DefaultObjectMapper();
+ DataSegment segment = new DataSegment(
+ "ds",
+ Intervals.of("2024-01-01/2024-02-01"),
+ "v1",
+ Map.of("type", "local"),
+ List.of("d"),
+ List.of("m"),
+ NoneShardSpec.instance(),
+ IndexIO.CURRENT_VERSION_ID,
+ 1
+ );
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/11173)
--
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]
