Skip to content
This repository was archived by the owner on Apr 30, 2025. It is now read-only.

fix: do not retry if context has been cancelled #345

Conversation

maxmoehl
Copy link
Member

@maxmoehl maxmoehl commented May 8, 2023

The additional retry logic introduced by #337 caused any error to become retry-able if the request did not make it to the back end. This included errors that came from context cancellation. This commit makes requests non-retry-able as soon as the context has been cancelled.

See related Slack discussion: https://cloudfoundry.slack.com/archives/C01ABMVNE9E/p1683313910835329

  • I have viewed signed and have submitted the Contributor License Agreement
  • I have made this pull request to the main branch
  • I have run all the unit tests using scripts/run-unit-tests-in-docker from routing-release.
  • (Optional) I have run Routing Acceptance Tests and Routing Smoke Tests on bosh lite
  • (Optional) I have run CF Acceptance Tests on bosh lite

@maxmoehl maxmoehl force-pushed the dont-retry-on-context-cancel branch from 9983c22 to 5308c3f Compare May 8, 2023 09:20
@maxmoehl
Copy link
Member Author

maxmoehl commented May 8, 2023

This fix should address the issues reported in the linked Slack thread. Nevertheless we should make the classifier lists distinct and re-consider under which circumstances an endpoint should be removed from the pool, but this is out of scope for this PR.

@maxmoehl
Copy link
Member Author

maxmoehl commented May 8, 2023

Running the full unit test suite in docker fails because the image is gone:

Error response from daemon: pull access denied for cloudfoundry/cf-routing-pipeline, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

@maxmoehl maxmoehl marked this pull request as ready for review May 8, 2023 09:33
@maxmoehl maxmoehl force-pushed the dont-retry-on-context-cancel branch from 5308c3f to 9025068 Compare May 8, 2023 11:39
The additional retry logic introduced by cloudfoundry#337 caused any error to become
retry-able if the request did not make it to the back end. This included
errors that came from context cancellation. This commit makes requests
non-retry-able as soon as the context has been cancelled.

Co-Authored-By: Dominik Froehlich <[email protected]>
@maxmoehl maxmoehl force-pushed the dont-retry-on-context-cancel branch from 9025068 to a3bd9ab Compare May 8, 2023 13:39
@geofffranks geofffranks merged commit 76f5865 into cloudfoundry:main May 8, 2023
@maxmoehl maxmoehl deleted the dont-retry-on-context-cancel branch May 8, 2023 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants