Optimize retrieval to do minimal fetcher from singularity

[gammazero]

Read HTTP range headers and do fetch for entire range instead of allowing the io.CopyN, used by http.ServeContent, to do multiple small fetches.

Fixes #143

[codecov-commenter]

Codecov Report

Merging #195 (419afb6) into main (0ef904d) will increase coverage by 6.71%.
The diff coverage is 51.89%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #195      +/-   ##
==========================================
+ Coverage   31.80%   38.51%   +6.71%     
==========================================
  Files           5        6       +1     
  Lines         761      914     +153     
==========================================
+ Hits          242      352     +110     
- Misses        482      516      +34     
- Partials       37       46       +9     

Files	Coverage Δ
blob/blob.go	37.50% <ø> (ø)
integration/singularity/range_reader.go	78.37% <78.37%> (ø)
integration/singularity/reader.go	61.78% <63.46%> (+61.78%)	⬆️
integration/singularity/store.go	35.36% <2.27%> (-1.56%)	⬇️

[xinaxu]

I see the intent of reducing number of requests to Singularity API, and I understand it is not simple to implement io.ReadSeekCloser by making requests to an HTTP API.
If we don't have the Store.Get interface, we can simply open another HTTP request to Singularity API with the same range header and let the Singularity API handle the request. It seems that Store.Get is making this more difficult than I thought.
I am concerned that having a custom implementation of ServeContent may be hard to maintain and test in long run, and outweights the benefits of complying to Store.Get.
Hence, I am proposing to have Singularity_store implement a pass_through interface and bypass the store.Get interface

PassThrough interface {
  Pass(context.Context, http.ResponseWriter, http.Request, ID) error
}

handleBlobGetByID(w, r, id) {
  if pass, ok := m.store.(PassThrough); ok {
    return pass.Pass(ctx, w, r, id)
  } else {
    blobReader, err := m.store.Get(r.Context(), id)
  }
}

what do you think?

[xinaxu]

see above

[gammazero]

@xinaxu I think it makes sense. So the PassThrough will translate the blob ID into a singularity file ID, and then call the singularity API. The generated Singularity client code is called like this:

r.client.File.RetrieveFile(&file.RetrieveFileParams{
    Context: ctx,
    ID:      fileID,
    Range:   ptr.String(byteRange),
}, w)

So it seems like the generated client will not be usable for PassThrough. Should a new client function be created, or should motion construct a custom HTTP request?

[xinaxu]

So it seems like the generated client will not be usable for PassThrough.

Could you explain why it is not usable?

[gammazero]

Could you explain why it is not usable?

Actually, I suppose it will now that I see that the Range parameter is the whole range header, not just a single range as I had previously though. I am coding/testing pass-through now.

[xinaxu]

Question: do you think it's better to just return

new Error("not implemented, please use PassGet")

so we don't end up maintaining code path that never gets hit

[gammazero]

I left Get because I did not know if there would ever be a case where we want to retrieve a blob from singularity, but we are not doing this in response to an HTTP request. For example, suppose we want to retrieve a file from singularity, and then do something to that file (filter it, compress it, etc.), and then return the result. In that case, we will need the original Get. WDYT?

[gammazero]

We can always remove it later in another PR if we determine there is no use.