Some optimizations for uploading to minio

[lunny]

A fixed size to minio upload sdk may spend less memory than give it a -1. So let's give the size when possible.

close #15253

[jochenhz]

I can confirm that this PR fixes #15253
Thanks a lot! This was really important :)

[zeripath]

I see a number of architectural issues with this approach.

I can understand why an argument polymorphism approach has been chosen instead of an explicit argument approach here and whilst I'm genuinely not sure it's the right thing - I think it's probably OK.

But I absolutely do not think that the ReadSizer interface should be outside of modules/storage - (whether the WithSize constructor function should be is another question.)

The ReadSizer interface should be within modules/storage and the documentation for the storage.Save function should refer to it explaining that if an io.Reader is passed that matches the interface storage.ReadSizer then the Size reported from that will be used.

Thus it's documented in one place and self contained. See https://github.com/golang/go/wiki/CodeReviewComments#interfaces

In terms of the WithSize constructor function - its name is not ideal - ReaderWithSize(...) would be better unless that function is going to end up in a reader specific module, e.g. : modules/ioutil/reader. (In which case the the git.LimitedReadCloser from modules/git/util.go should get moved there with a constructor WithLimitCloser(...)

But further everything that is using a ReadSizer has a direct dependency on storage anyway so you're just adding another module for no great reason. If we genuinely want to use this in places where there isn't a direct dependency on storage later on then we can move the ReaderWithSize(...) function into another module and lightly bind its interface dependence on storage through a test within modules/storage that uses it.

[zeripath]

Actually this argument polymorphism is really going to be confusing:

Consider what happens if you pass a bufio.Reader into Save... to a cursory glance it looks like it implements ReadSizer but it doesn't - thankfully as its Size function means a very different thing.

We really should be implementing and using the io/fs interfaces but in the meantime just add a size argument to Save

[lunny]

Actually this argument polymorphism is really going to be confusing:

Consider what happens if you pass a bufio.Reader into Save... to a cursory glance it looks like it implements ReadSizer but it doesn't - thankfully as its Size function means a very different thing.

We really should be implementing and using the io/fs interfaces but in the meantime just add a size argument to Save

Please send a PR to replace this one.