Skip to content

Conversation

@anitarua
Copy link
Contributor

@anitarua anitarua commented May 21, 2025

Work towards https://github.com/momentohq/dev-eco-issue-tracker/issues/1231

  • Found that the java sdk does not keep track of the number of concurrent subscriptions while auditing the implementations of separate grpc channels for publish and subscribe.

    • This PR adds the necessary bookkeeping so that the TopicClient can reject new subscribe requests if all grpc channels are currently occupied with active streams.
    • IIRC, Java has a multi-threaded async runtime, meaning multiple threads could be simultaneously handling multiple subscribe requests. Need to ensure the bookkeeping takes this into account so bursts of subscribe/unsubscribe requests are handled properly.
  • Also added a new retry test file to verify that the TopicClient does not allow >100 streams on a single grpc channel and that it is capable of handling bursts of subscribe and unsubscribe requests.

Note: this PR is based on top of Nate's PR for other subscriptions improvements

@anitarua anitarua force-pushed the subscriptions-bookkeeping branch from b0b798c to 31c5499 Compare May 30, 2025 20:23
@anitarua anitarua changed the base branch from main to subscription-retry-improvements May 30, 2025 20:23
Base automatically changed from subscription-retry-improvements to main June 3, 2025 16:18
@anitarua anitarua force-pushed the subscriptions-bookkeeping branch from 6f6ffd6 to 69250c6 Compare June 3, 2025 16:43
@anitarua anitarua marked this pull request as ready for review June 3, 2025 18:45
@anitarua anitarua requested a review from a team June 3, 2025 18:52
@anitarua anitarua requested a review from malandis June 4, 2025 22:10
Comment on lines +150 to +160
for (int i = 0; i < maximumActiveSubscriptions; i++) {
final StreamStubWithCount stubWithCount =
streamStubs.get(streamIndex.getAndIncrement() % this.numStreamGrpcChannels);
try {
stubWithCount.acquireStubOrThrow();
return stubWithCount;
} catch (ClientSdkException e) {
// If the stub is at capacity, continue to the next one.
continue;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@anitarua anitarua merged commit b5acdd4 into main Jun 4, 2025
6 checks passed
@anitarua anitarua deleted the subscriptions-bookkeeping branch June 4, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants