Skip to content

Commit a3362c4

Browse files
lucliu1108lucasbru
andauthored
KAFKA-20625: Add StreamsGroupTopologyDescriptionRequestManager and Streams client topology push [2/N] (apache#22640)
## Summary Adds the client-side request manager that pushes topology descriptions to the broker, wires the heartbeat path to flip the push-required flag on responses, and registers the new manager in the consumer network thread. Should be based on apache#22639 ## File changes - **`StreamsGroupTopologyDescriptionRequestManager`**: new RequestManager that implements the KIP-1331 response handling with unit tests. - **`StreamsGroupHeartbeatRequestManager`**: `onSuccessResponse` propagates `memberId` and sets `topologyPushRequired` when the broker requests a push, with unit tests. - **`RequestManagers`**: registers the new manager in the Streams branch and the entries list. Current tests updated. - **`StreamThread.initStreamsRebalanceData`**: bumped to package-private for testing. 2 new tests verify the wire description is populated/skipped based on the config. Reviewers: Lucas Brutschy <lbrutschy@confluent.io> --------- Co-authored-by: Lucas Brutschy <lbrutschy@confluent.io>
1 parent 5a91e40 commit a3362c4

9 files changed

Lines changed: 681 additions & 4 deletions

File tree

clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestManagers.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public class RequestManagers implements Closeable {
6161
public final FetchRequestManager fetchRequestManager;
6262
public final Optional<ShareConsumeRequestManager> shareConsumeRequestManager;
6363
public final Optional<StreamsGroupHeartbeatRequestManager> streamsGroupHeartbeatRequestManager;
64+
public final Optional<StreamsGroupTopologyDescriptionRequestManager> streamsGroupTopologyDescriptionRequestManager;
6465
private final List<RequestManager> entries;
6566
private final IdempotentCloser closer = new IdempotentCloser();
6667

@@ -73,6 +74,7 @@ public RequestManagers(LogContext logContext,
7374
Optional<ConsumerHeartbeatRequestManager> heartbeatRequestManager,
7475
Optional<ConsumerMembershipManager> membershipManager,
7576
Optional<StreamsGroupHeartbeatRequestManager> streamsGroupHeartbeatRequestManager,
77+
Optional<StreamsGroupTopologyDescriptionRequestManager> streamsGroupTopologyDescriptionRequestManager,
7678
Optional<StreamsMembershipManager> streamsMembershipManager) {
7779
this.log = logContext.logger(RequestManagers.class);
7880
this.offsetsRequestManager = requireNonNull(offsetsRequestManager, "OffsetsRequestManager cannot be null");
@@ -84,6 +86,7 @@ public RequestManagers(LogContext logContext,
8486
this.consumerHeartbeatRequestManager = heartbeatRequestManager;
8587
this.shareHeartbeatRequestManager = Optional.empty();
8688
this.streamsGroupHeartbeatRequestManager = streamsGroupHeartbeatRequestManager;
89+
this.streamsGroupTopologyDescriptionRequestManager = streamsGroupTopologyDescriptionRequestManager;
8790
this.consumerMembershipManager = membershipManager;
8891
this.streamsMembershipManager = streamsMembershipManager;
8992
this.shareMembershipManager = Optional.empty();
@@ -94,6 +97,7 @@ public RequestManagers(LogContext logContext,
9497
heartbeatRequestManager.ifPresent(list::add);
9598
membershipManager.ifPresent(list::add);
9699
streamsGroupHeartbeatRequestManager.ifPresent(list::add);
100+
streamsGroupTopologyDescriptionRequestManager.ifPresent(list::add);
97101
streamsMembershipManager.ifPresent(list::add);
98102
list.add(offsetsRequestManager);
99103
list.add(topicMetadataRequestManager);
@@ -112,6 +116,7 @@ public RequestManagers(LogContext logContext,
112116
this.commitRequestManager = Optional.empty();
113117
this.consumerHeartbeatRequestManager = Optional.empty();
114118
this.streamsGroupHeartbeatRequestManager = Optional.empty();
119+
this.streamsGroupTopologyDescriptionRequestManager = Optional.empty();
115120
this.shareHeartbeatRequestManager = shareHeartbeatRequestManager;
116121
this.consumerMembershipManager = Optional.empty();
117122
this.streamsMembershipManager = Optional.empty();
@@ -199,6 +204,7 @@ protected RequestManagers create() {
199204
CoordinatorRequestManager coordinator = null;
200205
CommitRequestManager commitRequestManager = null;
201206
StreamsGroupHeartbeatRequestManager streamsGroupHeartbeatRequestManager = null;
207+
StreamsGroupTopologyDescriptionRequestManager streamsGroupTopologyDescriptionRequestManager = null;
202208
StreamsMembershipManager streamsMembershipManager = null;
203209

204210
if (groupRebalanceConfig != null && groupRebalanceConfig.groupId != null) {
@@ -247,6 +253,16 @@ protected RequestManagers create() {
247253
metrics,
248254
streamsRebalanceData.get()
249255
);
256+
257+
streamsGroupTopologyDescriptionRequestManager = new StreamsGroupTopologyDescriptionRequestManager(
258+
logContext,
259+
time,
260+
retryBackoffMs,
261+
retryBackoffMaxMs,
262+
streamsMembershipManager,
263+
streamsRebalanceData.get(),
264+
coordinator
265+
);
250266
} else {
251267
membershipManager = new ConsumerMembershipManager(
252268
groupRebalanceConfig.groupId,
@@ -308,6 +324,7 @@ protected RequestManagers create() {
308324
Optional.ofNullable(heartbeatRequestManager),
309325
Optional.ofNullable(membershipManager),
310326
Optional.ofNullable(streamsGroupHeartbeatRequestManager),
327+
Optional.ofNullable(streamsGroupTopologyDescriptionRequestManager),
311328
Optional.ofNullable(streamsMembershipManager)
312329
);
313330
}

clients/src/main/java/org/apache/kafka/clients/consumer/internals/StreamsGroupHeartbeatRequestManager.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,10 @@ private void onSuccessResponse(final StreamsGroupHeartbeatResponse response, fin
571571
streamsRebalanceData.setTaskOffsetIntervalMs(data.taskOffsetIntervalMs());
572572
streamsRebalanceData.setAcceptableRecoveryLag(data.acceptableRecoveryLag());
573573

574+
if (data.topologyDescriptionRequired() && streamsRebalanceData.wireTopologyDescription() != null) {
575+
streamsRebalanceData.setTopologyPushRequired(true);
576+
}
577+
574578
if (data.partitionsByUserEndpoint() != null) {
575579
streamsRebalanceData.setPartitionsByHost(convertHostInfoMap(data));
576580
}
@@ -673,6 +677,7 @@ private void onErrorResponse(final StreamsGroupHeartbeatResponse response, final
673677
membershipManager.onFenced();
674678
// Skip backoff so that a next HB to rejoin is sent as soon as the fenced member releases its assignment
675679
heartbeatRequestState.reset();
680+
streamsRebalanceData.setTopologyPushRequired(false);
676681
break;
677682

678683
case UNKNOWN_MEMBER_ID:
@@ -685,6 +690,7 @@ private void onErrorResponse(final StreamsGroupHeartbeatResponse response, final
685690
membershipManager.onFenced();
686691
// Skip backoff so that a next HB to rejoin is sent as soon as the fenced member releases its assignment
687692
heartbeatRequestState.reset();
693+
streamsRebalanceData.setTopologyPushRequired(false);
688694
break;
689695

690696
case UNSUPPORTED_VERSION:
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.kafka.clients.consumer.internals;
18+
19+
import org.apache.kafka.clients.ClientResponse;
20+
import org.apache.kafka.common.errors.RetriableException;
21+
import org.apache.kafka.common.message.StreamsGroupTopologyDescriptionUpdateRequestData;
22+
import org.apache.kafka.common.protocol.Errors;
23+
import org.apache.kafka.common.requests.StreamsGroupTopologyDescriptionUpdateRequest;
24+
import org.apache.kafka.common.requests.StreamsGroupTopologyDescriptionUpdateResponse;
25+
import org.apache.kafka.common.utils.Time;
26+
import org.apache.kafka.common.utils.internals.LogContext;
27+
28+
import org.slf4j.Logger;
29+
30+
import java.util.Collections;
31+
import java.util.Objects;
32+
33+
public class StreamsGroupTopologyDescriptionRequestManager implements RequestManager {
34+
35+
private final Logger logger;
36+
private final Time time;
37+
private final StreamsMembershipManager membershipManager;
38+
private final StreamsRebalanceData streamsRebalanceData;
39+
private final CoordinatorRequestManager coordinatorRequestManager;
40+
private final RequestState pushRequestState;
41+
42+
private long nextPushTimeMs = 0L;
43+
44+
public StreamsGroupTopologyDescriptionRequestManager(final LogContext logContext,
45+
final Time time,
46+
final long retryBackoffMs,
47+
final long retryBackoffMaxMs,
48+
final StreamsMembershipManager membershipManager,
49+
final StreamsRebalanceData streamsRebalanceData,
50+
final CoordinatorRequestManager coordinatorRequestManager) {
51+
this.logger = logContext.logger(getClass());
52+
this.time = Objects.requireNonNull(time);
53+
this.membershipManager = Objects.requireNonNull(membershipManager);
54+
this.streamsRebalanceData = Objects.requireNonNull(streamsRebalanceData);
55+
this.coordinatorRequestManager = Objects.requireNonNull(coordinatorRequestManager);
56+
this.pushRequestState = new RequestState(
57+
logContext,
58+
StreamsGroupTopologyDescriptionRequestManager.class.getSimpleName(),
59+
retryBackoffMs,
60+
retryBackoffMaxMs);
61+
}
62+
63+
@Override
64+
public NetworkClientDelegate.PollResult poll(final long currentTimeMs) {
65+
if (!shouldSendTopologyDescriptionUpdate(currentTimeMs)) {
66+
return NetworkClientDelegate.PollResult.EMPTY;
67+
}
68+
69+
final StreamsGroupTopologyDescriptionUpdateRequestData data = new StreamsGroupTopologyDescriptionUpdateRequestData()
70+
.setGroupId(membershipManager.groupId())
71+
.setMemberId(membershipManager.memberId())
72+
.setTopologyEpoch(streamsRebalanceData.topologyEpoch())
73+
.setTopologyDescription(streamsRebalanceData.wireTopologyDescription());
74+
75+
final NetworkClientDelegate.UnsentRequest unsent = new NetworkClientDelegate.UnsentRequest(
76+
new StreamsGroupTopologyDescriptionUpdateRequest.Builder(data),
77+
coordinatorRequestManager.coordinator()
78+
);
79+
unsent.whenComplete((response, exception) -> onResponse(response, exception));
80+
81+
pushRequestState.onSendAttempt(currentTimeMs);
82+
return new NetworkClientDelegate.PollResult(Collections.singletonList(unsent));
83+
}
84+
85+
@Override
86+
public long maximumTimeToWait(final long currentTimeMs) {
87+
if (!streamsRebalanceData.topologyPushRequired()) {
88+
return Long.MAX_VALUE;
89+
}
90+
final long backoffRemainingMs = pushRequestState.remainingBackoffMs(currentTimeMs);
91+
final long throttleRemainingMs = Math.max(0L, nextPushTimeMs - currentTimeMs);
92+
final long waitMs = Math.max(backoffRemainingMs, throttleRemainingMs);
93+
if (waitMs > 0L) {
94+
return waitMs;
95+
}
96+
return shouldSendTopologyDescriptionUpdate(currentTimeMs) ? 0L : Long.MAX_VALUE;
97+
}
98+
99+
private boolean shouldSendTopologyDescriptionUpdate(final long currentTimeMs) {
100+
if (!pushRequestState.canSendRequest(currentTimeMs) || currentTimeMs < nextPushTimeMs) {
101+
return false;
102+
}
103+
if (!streamsRebalanceData.topologyPushRequired() || streamsRebalanceData.wireTopologyDescription() == null) {
104+
return false;
105+
}
106+
final String memberId = membershipManager.memberId();
107+
if (memberId == null || memberId.isEmpty()) {
108+
return false;
109+
}
110+
return coordinatorRequestManager.coordinator().isPresent();
111+
}
112+
113+
private void onResponse(final ClientResponse response, final Throwable exception) {
114+
final long responseTimeMs = time.milliseconds();
115+
116+
if (exception != null) {
117+
if (exception instanceof RetriableException) {
118+
pushRequestState.onFailedAttempt(responseTimeMs);
119+
coordinatorRequestManager.handleCoordinatorDisconnect(exception, responseTimeMs);
120+
logger.warn("Topology description push failed with retriable exception; will retry on next poll", exception);
121+
} else {
122+
// Non-retriable exceptions should clear the flag and give up.
123+
pushRequestState.onSuccessfulAttempt(responseTimeMs);
124+
streamsRebalanceData.setTopologyPushRequired(false);
125+
logger.warn("Topology description push failed with non-retriable exception.", exception);
126+
}
127+
return;
128+
}
129+
130+
final StreamsGroupTopologyDescriptionUpdateResponse body =
131+
(StreamsGroupTopologyDescriptionUpdateResponse) response.responseBody();
132+
final Errors error = Errors.forCode(body.data().errorCode());
133+
final String errorMessage = body.data().errorMessage();
134+
135+
if (body.data().throttleTimeMs() > 0) {
136+
nextPushTimeMs = responseTimeMs + body.data().throttleTimeMs();
137+
}
138+
139+
switch (error) {
140+
case NONE:
141+
pushRequestState.onSuccessfulAttempt(responseTimeMs);
142+
streamsRebalanceData.setTopologyPushRequired(false);
143+
break;
144+
145+
case NOT_COORDINATOR:
146+
case COORDINATOR_NOT_AVAILABLE:
147+
pushRequestState.onFailedAttempt(responseTimeMs);
148+
logger.info("Coordinator error {} pushing topology description. Will rediscover and retry: {}", error, errorMessage);
149+
coordinatorRequestManager.markCoordinatorUnknown(errorMessage, responseTimeMs);
150+
break;
151+
152+
case COORDINATOR_LOAD_IN_PROGRESS:
153+
pushRequestState.onFailedAttempt(responseTimeMs);
154+
logger.info("Coordinator is loading; will retry on next poll: {}", errorMessage);
155+
break;
156+
157+
case UNKNOWN_MEMBER_ID:
158+
// Member was dropped — clear the flag and let the heartbeat path drive the rejoin.
159+
// onSuccessfulAttempt resets request state without backoff since no retry follows.
160+
pushRequestState.onSuccessfulAttempt(responseTimeMs);
161+
logger.info("Topology description push rejected with UNKNOWN_MEMBER_ID; heartbeat will trigger rejoin: {}", errorMessage);
162+
streamsRebalanceData.setTopologyPushRequired(false);
163+
break;
164+
165+
case STREAMS_TOPOLOGY_DESCRIPTION_UPDATE_FAILED:
166+
case INVALID_REQUEST:
167+
case UNSUPPORTED_VERSION:
168+
case GROUP_ID_NOT_FOUND:
169+
case GROUP_AUTHORIZATION_FAILED:
170+
default:
171+
// Use onSuccessfulAttempt because no retry follows,
172+
// a future push triggered by a later heartbeat should not inherit backoff from this failure.
173+
// the broker will re-signal via heartbeat if a push is needed again.
174+
pushRequestState.onSuccessfulAttempt(responseTimeMs);
175+
logger.warn("Topology description push failed with {}: {}", error, errorMessage);
176+
streamsRebalanceData.setTopologyPushRequired(false);
177+
break;
178+
}
179+
}
180+
181+
}

clients/src/test/java/org/apache/kafka/clients/consumer/internals/RequestManagersTest.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
public class RequestManagersTest {
4040

4141
@Test
42-
public void testMemberStateListenerRegistered() {
42+
public void testConsumerGroupRequestManagersAndListenersWired() {
4343

4444
final MemberStateListener listener = (memberEpoch, memberId) -> { };
4545

@@ -77,6 +77,7 @@ public void testMemberStateListenerRegistered() {
7777
assertTrue(requestManagers.consumerMembershipManager.isPresent());
7878
assertTrue(requestManagers.streamsMembershipManager.isEmpty());
7979
assertTrue(requestManagers.streamsGroupHeartbeatRequestManager.isEmpty());
80+
assertTrue(requestManagers.streamsGroupTopologyDescriptionRequestManager.isEmpty());
8081

8182
assertEquals(2, requestManagers.consumerMembershipManager.get().stateListeners().size());
8283
assertTrue(requestManagers.consumerMembershipManager.get().stateListeners().stream()
@@ -85,7 +86,7 @@ public void testMemberStateListenerRegistered() {
8586
}
8687

8788
@Test
88-
public void testStreamMemberStateListenerRegistered() {
89+
public void testStreamsGroupRequestManagersAndListenersWired() {
8990

9091
final MemberStateListener listener = (memberEpoch, memberId) -> { };
9192

@@ -122,6 +123,9 @@ public void testStreamMemberStateListenerRegistered() {
122123
).get();
123124
assertTrue(requestManagers.streamsMembershipManager.isPresent());
124125
assertTrue(requestManagers.streamsGroupHeartbeatRequestManager.isPresent());
126+
assertTrue(requestManagers.streamsGroupTopologyDescriptionRequestManager.isPresent());
127+
assertTrue(requestManagers.entries().stream()
128+
.anyMatch(rm -> rm instanceof StreamsGroupTopologyDescriptionRequestManager));
125129
assertTrue(requestManagers.consumerMembershipManager.isEmpty());
126130

127131
assertEquals(2, requestManagers.streamsMembershipManager.get().stateListeners().size());

0 commit comments

Comments
 (0)