FrankChen021 commented on code in PR #20558:
URL: https://github.com/apache/druid/pull/20558#discussion_r4270854924
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/TaskReportData.java:
##########
@@ -37,15 +38,17 @@ public class TaskReportData<PartitionIdType,
SequenceOffsetType>
private final Map<PartitionIdType, Long> lag;
private final Map<PartitionIdType, Long> lagMillis;
+ @JsonCreator
public TaskReportData(
- String id,
- @Nullable Map<PartitionIdType, SequenceOffsetType> startingOffsets,
- @Nullable Map<PartitionIdType, SequenceOffsetType> currentOffsets,
- @Nullable DateTime startTime,
- Long remainingSeconds,
- TaskType type,
- @Nullable Map<PartitionIdType, Long> lag,
- @Nullable Map<PartitionIdType, Long> lagMillis
+ @JsonProperty("id") String id,
+ // startingOffsets/currentOffsets/lag/lagMillis are left unbound from
JSON since we can't derive the map key type.
+ @JsonProperty("ignored1") @Nullable Map<PartitionIdType,
SequenceOffsetType> startingOffsets,
Review Comment:
[P1] Task serde discards offsets and lag
**Finding:** The getStatus() path generates reports with startingOffsets,
currentOffsets, lag, and lagMillis populated. Because the creator parameters
are named ignored1 through ignored4, Jackson ignores those JSON properties and
all four fields become null on deserialization, leaving consumers with
incomplete task reports.
**Suggestion:** Bind the parameters using concrete task generic types or
provide a custom deserializer so all four map properties are retained.
##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorReportPayload.java:
##########
@@ -49,7 +50,9 @@ public KafkaSupervisorReportPayload(
@JsonProperty("healthy") boolean healthy,
@JsonProperty("ignored1") SupervisorStateManager.State state, // No
serializer for State class
@JsonProperty("ignored2") SupervisorStateManager.State detailedState,
- @JsonProperty("recentErrors")
List<SupervisorStateManager.ExceptionEvent> recentErrors
+ @JsonProperty("recentErrors")
List<SupervisorStateManager.ExceptionEvent> recentErrors,
+ @JsonProperty("activeTasks") @Nullable List<TaskReportData> activeTasks,
Review Comment:
[P2] Preserve the existing Kafka payload constructor
**Finding:** Adding activeTasks and publishingTasks to the only public
KafkaSupervisorReportPayload constructor removes the previous 17-argument JVM
signature. Existing compiled callers can fail with NoSuchMethodError, and
source callers can fail to compile after upgrading, even though the base
payload class preserves its old constructor overload.
**Suggestion:** Keep a delegating overload with the previous signature that
supplies null task lists, while using the extended overload for Jackson.
--
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]
