mjsax commented on code in PR #23341:
URL: https://github.com/apache/kafka/pull/23341#discussion_r3927400850
##########
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:
Yes, it's information loss, but I think it's correct. The assignor does not
assign warmup tasks, and on older broker there is no refiner to inject warmups
either, so the next assignment won't have any warmups. -- The assignor might
assign as active or standby, by the warmup would go away in the next assignment
sent to the client. Thus, the existing warmup task would just be converted
in-place from warmup -> active or warmup -> standby (which is a no-op, because
client side warmup and standby are the same thing anyway). KS has corresponding
conversion logic.
Reporting as warmup would break things, (cf my other reply) and the member
could get fenced, what would make it worse.
--
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]
