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

[fix][broker] Support advertised listeners when gracefully transferring topics (ExtensibleLoadManagerImpl only) #22862

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

heesung-sn
Copy link
Contributor

PIP: https://github.com/apache/pulsar/blob/master/pip/pip-307.md

Motivation

  • PIP-307 does not cover advertised listeners. This PR is trying to support advertised listeners when gracefully transferring topics (ExtensibleLoadManagerImpl only)

Modifications

By utilizing serverCnx.listenerName, added by #12056, brokers return advertised listener broker urls in the producer|consumer close commands, if the connection is through the listenerName.

if(serverCnx.listenerName != null){
  listener =  lookup.getAdvertisedListener(serverCnx.listenerName)
  return listener's brokerServiceUrl and brokerServiceUrlTls
} else {
  return  lookup.pulsarServiceUrl and lookup.pulsarServiceUrlTls
}

Verifying this change

  • Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: heesung-sn#68

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 6, 2024
@heesung-sn heesung-sn self-assigned this Jun 6, 2024
@heesung-sn heesung-sn added this to the 3.4.0 milestone Jun 6, 2024
@lhotari
Copy link
Member

lhotari commented Jun 6, 2024

good work @heesung-sn!

@heesung-sn heesung-sn requested a review from lhotari June 6, 2024 21:04
@heesung-sn heesung-sn closed this Jun 7, 2024
@heesung-sn heesung-sn reopened this Jun 7, 2024
@heesung-sn heesung-sn merged commit 5af0595 into apache:master Jun 7, 2024
68 of 72 checks passed
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
…ng topics (ExtensibleLoadManagerImpl only) (apache#22862)

(cherry picked from commit 5af0595)
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
…ng topics (ExtensibleLoadManagerImpl only) (apache#22862)

(cherry picked from commit 5af0595)
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
…ng topics (ExtensibleLoadManagerImpl only) (apache#22862)

(cherry picked from commit 5af0595)
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.

3 participants