lucasbru commented on PR #22245: URL: https://github.com/apache/kafka/pull/22245#issuecomment-4496329419
### Code review Found 1 issue: 1. `hasStreamsMemberMetadataChanged` unconditionally adds a `StreamsGroupMemberMetadata` record even when `hasEpochRelevantMemberConfigChanged` returns `false` (i.e., no epoch-relevant change during static member rejoin). This means two `StreamsGroupMemberMetadata` records are written for the same new `memberId` — one from `replaceStreamsMember` (carrying old processId/rackId from the copied member) and one from `hasStreamsMemberMetadataChanged` (carrying updated values). The final write wins so correctness is preserved, but this is an unnecessary double-write that differs from the consumer group's `replaceMember` + `hasMemberSubscriptionChanged` pattern, where the metadata record is only written once during replacement. The consumer group path avoids this by writing the final (updated) record directly in `replaceMember`. For Streams, `replaceStreamsMember` writes the *copied* (pre-update) member metadata, and then `hasStreamsMemberMetadataChanged` overwrites it with the updated values unconditionally any time fields like `clientId` or `clientHost` differ — which they always will across reconnects. This is a latent correctness concern if the record ordering ever matters, and it produces unnecessary log churn. Compare the consumer group analog in `replaceMember`: https://github.com/apache/kafka/blob/441be69a5f3c47ef7f141ffbc17ab488edb42c07/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java#L4464-L4486 vs. the new `replaceStreamsMember`: https://github.com/apache/kafka/blob/441be69a5f3c47ef7f141ffbc17ab488edb42c07/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java#L4723-L4765 The fix would be to write the final (post-update) member into `replaceStreamsMember` instead of the intermediate copy, or to skip writing the member record in `hasStreamsMemberMetadataChanged` when the record was already emitted by `replaceStreamsMember`. 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- 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]
