You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "AuroraTwinkle (via GitHub)" <gi...@apache.org> on 2024/06/18 06:38:33 UTC
[I] [improvement]Perform health checks on the instance passed in by serviceUrl [pulsar]
AuroraTwinkle opened a new issue, #22934:
URL: https://github.com/apache/pulsar/issues/22934
### Search before asking
- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.
### Motivation
Reference Links: https://github.com/apache/pulsar/discussions/22933
### Solution
In the implementation of the `ServiceNameResolver` interface, we can perform periodic node health check and corresponding removal or recovery operations to avoid selecting unhealthy nodes when resolvingHost!
### Alternatives
_No response_
### Anything else?
_No response_
### Are you willing to submit a PR?
- [X] I'm willing to submit a PR!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] consumer or producer will create failed frequently when build PulsarClient with many unavailable broker nodes [pulsar]
Posted by "AuroraTwinkle (via GitHub)" <gi...@apache.org>.
AuroraTwinkle commented on issue #22934:
URL: https://github.com/apache/pulsar/issues/22934#issuecomment-2952283541
> [@AuroraTwinkle](https://github.com/AuroraTwinkle) I see the referenced discussion [#22933](https://github.com/apache/pulsar/discussions/22933). That's a good problem description. I think it's a bug when the next available serviceUrl isn't attempted. That's the whole point of having multiple endpoints. However the solution in [#22935](https://github.com/apache/pulsar/pull/22935) has it's issues as I've commented on the PR. The general approach in the client should be that the connection is attempted and if it fails, it will pick the next endpoint.
>
> I took a look at the code and it seems that `client.getCnxPool().getConnection(socketAddress)` calls should be using `client.getCnxPool().getConnection(serviceNameResolver)`.
>
> `org.apache.pulsar.client.impl.ConnectionPool` would need a new `getConnection` method that accepts the `org.apache.pulsar.client.impl.ServiceNameResolver` instance. The ServiceNameResolver interface will need some changes. When there's a failure for one address, there should be a method that can be called by ConnectionPool to provide feedback that the connection failed for that address. This way the ServiceNameResolver can remove it from the main pool of addresses for a configurable period of time. If all main pool addresses are offline, the resolver implementation could start getting random addresses from the offline pool. There would also be a need to have a success feedback method so that ConnectionPool could call it each time a connection was successful. Would you like to implement such solution instead of [#22935](https://github.com/apache/pulsar/pull/22935)?
@lhotari Hi, Lari :
I have submitted a new PR to implement this solution. Would you please review it if you have time. Any suggestions are welcome.
PIP-425: https://github.com/apache/pulsar/pull/24394
Implemention: https://github.com/apache/pulsar/pull/24387
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [improve]Perform health checks on the endpoints passed in by serviceUrl [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #22934:
URL: https://github.com/apache/pulsar/issues/22934#issuecomment-2943690398
@AuroraTwinkle I see the referenced discussion https://github.com/apache/pulsar/discussions/22933. That's a good problem description.
I think it's a bug when the next available serviceUrl isn't attempted. That's the whole point of having multiple endpoints. However the solution in #22935 has it's issues as I've commented on the PR. The general approach in the client should be that the connection is attempted and if it fails, it will pick the next endpoint.
I took a look at the code and it seems that
`client.getCnxPool().getConnection(socketAddress)` calls should be using `client.getCnxPool().getConnection(serviceNameResolver)`.
`org.apache.pulsar.client.impl.ConnectionPool` would need a new `getConnection` method that accepts the `org.apache.pulsar.client.impl.ServiceNameResolver` instance. The ServiceNameResolver interface will need some changes. When there's a failure for one address, there should be a method that can be called by ConnectionPool to provide feedback that the connection failed for that address. This way the ServiceNameResolver can remove it from the main pool of addresses for a configurable period of time. If all main pool addresses are offline, the resolver implementation could start getting random addresses from the offline pool. There would also be a need to have a success feedback method so that ConnectionPool could call it each time a connection was successful.
Would you like to implement such solution instead of #22935?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [improve]Perform health checks on the endpoints passed in by serviceUrl [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #22934:
URL: https://github.com/apache/pulsar/issues/22934#issuecomment-2943626635
@AuroraTwinkle This issue doesn't describe any problem, only a solution. Please add more context about what solution you are trying to solve.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [BUG] consumer or producer will create failed frequently when build PulsarClient with many unavailable broker nodes [pulsar]
Posted by "liangyepianzhou (via GitHub)" <gi...@apache.org>.
liangyepianzhou closed issue #22934: [BUG] consumer or producer will create failed frequently when build PulsarClient with many unavailable broker nodes
URL: https://github.com/apache/pulsar/issues/22934
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [improve]Perform health checks on the endpoints passed in by serviceUrl [pulsar]
Posted by "AuroraTwinkle (via GitHub)" <gi...@apache.org>.
AuroraTwinkle commented on issue #22934:
URL: https://github.com/apache/pulsar/issues/22934#issuecomment-2944697725
> [@AuroraTwinkle](https://github.com/AuroraTwinkle) I see the referenced discussion [#22933](https://github.com/apache/pulsar/discussions/22933). That's a good problem description. I think it's a bug when the next available serviceUrl isn't attempted. That's the whole point of having multiple endpoints. However the solution in [#22935](https://github.com/apache/pulsar/pull/22935) has it's issues as I've commented on the PR. The general approach in the client should be that the connection is attempted and if it fails, it will pick the next endpoint.
>
> I took a look at the code and it seems that `client.getCnxPool().getConnection(socketAddress)` calls should be using `client.getCnxPool().getConnection(serviceNameResolver)`.
>
> `org.apache.pulsar.client.impl.ConnectionPool` would need a new `getConnection` method that accepts the `org.apache.pulsar.client.impl.ServiceNameResolver` instance. The ServiceNameResolver interface will need some changes. When there's a failure for one address, there should be a method that can be called by ConnectionPool to provide feedback that the connection failed for that address. This way the ServiceNameResolver can remove it from the main pool of addresses for a configurable period of time. If all main pool addresses are offline, the resolver implementation could start getting random addresses from the offline pool. There would also be a need to have a success feedback method so that ConnectionPool could call it each time a connection was successful. Would you like to implement such solution instead of [#22935](https://github.com/apache/pulsar/pull/22935)?
Good suggestion! Removing abnormal nodes through the feedback mechanism can avoid frequent connection establishment and increase the system load. I will submit a new PR to implement it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [improve]Perform health checks on the endpoints passed in by serviceUrl [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #22934:
URL: https://github.com/apache/pulsar/issues/22934#issuecomment-2943636166
> In the implementation of the `ServiceNameResolver` interface, we can perform periodic node health check and corresponding removal or recovery operations to avoid selecting unhealthy nodes when resolvingHost!
@AuroraTwinkle What problem occurs if an unhealthy node is selected? I would assume that in the current design, the next node would be selected after a connection attempt. Why would there be a need to change this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org