mjsax commented on PR #23873: URL: https://github.com/apache/kafka/pull/23873#issuecomment-6281211645
Thanks for the PR. -- I am not sure if the proposed fix is the right one or not. Maybe @lucasbru @squah-confluent can chime in? My understanding is, that we currently present stale group metadata to the assignor, and thus, the assignor might incorrectly assign a standby to a member in the group, even if a "sibling member" (ie, member of a different thread within the same Kafka Streams clients) hold a corresponding active task. Example: Single instance in the group with two static members M1 and M2. M1 hold active tasks t-active. t-standby cannot be assigned, because we have only one instance in the group. We do a rolling bounce (with -2 temp leave), and M2 re-join first with a new processId. This triggers an assignor run, and we pass in M1 with the old member m1-old and old processId p-old, while we did re-map M2 onto an new memberId m2-new and use the new processId p-new. The assignor now incorrectly thinks there is two clients, and incorrectly assigns t-standby to M2. Later, when M1 re-joins, before this fix we incorrectly give t-active to M1, and with this fix, we hold off to give t-active to M1. When M1 re-joins, it also triggers a rebalance (which might be delayed base on assignment-interval config), and when the second rebalance runs, it now correctly sees both member using processId p-new, and would only assign t-active and omit t-standby again. So the problem is self-healing. However, I am wondering if it would be better to try to re-map all member of the same processId p-old, to p-new, whenever a member re-joins? There is one corner case though: it's possible that a client restarts with more members: for this case building on our example from above, if we re-start with 3 members and the new/third member joins first, it's not possible to link it to the old processId... So maybe the current fix is the best we can do anyway? -- Or course, the root cause of the issue is, that we trigger a rebalance on a new processId to begin with, and if we could fix this issue, it would be much better. Thoughts? -- 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]
