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

[improve][broker] Add error logs and retry if the broker failed to register itself for metadata node deletion #23390

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Oct 2, 2024

Motivation

In my chaos test environment, I found some registerAsync call failed. However, there is no error log for it.

Modifications

Add the error logs for failure of registerAsync and retry in handleMetadataStoreNotification. For BrokerRegistryImpl#start, the error info is wrapped into the PulsarServerException propagated to the PulsarService#start's catch block and it will fail fast so we don't need to log errors here.

Documentation

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

Matching PR in forked repository

PR in forked repository:

@BewareMyPower BewareMyPower added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker labels Oct 2, 2024
@BewareMyPower BewareMyPower added this to the 4.0.0 milestone Oct 2, 2024
@BewareMyPower BewareMyPower self-assigned this Oct 2, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 2, 2024
@BewareMyPower BewareMyPower changed the title [improve][broker] Add error logs if the broker failed to register itself for metadata node deletion [improve][broker] Add error logs and retry if the broker failed to register itself for metadata node deletion Oct 2, 2024
@heesung-sn
Copy link
Contributor

This change conflicts with this one, #23382. I also added a retry logic in this PR, and I think we need to run the listeners after registering the lock. Otherwise, there can be a racing condition between orphan bundle cleanup and the lock recreation.

@BewareMyPower
Copy link
Contributor Author

Let's close this PR for now

@BewareMyPower BewareMyPower deleted the bewaremypower/log-register-failure branch October 3, 2024 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants