Unset DisableKeepalive for backfill HTTP client#2137
Unset DisableKeepalive for backfill HTTP client#2137Hayden-IO merged 2 commits intosigstore:mainfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2137 +/- ##
==========================================
+ Coverage 66.46% 68.61% +2.15%
==========================================
Files 92 92
Lines 9258 7293 -1965
==========================================
- Hits 6153 5004 -1149
+ Misses 2359 1545 -814
+ Partials 746 744 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Disabling Keep-Alive, as done by the default transport setting in the hashicorp cleanhttp package, seems to conflict with a network setting between the public good Rekor instances and the bastion and results in GET requests stalling or timing out after processing a few entries. This change adds an option to the rekor client to unset the DisableKeepalive setting and has the backfill script utilize that option. Other uses of the rekor client will see no change. Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
Head branch was pushed to by a user without write access
|
@haydentherapper I added another change, this was still hitting a rate limit after 1000 or so requests. |
Hayden-IO
left a comment
There was a problem hiding this comment.
SG, we might want to bypass the LB for the backfill (note the playbook is out of date, there is no longer a global rate limit, so you aren't affecting other clients)
| RetryCount uint | ||
| InsecureTLS bool | ||
| Logger interface{} | ||
| NoDisableKeepalives bool |
There was a problem hiding this comment.
Should this be EnableKeepalives?
There was a problem hiding this comment.
The field we're un-setting is DisableKeepalives. EnableKeepalives isn't really correct because it's not enabling Keepalives. The double negative seems more intuitive to me.
Increase the RetryCount setting from the default of 3 up to 10 in order to avoid giving up too quickly when the script hits the server rate limit. The retryablehttp client does an exponential backoff, so increasing the number of tries also increases the amount of time it will wait in between each try before it eventually succeeds. Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
Disabling Keep-Alive, as done by the default transport setting in the hashicorp cleanhttp package, seems to conflict with a network setting between the public good Rekor instances and the bastion and results in GET requests stalling or timing out after processing a few entries. This change adds an option to the rekor client to unset the DisableKeepalive setting and has the backfill script utilize that option. Other uses of the rekor client will see no change.
Summary
Release Note
Documentation