Fix known issues in group client #428
Conversation
| sasl); | ||
| newStream = newGroup::onApplication; | ||
|
|
||
| groupStreams.put(groupId, newGroup); |
There was a problem hiding this comment.
I think we can simplify this nested if.
if (group == null || HIGHLANDER_PROTOCOL.equals(protocol))
{
if (group != null)
{
// migrate group stream
}
else
{
// create group stream
}
}
seems like it can simplify to
if (group == null)
{
// create group stream
}
else if (HIGHLANDER_PROTOCOL.equals(protocol))
{
// migrate group stream
}
agree?
| @@ -1372,9 +1376,15 @@ private void onApplicationBegin( | |||
| topicMetadataLimit += metadataSize; | |||
| } | |||
|
|
|||
| initiateStream(); | |||
|
|
|||
There was a problem hiding this comment.
This seems like a no-op since state is not closed so can be removed, agree?
| { | ||
| stream.streamCleanup(traceId, traceId); | ||
| group.streamTakeover(traceId, traceId, application, originId, routedId, |
There was a problem hiding this comment.
Suggest changing to onApplicationMigrate(BeginFW, application).
Also, second parameter is authorization, not traceId, but that will get sorted out implicitly by above refactor.
| initialAck = 0; | ||
| replyAck = 0; | ||
| replySeq = 0; | ||
| state = 0; |
There was a problem hiding this comment.
Remove separate method and inline code at call-site (only one left after removal from onApplicationBegin).
|
|
||
| private int generationId; | ||
| private int nextJoinGroupRequestId; | ||
| private int nextJoinGroupResponseId; |
There was a problem hiding this comment.
If we are just detecting a request in flight, then is it necessary to track join-group requests separately, or does it still work if we just track generic requests?
There was a problem hiding this comment.
No that's what we discussed and it was your suggestion :) we had to track the join group requests separately
There was a problem hiding this comment.
Thanks, just confirming it is still necessary after the other changes we included. 😄
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)