Skip to content

[improve][pip] PIP-425: Support connecting with next available endpoint for multi-endpoint serviceUrls #24394

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.

Sign up for GitHub

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

Jump to bottom
Merged
liangyepianzhou merged 14 commits into from
Jun 25, 2025
Merged

[improve][pip] PIP-425: Support connecting with next available endpoint for multi-endpoint serviceUrls #24394

liangyepianzhou merged 14 commits into from
Jun 25, 2025

Conversation

@AuroraTwinkle
Copy link
Contributor

@AuroraTwinkle AuroraTwinkle commented Jun 7, 2025 •
edited
Loading

Fixes #22934 (comment)

Main Issue: #22934 (comment)

Implementation: #24387

Motivation

As #22934 and #22933 mentioned, when most of the nodes in serviceurl are down (but there is at least one available node), creating consumers and producers through PulsarClient will most likely fail. I think this is not as expected. If the code is robust enough, as long as there is one available node, it should be accessible normally. Therefore, this pip is going to optimize the logic, remove unavailable nodes through the feedback mechanism, and improve the success rate of PulsarClient requests.

By the way, #22935 removes faulty nodes through a regular health check mechanism, but this brings new problems (frequent creation of connections and increased system load), so this solution is abandoned. See #22934 (comment) for more details!

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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:

[improve][pip] PIP-425: fix problem that consumer or producer create …
d7dd357 
…failed when many nodes in PulsarClient serviceUrl become unavailable
@github-actions
Copy link

github-actions bot commented Jun 7, 2025

@AuroraTwinkle Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Jun 7, 2025
@AuroraTwinkle AuroraTwinkle mentioned this pull request Jun 7, 2025
Merged
15 tasks
@AuroraTwinkle AuroraTwinkle marked this pull request as ready for review June 7, 2025 10:02
@AuroraTwinkle AuroraTwinkle mentioned this pull request Jun 7, 2025
Closed
2 tasks
lhotari
lhotari reviewed Jun 9, 2025
View reviewed changes
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Some initial feedback

@AuroraTwinkle
Copy link
Contributor Author

AuroraTwinkle commented Jun 9, 2025 •
edited
Loading

Some initial feedback

@lhotari All good suggestions, I will fix them and update later. Thanks for your help!

@AuroraTwinkle AuroraTwinkle changed the title [improve][pip] PIP-425: fix problem that consumer or producer create failed when many nodes in PulsarClient serviceUrl become unavailable [improve][pip] PIP-425: Support connecting with next available endpoint for multi-endpoint serviceUrls Jun 9, 2025
@AuroraTwinkle
Copy link
Contributor Author

AuroraTwinkle commented Jun 9, 2025

Some initial feedback

Updated!

lhotari
lhotari reviewed Jun 9, 2025
View reviewed changes
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

good progress! added some follow up comments

@AuroraTwinkle AuroraTwinkle force-pushed the PIP-425 branch 2 times, most recently from 7f314b9 to 817a231 Compare June 9, 2025 14:53
@AuroraTwinkle
Copy link
Contributor Author

AuroraTwinkle commented Jun 9, 2025

good progress! added some follow up comments

@lhotari Modified and updated as suggested. Thanks!

lhotari
lhotari reviewed Jun 12, 2025
View reviewed changes
codelipenghui
codelipenghui reviewed Jun 16, 2025
View reviewed changes
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Hi @AuroraTwinkle

The proposal looks good to me.
I’ve added a few comments to help clarify the problem and the proposed solution, making it easier to understand.

@codelipenghui codelipenghui added this to the 4.1.0 milestone Jun 16, 2025
@AuroraTwinkle
Copy link
Contributor Author

AuroraTwinkle commented Jun 17, 2025

Hi @AuroraTwinkle

The proposal looks good to me. I’ve added a few comments to help clarify the problem and the proposed solution, making it easier to understand.

Ok, I will fix them later, Thanks!

AuroraTwinkle and others added 3 commits June 17, 2025 11:08
@AuroraTwinkle @codelipenghui
Update pip/pip-425.md
7dc259a 
Co-authored-by: Penghui Li <penghui@apache.org>
@AuroraTwinkle @codelipenghui
Update pip/pip-425.md
fa4e64e 
Co-authored-by: Penghui Li <penghui@apache.org>
@AuroraTwinkle @codelipenghui
Update pip/pip-425.md
ece2b2a 
Co-authored-by: Penghui Li <penghui@apache.org>
AuroraTwinkle and others added 5 commits June 17, 2025 11:10
@AuroraTwinkle @codelipenghui
Update pip/pip-425.md
ec024ac 
Co-authored-by: Penghui Li <penghui@apache.org>
@AuroraTwinkle @codelipenghui
Update pip/pip-425.md
6cbe13e 
Co-authored-by: Penghui Li <penghui@apache.org>
@AuroraTwinkle @codelipenghui
Update pip/pip-425.md
f24a5f1 
Co-authored-by: Penghui Li <penghui@apache.org>
fix review comments
b4cad0c
fix review comments
0e376b7
@AuroraTwinkle
Copy link
Contributor Author

AuroraTwinkle commented Jun 17, 2025

Hi @AuroraTwinkle

The proposal looks good to me. I’ve added a few comments to help clarify the problem and the proposed solution, making it easier to understand.

@codelipenghui Very interesting and detailed suggestions, I have fixed them, thank you again!

315157973
315157973 reviewed Jun 21, 2025
View reviewed changes
315157973
315157973 reviewed Jun 21, 2025
View reviewed changes
lhotari
lhotari approved these changes Jun 24, 2025
View reviewed changes
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM, good work @AuroraTwinkle

@AuroraTwinkle AuroraTwinkle requested a review from 315157973 June 24, 2025 09:26
@liangyepianzhou
Copy link
Contributor

liangyepianzhou commented Jun 25, 2025 •
edited
Loading

@lhotari @codelipenghui @315157973 We need more votes on the mailing list to close this PIP. Could you please help vote when you have a moment?
https://lists.apache.org/thread/c2zvjwf7bqp8nc2rpzbxd4kdtztk23xp

@liangyepianzhou liangyepianzhou merged commit be385c4 into apache:master Jun 25, 2025
20 checks passed
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
@AuroraTwinkle @KannarFr
[improve][pip] PIP-425: Support connecting with next available endpoi…
1a17a85 
…nt for multi-endpoint serviceUrls (apache#24394)

Fixes apache#22934 (comment)

Main Issue: apache#22934 (comment)

Implementation: apache#24387

### Motivation
As apache#22934 and apache#22933 mentioned, when most of the nodes in serviceurl are down (but there is at least one available node), creating consumers and producers through PulsarClient will most likely fail. I think this is not as expected. If the code is robust enough, as long as there is one available node, it should be accessible normally. Therefore, this pip is going to optimize the logic, remove unavailable nodes through the feedback mechanism, and improve the success rate of PulsarClient requests.

By the way, apache#22935 removes faulty nodes through a regular health check mechanism, but this brings new problems (frequent creation of connections and increased system load), so this solution is abandoned. See apache#22934 (comment) for more details!
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
@AuroraTwinkle @walkinggo
[improve][pip] PIP-425: Support connecting with next available endpoi…
db4b3ad 
…nt for multi-endpoint serviceUrls (apache#24394)

Fixes apache#22934 (comment)

Main Issue: apache#22934 (comment)

Implementation: apache#24387

### Motivation
As apache#22934 and apache#22933 mentioned, when most of the nodes in serviceurl are down (but there is at least one available node), creating consumers and producers through PulsarClient will most likely fail. I think this is not as expected. If the code is robust enough, as long as there is one available node, it should be accessible normally. Therefore, this pip is going to optimize the logic, remove unavailable nodes through the feedback mechanism, and improve the success rate of PulsarClient requests.

By the way, apache#22935 removes faulty nodes through a regular health check mechanism, but this brings new problems (frequent creation of connections and increased system load), so this solution is abandoned. See apache#22934 (comment) for more details!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhotari lhotari lhotari approved these changes

@Technoboy- Technoboy- Technoboy- approved these changes

@315157973 315157973 315157973 approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@liangyepianzhou liangyepianzhou liangyepianzhou approved these changes

Assignees

@AuroraTwinkle AuroraTwinkle

Labels

doc-not-needed Your PR changes do not impact docs PIP

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

[BUG] consumer or producer will create failed frequently when build PulsarClient with many unavailable broker nodes

6 participants

@AuroraTwinkle @liangyepianzhou @lhotari @Technoboy- @315157973 @codelipenghui