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

[pulsar-broker] Handle lookup redirect for V1-topics with different cluster #12743

Merged
merged 1 commit into from
Nov 11, 2021

Conversation

rdhabalia
Copy link
Contributor

Motivation

Right now, if pulsar broker is not redirecting lookup requests if V1-topic belongs to the different region instead it gives Authorization Exception.

23:48:06.480 [pulsar-client-io-1-1] WARN  org.apache.pulsar.client.impl.BinaryProtoLookupService - [persistent://p1/us-went1/ns/t1] failed to get Partitioned metadata : 
Proxy Client is not authorized to Get Partition Metadata  yrid: 
org.apache.pulsar.client.api.PulsarClientException$AuthorizationException: Proxy Client is not authorized to Get Partition Metadata

Modification

Broker should be able to redirect lookup to appropriate cluster so, user doesn't have to change broker URL and able to connect to right region after following redirect.

@github-actions
Copy link

@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

}
return pulsarResources.getClusterResources().listAsync()
.thenApply(clusters -> {
log.info("clusters {}, cluster {}",clusters, topicName.getCluster());
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be printed out maty times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh.. that was for testing. let me remove it.

@Anonymitaet
Copy link
Member

@rdhabalia seems this is a bug fix and do not affect docs?

@codelipenghui codelipenghui merged commit 25cbfad into apache:master Nov 11, 2021
@rdhabalia rdhabalia deleted the lookup_cluster branch November 11, 2021 17:58
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
codelipenghui pushed a commit that referenced this pull request Dec 20, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 20, 2021
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.

4 participants