spring-cloud-vault icon indicating copy to clipboard operation
spring-cloud-vault copied to clipboard

Published 20 hours ago • verified publisher icon spring-cloud

Retry mechanism

Open pmv opened this issue 3 years ago • 13 comments

Opening for discussion - I believe vault is critical enough infrastructure that clients should have a retry mechanism out of the box.

Relates to https://github.com/spring-projects/spring-vault/issues/504 and https://github.com/spring-cloud/spring-cloud-vault/issues/236

Due to the similarities (both bootstrap property sources), default retry properties were set to match spring cloud config's defaults (https://cloud.spring.io/spring-cloud-config/multi/multi__spring_cloud_config_client.html#config-client-retry)

I have not added tests - I can/will do so once a maintainer indicates this PR on the right track and has potential to be merged.

Thank you

pmv avatar Mar 03 '21 07:03 pmv

@pmv Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-issuemaster avatar Mar 03 '21 07:03 pivotal-issuemaster

Adding retry support here seems fine. However, we should not make use of aspect-oriented programming features. Instead, using RetryTemplate directly is much more efficient. Actually, the ideal variant would be a Retry-enabled ClientHttpRequestFactory.

mp911de avatar Mar 03 '21 07:03 mp911de

Adding retry support here seems fine. However, we should not make use of aspect-oriented programming features.

Originally I did not until I saw this: https://github.com/spring-cloud/spring-cloud-config/blob/master/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServiceBootstrapConfiguration.java#L62-L78, and then I modeled my changes after those. However, I am not set on that particular implementation.

Actually, the ideal variant would be a Retry-enabled ClientHttpRequestFactory.

Pushing an update to that effect.

pmv avatar Mar 08 '21 05:03 pmv

@pmv Thank you for signing the Contributor License Agreement!

pivotal-issuemaster avatar Mar 10 '21 04:03 pivotal-issuemaster

Let me know if there's any more legwork you'd like me to do for this one - thanks

pmv avatar Mar 31 '21 22:03 pmv

Let me know if you have any more feedback - I believe suggestions up to this point have been addressed.

pmv avatar Jun 11 '21 12:06 pmv

Thanks. I want to have a look at this in the upcoming days.

mp911de avatar Jun 11 '21 12:06 mp911de

Hi, are there any timelines when retry will be supported? Is there a workaround example how to setup retry for spring-cloud-vault.3.0.3?

stefan-g avatar Sep 10 '21 07:09 stefan-g

Right now, we don't have bandwidth to follow up timely.

mp911de avatar Sep 14 '21 06:09 mp911de

@mp911de - It will be great if this can be reviewed and supported for a subsequent release as it could save us from implementing custom retry solutions within every vault client app. Thanks!

krisiye avatar Apr 15 '22 15:04 krisiye

Iam agree, it will be be nice if this feature is implemented, the usage of vault for database access is very critical.

For the moment, if someone could share a full example of code workaround to handle retry (without AOP and forking the repository is better) it will be nice.

Thank you.

juriohacc avatar Apr 23 '22 12:04 juriohacc

Hi guys:

Are you planing to merge this branch? The future seems to be great!

Thanks

iozho avatar Nov 24 '22 14:11 iozho