Feature #14087

[controller] Fetch remote-hosted collections by PDH

Added by Peter Amstutz 9 months ago. Updated 6 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
09/10/2018
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
2.0
Release:
Release relationship:
Auto

Description

Fulfill a GET request for a collection by PDH by querying the federation.

  1. Match GET request for collection by PDH
  2. Send request to the local cluster
  3. If success, return result
  4. If 404, send parallel requests to list of federated sites defined in discovery document.
  5. 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)
  6. Return first result
  7. 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).

Subtasks

Task #14174: Review 14087-federated-collection-by-pdhResolvedPeter Amstutz


Related issues

Related to Arvados - Feature #13993: [API] Fetch remote-hosted collection by UUIDResolved09/07/2018

Associated revisions

Revision e0ecd284
Added by Peter Amstutz 8 months ago

Merge branch '14087-federated-collection-by-pdh' refs #14087

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#4 Updated by Peter Amstutz 9 months ago

  • Related to Feature #13993: [API] Fetch remote-hosted collection by UUID added

#5 Updated by Peter Amstutz 9 months ago

  • Description updated (diff)

#6 Updated by Peter Amstutz 9 months ago

  • Description updated (diff)
  • Target version changed from To Be Groomed to Arvados Future Sprints
  • Story points set to 2.0

#7 Updated by Peter Amstutz 9 months ago

  • Assigned To set to Peter Amstutz
  • Target version changed from Arvados Future Sprints to 2018-09-19 Sprint

#8 Updated by Peter Amstutz 9 months ago

14087-federated-collection-by-pdh @ f7fe4c8c4ba93eb1746c5392a820af78c2ed2562

#10 Updated by Peter Amstutz 9 months ago

Added rate limiting for parallel requests 61bad45c80df09b92cad96d4541421009183c142

#11 Updated by Peter Amstutz 9 months ago

#12 Updated by Lucas Di Pentima 9 months 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?

#13 Updated by Peter Amstutz 8 months ago

"Content-Encoding" needs to be added to the blacklist of headers not to be propagated by the proxy.

#14 Updated by Peter Amstutz 8 months 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

https://ci.curoverse.com/job/developer-run-tests/894/

#16 Updated by Peter Amstutz 8 months 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"

#17 Updated by Peter Amstutz 8 months ago

  • Status changed from New to Resolved

#18 Updated by Tom Morris 6 months ago

  • Release set to 14

Also available in: Atom PDF