Feature #14087
closed
[controller] Fetch remote-hosted collections by PDH
100%
Description
Fulfill a GET request for a collection by PDH by querying the federation.
- Match GET request for collection by PDH
- Send request to the local cluster
- If success, return result
- If 404, send parallel requests to list of federated sites defined in discovery document.
- On the first successful result, alter the signature hints in the returned record so keepstore will be able to get the data from the remote cluster (see Federated collections wiki and #13993)
- Return first result
- If all requests fail / time out, return error. Either 404 (if all sites return 404) 502 (Bad Gateway) or 504 (Gateway timeout) depending on the type of failure (return 5xx error only when it makes sense for the client to retry).
Updated by Peter Amstutz over 6 years ago
- Related to Feature #13993: [API] Fetch remote-hosted collection by UUID added
Updated by Peter Amstutz over 6 years ago
- Description updated (diff)
- Target version changed from To Be Groomed to Arvados Future Sprints
- Story points set to 2.0
Updated by Peter Amstutz over 6 years ago
- Assigned To set to Peter Amstutz
- Target version changed from Arvados Future Sprints to 2018-09-19 Sprint
Updated by Peter Amstutz over 6 years ago
14087-federated-collection-by-pdh @ f7fe4c8c4ba93eb1746c5392a820af78c2ed2562
Updated by Peter Amstutz over 6 years ago
Rebased, now a4d77e7f0460e85a4dd8c3730c65fea60a8d212f
Updated by Peter Amstutz over 6 years ago
Added rate limiting for parallel requests 61bad45c80df09b92cad96d4541421009183c142
Updated by Peter Amstutz over 6 years ago
Added rate limiting, do 4 parallel requests at a time
Rebased again @ 3539e6e39515a5e08bb2f10dc71be5841827b294
https://ci.curoverse.com/view/Developer/job/developer-run-tests/884/
Updated by Lucas Di Pentima over 6 years ago
Sorry for the delay, here're my questions/comments:
- federation.go
- Line 205: Comment typo: s/we want/we/
- Lines 234-325: Could this be written in some other way to avoid having such a large conditional block? I found it somewhat difficult to follow.
- Would it be a good idea to test the case with more than 1 remote, some returning 404 and one returning what’s requested?
Updated by Peter Amstutz over 6 years ago
"Content-Encoding" needs to be added to the blacklist of headers not to be propagated by the proxy.
Updated by Peter Amstutz over 6 years ago
Lucas Di Pentima wrote:
Sorry for the delay, here're my questions/comments:
- federation.go
- Line 205: Comment typo: s/we want/we/
Fixed, thanks.
- Lines 234-325: Could this be written in some other way to avoid having such a large conditional block? I found it somewhat difficult to follow.
I refactored the code a bit. The shorter, non-PDH search case now comes first. I also adjusted the logic that decides when to skip federation and made it wrap the federation endpoint routing.
- Would it be a good idea to test the case with more than 1 remote, some returning 404 and one returning what’s requested?
That's already more or less happening, the test harness sets up two remotes, one called "zzzzz" and the other called "zmock" which always returns 200 OK with an empty response. So that tests the failure case where one of the remotes returns a non-parsable response.
now 14087-federated-collection-by-pdh @ cf1f57e51578de2ad9121e300f5b816b74938684
Updated by Lucas Di Pentima over 6 years ago
cf1f57e51578de2ad9121e300f5b816b74938684 LGTM, thanks!
Updated by Peter Amstutz over 6 years ago
Peter Amstutz wrote:
"Content-Encoding" needs to be added to the blacklist of headers not to be propagated by the proxy.
And also "Accept-Encoding"
Updated by Tom Morris