Reuse persistent HTTP connections

[miq-bot]

I noticed recently we don't reuse HTTP connections. A new TCP/TLS/HTTP connection every request may add up to an order of magnitude speed difference:
https://bibwild.wordpress.com/2012/04/30/ruby-http-performance-shootout-redux/

• HawkularClient creates a new RestClient instance for every single call.

• Kubeclient does cache RestClient instance, but RestClient itself doesn't support connection reuse!
  Connection reuse rest-client/rest-client#453

• [?] Net::HTTP when used directly does support persistent connections in limited scenarios (passing a block to .start). I believe we use it through MiqFs for image scanning, not sure how exactly.

• [?] @yaacov also told me one of the client libs (maybe in python though?) does API discovery anew before every single action.

P.S. new connections are probably more expensive when going through proxies, e.g. to hawkular running in a pod?

After some reading, for fresh developement, httpclient or excon could be better than restclient, on several aspects.
For our existing code, https://github.com/drbrain/net-http-persistent (co-authored by tenderlove :-) sounds a good start. I propose trying quick&dirty patch to use it in RestClient (and MiqFs), measuring how much we stand to gain, and working with RestClient to add at least opt-in option.

• Further ahead, given connection reuse, could gain even more from HTTP/2, should keep it in mind when choosing gems.

@miq-bot add-label providers/containers, providers/hawkular, performance
/cc @lucasponce

Probably happens with other providers, a quick search through Gemfile.lock shows only excon used by fog & OpenstackHandle, httpclient used by WinRm.

---

This issue was moved to this repository from ManageIQ/manageiq#13345, originally opened by @cben

[cben]

Summary from original issue + updates:

• refresh is not a problem now — we do <20 requests total.

  • would become significant if we do targetted queries (but not problem if we just use watch, keeps open connection).

• hawkular-client creates new RestClient instance each .get
  https://github.com/hawkular/hawkular-client-ruby/blob/v4.1.0/lib/hawkular/base_client.rb#L43-L44

• in our usage, we hit /hawkular/metrics/status before every single query => 2X waste:
  vcr: https://github.com/ManageIQ/manageiq-providers-kubernetes/blob/master/spec/vcr_cassettes/manageiq/providers/kubernetes/container_manager/metrics_capture/hawkular_capture_context_pod_metrics.yml
  Seems HawkularClient does it on construction, our fault for not reusing HawkularClient instance:
  https://github.com/hawkular/hawkular-client-ruby/blob/v4.1.0/lib/hawkular/metrics/metrics_client.rb#L62

• prometheus-api-client currently uses Faraday
  https://github.com/prometheus/prometheus_api_client_ruby/blob/master/lib/prometheus/api_client/client.rb
  I think does not reuse by default (?), but Faraday can be configured to use net-http-persistent backend.

• prometheus-api-client does no discovery, only actual queries
  vcr: https://github.com/ManageIQ/manageiq-providers-kubernetes/blob/master/spec/vcr_cassettes/manageiq/providers/kubernetes/container_manager/metrics_capture/prometheus_capture_context_pod_metrics.yml

• alerts: no idea

@yaacov told me it'd be possible to modify our metrics capture to do much fewer batch queries, both with hawkular and prometheus, potentially making this non-issue.

[miq-bot]

@miq-bot Cannot apply the following labels because they are not recognized: providers/containers, providers/hawkular

[miq-bot]

@miq-bot unrecognized command 'Cannot', ignoring...

Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

[yaacov]

@yaacov told me it'd be possible to modify our metrics capture to do much fewer batch queries, both with hawkular and prometheus, potentially making this non-issue.

@cben , yes it's a POC planed by @agrare , It's on my todo list, didn't get to it yet :-(

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

[gtanzillo]

@cben Would you have an idea of the sizing of this issue? S / M / L / XL.

[miq-bot]

@miq-bot Cannot apply the following labels because they are not recognized:

• providers/containers
• providers/hawkular

All labels for ManageIQ/manageiq-providers-kubernetes: https://github.com/ManageIQ/manageiq-providers-kubernetes/labels

[miq-bot]

@miq-bot unrecognized command 'Cannot', ignoring...

Accepted commands are: add_label, add_reviewer, request_review, assign, close_issue, cross_repo_test, move_issue, remove_label, rm_label, remove_reviewer, set_milestone, unassign

[cben]

Kubeclient 4 used rest-client, which is no longer maintained, and doesn't support persistent connections: rest-client/rest-client#453

Kubeclient 5.0 will support this, at least partially.

• Replace rest_client with faraday kubeclient#466 (merged to master) replaced rest-client (used for all requests except watch) with Faraday.
  This will allow client.faraday_client.adapter(:net_http_persistent) to reuse connections within a single Client object (manageiq needs several Client objects — one per api group).

• Replace http gem with faraday kubeclient#488 will replace http gem (used for watch) with Faraday. This got stuck on how to implement .finish, need to decide how to proceed. However as a watch remains open it ties up 1 connection anyway, so this doesn't matter for connection reuse — it's just an open problem that has delayed progress towards release...

• some more goals I hoped for 5.0 release, maybe not all blockers: 5.0 incompatible changes plan kubeclient#435

[miq-bot]

@miq-bot unrecognized command 'unrecognized', ignoring...

Accepted commands are: add_label, add_reviewer, request_review, assign, close_issue, cross_repo_test, move_issue, remove_label, rm_label, remove_reviewer, set_milestone, unassign