Skip to content

Conversation

philippthun
Copy link
Member

@philippthun philippthun commented Feb 1, 2023

This replaces HTTPClient with Net::HTTP::Persistent in the Diego::Client class.

Net::HTTP::Persistent is based on Net::HTTP which automatically triggers a failover to another IP address after reaching the given connect timeout.

Furthermore Net::HTTP::Persistent adds persistent HTTP connections and thread safety. It is configured to use a pool of up to 50 connections. This equals the size of WorkPools used by various background processes (e.g. deployment updater).

Background information

ℹ️ Use case: Mitigates the risk of Runner is unavailable errors during a cf push when the client is unable to reach a bbs instance.

❌ First, discarded solution proposal: #3002

✔️ Second, merged solution proposal: #3048

💥 Issues caused by #3048

🔄 Revert PR: #3113

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

This replaces HTTPClient with Net::HTTP::Persistent in the Diego::Client
class.

Net::HTTP::Persistent is based on Net::HTTP which automatically triggers
a failover to another IP address after reaching the given connect
timeout.

Furthermore Net::HTTP::Persistent adds persistent HTTP connections and
thread safety. It is configured to use a pool of up to 50 connections.
This equals the size of WorkPools used by various background processes
(e.g. deployment updater).

Co-authored-by: Johannes Haass <[email protected]>
Co-authored-by: Will Gant <[email protected]>
@philippthun
Copy link
Member Author

@johha and me prepared a local test environment to simulate an unresponsive bbs server and ran tests with different HTTP clients:

HTTP client 10 sequential pings 50 parallel threads Notes
httpclient ❌ fails n/a current implementation
httpclient + monkey patch ✅ succeeds ✅ succeeds set connect_timeout on TCPSocket
net_http ✅ succeeds ❌ fails
net_http_persistent ✅ succeeds ✅ succeeds pool_size: 50
http.rb ❌ fails n/a

@philippthun philippthun marked this pull request as ready for review February 1, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants