lucasbru commented on code in PR #23341:
URL: https://github.com/apache/kafka/pull/23341#discussion_r3925556242
##########
clients/src/main/java/org/apache/kafka/common/requests/StreamsGroupHeartbeatRequest.java:
##########
@@ -49,6 +51,16 @@ public StreamsGroupHeartbeatRequest build(short version) {
// TaskOffsets/TaskEndOffsets are only supported by brokers
supporting v1+ request versions
data.setTaskOffsets(null);
data.setTaskEndOffsets(null);
+ // A v0 coordinator rejects a heartbeat that reports an owned
warm-up task, so a member that
+ // holds one after downgrading to v0 must report none. Unlike
the two fields above, null is
+ // not a safe unconditional value here: it means "unchanged",
but a v0 coordinator separately
+ // requires the three owned-task lists to be all null or all
non-null, and active/standby tasks stay
+ // null on the very same heartbeats where warmup tasks would.
So this only clears the list when it is
+ // non-null, i.e. only on a heartbeat that is (re-)sending the
assignment this round -- the same
+ // heartbeats where active/standby tasks are non-null too.
+ if (data.warmupTasks() != null) {
+ data.setWarmupTasks(List.of());
+ }
Review Comment:
I think this is better, but is it correct? So the member here still "owns" a
warm-up task, but doesn't report it. Just silently drops this information. The
assignment algorithm could assign the task as an active task or standby task to
a thread on the same process, because it doesn't know it already exists as a
warmup task.
The alternative would be to report warm-up tasks as standby-tasks, wdyt?
--
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]
