Explicitly Close() obj after ReadFull()

[benagricola]

Restic makes heavy use of short reads from S3 when performing incremental backups. It runs GetObject from minio-go and reads part of the returned stream (e.g. 45 bytes from a specific offset) and then returns.

This leaves the HTTP connection managed by minio-go containing unread data, and in this situation the minio-go library will create a new connection for subsequent requests even though restic is finished with the previous object. Performing an incremental backup of a 7GiB filesystem I noticed connection counts from Restic to the S3 endpoint of up to 30,000 when the open file limit was raised high enough to allow this.

Explicitly calling Close() after io.ReadFull() causes the remaining data to be discarded and allows minio-go to handle connection reuse effectively. The same incremental backup described above will use on average 15 to 30 connections for the duration of the backup.

[fd0]

Hey, thanks for giving this a try!

[fd0]

This discards the error that Close() may have returned. What do you think about returning the error from Close() in case ReadFull() did not return any error?

[benagricola]

I assume in this situation that the important part is the Read error rather than the close error.

I wonder if it's possible for a Read error to cause obj to be already-closed, in which case this would throw an error attempting to close it twice...

[benagricola]

Also one thing to note, there's been a number of changes to minio-go upstream which help to reduce the concurrent connection count (also to do with connections being recreated instead of reused). Would be good to have updated minio-go pulled in.

[fd0]

Yes, I agree. Thanks for your contribution!