Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable EventHub Management Node to use RequestResponseChannelCache #41805

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

anuchandy
Copy link
Member

This is a follow up for #39107, in this PR weupdate the EventHub Management node to utilize RequestResponseChannelCache when V2 stack is opted-in as shown below –

final EventHubConsumerClient client = new EventHubClientBuilder()
        .connectionString(EH_CONNECTION_STRING)
        .consumerGroup("$Default")
        .configuration(new ConfigurationBuilder()
                .putProperty("com.azure.messaging.eventhubs.v2", "true")
                .build())
        .buildConsumerClient();

    // The API call that uses RequestResponseChannel obtained from RequestResponseChannelCache.
    client.getPartitionIds().forEach(partitionId -> {
        System.out.println(partitionId);
    });

If necessary, the new caches in v2 can be turned off by configuring 'com.azure.core.amqp.cache' to 'false'

@anuchandy anuchandy force-pushed the add-management-node-switch-for-v2 branch from 9d72b69 to 857e842 Compare September 11, 2024 15:42
@anuchandy anuchandy marked this pull request as ready for review September 11, 2024 15:45
@anuchandy anuchandy self-assigned this Sep 11, 2024
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@joshfree
Copy link
Member

If necessary, the new caches in v2 can be turned off by configuring 'com.azure.core.amqp.cache' to 'false'

Is there value in having a 'clearCachedData' API so that apps can flush the cache without restarting?

@joshfree
Copy link
Member

Could you include a regression test case?

Copy link
Member

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

+1 to add a test-case

@anuchandy
Copy link
Member Author

@joshfree

Is there value in having a 'clearCachedData' API so that apps can flush the cache without restarting?

We control it internally where cache gets cleared and refreshed automatically when cached channel gets disconnected like this and this

@anuchandy
Copy link
Member Author

anuchandy commented Sep 11, 2024

Could you include a regression test case?

This will be covered in the follow up pr (work item), that enables the existing "live tests" for v2 EH (like SB) then this path will get executed/exercised by the processor/management API test cases. (For the caches itself we have extensive "unit tests").

@anuchandy anuchandy merged commit adbd2f7 into Azure:main Sep 11, 2024
23 checks passed
@anuchandy anuchandy deleted the add-management-node-switch-for-v2 branch September 11, 2024 16:43
jairmyree pushed a commit to jairmyree/azure-sdk-for-java that referenced this pull request Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants