ableegoldman commented on code in PR #12869:
URL: https://github.com/apache/kafka/pull/12869#discussion_r1025887660
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java:
##########
@@ -298,6 +298,7 @@ public boolean isRunning() {
private volatile ThreadMetadata threadMetadata;
private StreamThread.StateListener stateListener;
private final Optional<String> getGroupInstanceID;
+ private final String threadIdSuffix; // shortened version of the threadId:
{processUUID}-StreamThread-{threadIdx}
Review Comment:
Ah shoot, good point :/ This was my attempt at a compromise between staying
relatively short, and containing enough info to actually be useful. Maybe I'm
being overly paranoid, but we have twice had to revert the commit which added
the `reason` message to `#enforceRebalance` due to perf issues related to
strings/string length -- and I've seen some VERY long application/client ids.
I'm not sure how long is too long, but if you look into the underlying
consumer/coordinator methods, everywhere else we pass in a "full reason" for
logging plus a "short reason" for actually embedding in the request, due to
said perf regression with longer strings.
But maybe we don't actually need the client id here after all? I'm not sure
how this is ultimately used but I would hope the broker would log the client id
for a given `reason` string in the JoinGroup request..I'm just going to remove
this part for now and if we do find it would be useful, I can do a separate PR.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
