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

[client] Limit the number of times lookup requests are redirected #7096

Merged
merged 1 commit into from
May 31, 2020

Conversation

massakam
Copy link
Contributor

Master Issue: #7041

Motivation

When a leader broker is restarted, some producers for topics owned by that broker may not be reopened on the new broker. When this happens, message publishing will continue to fail until the client application is restarted.

As a result of the investigation, I found that lookup requests sent by the producers in question are redirected more than 10,000 times between multiple brokers.

When a lookup request is redirected, BinaryProtoLookupService#findBroker() is called recursively. Therefore, tens of thousands of redirects will cause StackOverflowError and BinaryProtoLookupService#findBroker() will never complete.

Modifications

Limit the number of times a lookup is redirected to 100. This maximum is user configurable. If the number of redirects exceeds 100, the lookup will fail. But ConnectionHandler retries lookup so that the producer can eventually reconnect to the new broker.

@massakam massakam added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/client labels May 29, 2020
@massakam massakam added this to the 2.6.0 milestone May 29, 2020
@massakam massakam self-assigned this May 29, 2020
@codelipenghui codelipenghui linked an issue May 31, 2020 that may be closed by this pull request
@codelipenghui codelipenghui merged commit 04035c5 into apache:master May 31, 2020
@massakam massakam deleted the too-many-redirects branch June 1, 2020 01:20
Huanli-Meng pushed a commit to Huanli-Meng/pulsar that referenced this pull request Jun 1, 2020
Master Issue: apache#7041

### Motivation

When a leader broker is restarted, some producers for topics owned by that broker may not be reopened on the new broker. When this happens, message publishing will continue to fail until the client application is restarted.

As a result of the investigation, I found that lookup requests sent by the producers in question are redirected more than 10,000 times between multiple brokers.

When a lookup request is redirected, `BinaryProtoLookupService#findBroker()` is called recursively. Therefore, tens of thousands of redirects will cause `StackOverflowError` and `BinaryProtoLookupService#findBroker()` will never complete.

### Modifications

Limit the number of times a lookup is redirected to 100. This maximum is user configurable. If the number of redirects exceeds 100, the lookup will fail. But `ConnectionHandler` retries lookup so that the producer can eventually reconnect to the new broker.
Huanli-Meng pushed a commit to Huanli-Meng/pulsar that referenced this pull request Jun 1, 2020
Master Issue: apache#7041

### Motivation

When a leader broker is restarted, some producers for topics owned by that broker may not be reopened on the new broker. When this happens, message publishing will continue to fail until the client application is restarted.

As a result of the investigation, I found that lookup requests sent by the producers in question are redirected more than 10,000 times between multiple brokers.

When a lookup request is redirected, `BinaryProtoLookupService#findBroker()` is called recursively. Therefore, tens of thousands of redirects will cause `StackOverflowError` and `BinaryProtoLookupService#findBroker()` will never complete.

### Modifications

Limit the number of times a lookup is redirected to 100. This maximum is user configurable. If the number of redirects exceeds 100, the lookup will fail. But `ConnectionHandler` retries lookup so that the producer can eventually reconnect to the new broker.
Huanli-Meng pushed a commit to Huanli-Meng/pulsar that referenced this pull request Jun 12, 2020
Master Issue: apache#7041

### Motivation

When a leader broker is restarted, some producers for topics owned by that broker may not be reopened on the new broker. When this happens, message publishing will continue to fail until the client application is restarted.

As a result of the investigation, I found that lookup requests sent by the producers in question are redirected more than 10,000 times between multiple brokers.

When a lookup request is redirected, `BinaryProtoLookupService#findBroker()` is called recursively. Therefore, tens of thousands of redirects will cause `StackOverflowError` and `BinaryProtoLookupService#findBroker()` will never complete.

### Modifications

Limit the number of times a lookup is redirected to 100. This maximum is user configurable. If the number of redirects exceeds 100, the lookup will fail. But `ConnectionHandler` retries lookup so that the producer can eventually reconnect to the new broker.
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
Master Issue: apache#7041

### Motivation

When a leader broker is restarted, some producers for topics owned by that broker may not be reopened on the new broker. When this happens, message publishing will continue to fail until the client application is restarted.

As a result of the investigation, I found that lookup requests sent by the producers in question are redirected more than 10,000 times between multiple brokers.

When a lookup request is redirected, `BinaryProtoLookupService#findBroker()` is called recursively. Therefore, tens of thousands of redirects will cause `StackOverflowError` and `BinaryProtoLookupService#findBroker()` will never complete.

### Modifications

Limit the number of times a lookup is redirected to 100. This maximum is user configurable. If the number of redirects exceeds 100, the lookup will fail. But `ConnectionHandler` retries lookup so that the producer can eventually reconnect to the new broker.
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Nov 29, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Nov 29, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Nov 29, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 2, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 2, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 2, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 7, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 7, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 7, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 12, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 12, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 12, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 16, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 16, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 16, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Dec 16, 2022
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Jan 3, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Jan 3, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Jan 3, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Jan 3, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Jan 4, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Jan 4, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Feb 14, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Feb 14, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Feb 14, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Feb 14, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Feb 14, 2023
tongsucn added a commit to tongsucn/pulsar-client-cpp that referenced this pull request Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client 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.

Producers failed to open when leader broker shut down
3 participants