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

Fix creating producer or consumer is not retried for connection failure #396

Merged

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Feb 4, 2024

Fixes #391

Motivation

When connectionFailed is called, no matter if the result is retryable the creation of producer or consumer will fail without retry.

Modifications

Check if the result is retryable in connectionFailed for ProducerImpl and ConsumerImpl and only fail for non-retryable errors or the timeout error. Register another timer in HandlerBase to propagate the timeout error to connectionFailed.

Add testRetryUntilSucceed, testRetryTimeout, testNoRetry to verify client could retry according to the result returned by ClientImpl::getConnection.

@BewareMyPower BewareMyPower self-assigned this Feb 4, 2024
@BewareMyPower BewareMyPower added the bug Something isn't working label Feb 4, 2024
@BewareMyPower BewareMyPower added this to the 3.5.0 milestone Feb 4, 2024
@BewareMyPower BewareMyPower marked this pull request as draft February 4, 2024 11:24
@BewareMyPower BewareMyPower force-pushed the bewaremypower/retry-create-handler branch from 56c4864 to c4d1473 Compare February 4, 2024 12:26
@BewareMyPower BewareMyPower force-pushed the bewaremypower/retry-create-handler branch from c59a2f3 to 9b57972 Compare February 5, 2024 12:00
@BewareMyPower BewareMyPower marked this pull request as ready for review February 5, 2024 12:00
@BewareMyPower BewareMyPower force-pushed the bewaremypower/retry-create-handler branch from 9b57972 to 60f262b Compare February 5, 2024 12:07
@BewareMyPower BewareMyPower marked this pull request as draft February 5, 2024 13:06
@BewareMyPower BewareMyPower marked this pull request as ready for review February 5, 2024 14:12
@BewareMyPower BewareMyPower marked this pull request as draft February 6, 2024 02:11
@BewareMyPower BewareMyPower force-pushed the bewaremypower/retry-create-handler branch from 544f43c to 8528a23 Compare February 6, 2024 13:09
Fixes apache#391

### Motivation

When `connectionFailed` is called, no matter if the result is retryable
the creation of producer or consumer will fail without retry.

### Modifications

Check if the result is retryable in `connectionFailed` for
`ProducerImpl` and `ConsumerImpl` and only fail for non-retryable errors
or the timeout error. Register another timer in `HandlerBase` to
propagate the timeout error to `connectionFailed`.

Add `testRetryUntilSucceed`, `testRetryTimeout`, `testNoRetry` to verify
client could retry according to the result returned by
`ClientImpl::getConnection`.
@BewareMyPower BewareMyPower force-pushed the bewaremypower/retry-create-handler branch from 1b7a596 to 198b998 Compare February 6, 2024 14:16
@BewareMyPower BewareMyPower marked this pull request as ready for review February 6, 2024 14:16
@BewareMyPower BewareMyPower merged commit 68b4244 into apache:main Feb 7, 2024
12 checks passed
@BewareMyPower BewareMyPower deleted the bewaremypower/retry-create-handler branch February 7, 2024 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Creating producer or consumer doesn't retry for temporarily errors
2 participants