Add connection timeout client configuration option #2852
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allows the client to specify how long to wait for brokers to respond.
ivankelly
added
type/enhancement
Contributor
Author
merlimat
approved these changes
Oct 26, 2018
Contributor
|
run integration tests |
1 similar comment
Contributor
|
run integration tests |
Contributor
Author
|
rerun integration tests |
sijie
reviewed
Oct 29, 2018
pulsar-client/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java
Show resolved
Hide resolved
| .connectionTimeout(24, TimeUnit.HOURS).build()) { | ||
| CompletableFuture<Producer<byte[]>> f = client.newProducer().topic("foo").createAsync(); | ||
|
|
||
| Thread.sleep(12000); // sleep for 12 seconds (default timeout is 10) |
Member
There was a problem hiding this comment.
Can we reduce the default timeout and change the sleep here?
Contributor
Author
There was a problem hiding this comment.
Changing the default timeout is a behavioral change. Currently, it's 10 seconds hardcoded. It does suck that it added 12 seconds to all test runs. A better option would be to disable or just remove this test.
Member
There was a problem hiding this comment.
can we make the timeout configurable and reduce it in the test?
Contributor
Author
There was a problem hiding this comment.
this whole patch is making the timeout configurable. this test is to check that times configured more than the default take effect. I think the easiest would be to just remove this test. Testing that is takes effect is already tested by the other test.
pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java
Show resolved
Hide resolved
sijie
approved these changes
Oct 30, 2018
Contributor
|
run integration tests |
Contributor
Author
|
rerun integration tests |
Contributor
Author
|
rerun java8 tests |
1 task
sijie
pushed a commit
that referenced
this pull request
Jun 25, 2021
Fixes #10747 ### Motivation This PR is a catchup of #2852 and adds connection timeout configuration to C++ and Python client. ### Modifications - Add a `PeriodicTask` class to execute tasks periodically and the relate unit tests: `PeriodicTastTest`. - Use `PeriodicTask` to register a timer before connecting to broker asynchronously, if the connection was not established when the timer is triggered, close the socket so that `handleTcpConnected` can be triggered immediately with a failure. - Add connection timeout (in milliseconds) to both C++ and Python clients. - Add `ClientTest.testConnectTimeout` (C++) and `test_connect_timeout` (Python) and to verify the connection timeout works. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change added tests and can be verified as follows: - PeriodicTaskTest - ClientTest.testConnectTimeout - test_connect_timeout
codelipenghui
pushed a commit
that referenced
this pull request
Aug 5, 2021
Fixes #10747 ### Motivation This PR is a catchup of #2852 and adds connection timeout configuration to C++ and Python client. ### Modifications - Add a `PeriodicTask` class to execute tasks periodically and the relate unit tests: `PeriodicTastTest`. - Use `PeriodicTask` to register a timer before connecting to broker asynchronously, if the connection was not established when the timer is triggered, close the socket so that `handleTcpConnected` can be triggered immediately with a failure. - Add connection timeout (in milliseconds) to both C++ and Python clients. - Add `ClientTest.testConnectTimeout` (C++) and `test_connect_timeout` (Python) and to verify the connection timeout works. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change added tests and can be verified as follows: - PeriodicTaskTest - ClientTest.testConnectTimeout - test_connect_timeout (cherry picked from commit 6062d2f)
bharanic-dev
pushed a commit
to bharanic-dev/pulsar
that referenced
this pull request
Mar 18, 2022
Fixes apache#10747 ### Motivation This PR is a catchup of apache#2852 and adds connection timeout configuration to C++ and Python client. ### Modifications - Add a `PeriodicTask` class to execute tasks periodically and the relate unit tests: `PeriodicTastTest`. - Use `PeriodicTask` to register a timer before connecting to broker asynchronously, if the connection was not established when the timer is triggered, close the socket so that `handleTcpConnected` can be triggered immediately with a failure. - Add connection timeout (in milliseconds) to both C++ and Python clients. - Add `ClientTest.testConnectTimeout` (C++) and `test_connect_timeout` (Python) and to verify the connection timeout works. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change added tests and can be verified as follows: - PeriodicTaskTest - ClientTest.testConnectTimeout - test_connect_timeout
merlimat
pushed a commit
to apache/pulsar-client-python
that referenced
this pull request
Sep 29, 2022
Fixes #10747 ### Motivation This PR is a catchup of apache/pulsar#2852 and adds connection timeout configuration to C++ and Python client. ### Modifications - Add a `PeriodicTask` class to execute tasks periodically and the relate unit tests: `PeriodicTastTest`. - Use `PeriodicTask` to register a timer before connecting to broker asynchronously, if the connection was not established when the timer is triggered, close the socket so that `handleTcpConnected` can be triggered immediately with a failure. - Add connection timeout (in milliseconds) to both C++ and Python clients. - Add `ClientTest.testConnectTimeout` (C++) and `test_connect_timeout` (Python) and to verify the connection timeout works. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change added tests and can be verified as follows: - PeriodicTaskTest - ClientTest.testConnectTimeout - test_connect_timeout
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allows the client to specify how long to wait for brokers to respond.