Make file upload be robust to file size changes

[yarikoptic]

Per request of @jwodder in #1257 (comment) filing it as a separate issue.

Background: As discovered in that issue we caught NFS (or some layer above on the way to Python) lying about file size to be 0 whenever it is not: the reported size gets changed within few seconds. We also spotted S3 failing some uploads due to "Content-MD5 you specified did not match what we received", an exact reason for which we do not know.
So, apparently we cannot really rely on file system to provide us robust file size, and thus we should 1. provide mitigation to the 0-size file issue and to possibly help isolate any other incorrect file size reporting add a safe-guard.

I am filing it as a single issue since IMHO two aspects are very related and likely to be implemented in a concerted effort. I will provide only high level description of desired behavior instead of mandating any particular implementation.

• Should be in effect for any file upload, be it a blob or a file within zarr folder upload
• Upon initiation of upload do a filesize check: a file size reported to be 0, loop for up to 2 seconds (sleeping may be 100ms between) re-checking the file size. If it changes from 0 - can immediately proceed forward. If it remains 0 - proceed forward with 0 file size after those 2 seconds.
• Upon completion of upload, be it successful or failed with an error, compare filesize with the one obtained in previous filesize check step. If size differs: if upload finished successfully - do raise an (IOError or other you find more appropriate) exception summarizing size changes, if upload errorred out - just log an ERROR level log message summarizing size changes.

[jwodder]

Upon initiation of upload do a filesize check: a file size reported to be 0, loop for up to 2 seconds (sleeping may be 100ms between) re-checking the file size. If it changes from 0 - can immediately proceed forward. If it remains 0 - proceed forward with 0 file size after those 2 seconds.

What exactly is the goal of re-checking zero-sized files? Are you assuming that, once NFS reports a non-zero size, it will report a non-zero size again when requests queries the file size? Or do you want the value from this check to be used as the Content-Length in the upload request? The latter would require always monkeypatching super_len().

[jwodder]

loop for up to 2 seconds (sleeping may be 100ms between)

To be clear: Do you mean that the total time spent looping should be no more than two seconds or that the individual sleeps should increase up to a maximum of two seconds each? Either way, how many iterations through the loop should there be?

[yarikoptic]

What exactly is the goal of re-checking zero-sized files? Are you assuming that, once NFS reports a non-zero size, it will report a non-zero size again when requests queries the file size?

yes, that is the assumption I have that once it is non-zero, it stays non-zero for the nearest future. I can be proven wrong.

Or do you want the value from this check to be used as the Content-Length in the upload request?

Per my assumption above, I was assuming that it would be the same value used in Content-Length.

The latter would require always monkeypatching super_len().

It is ok with me if it would be so (as I have mentioned before), we might just need to add a unittest to catch a moment when requests RFs their API so there is no use of that super_len (by may be making a test where we patch but raise exception there and then use requests to upload - if succeeds without that exception raised -- they changed behavior)

[yarikoptic]

loop for up to 2 seconds (sleeping may be 100ms between)

To be clear: Do you mean that the total time spent looping should be no more than two seconds or that the individual sleeps should increase up to a maximum of two seconds each? Either way, how many iterations through the loop should there be?

individual sleeps of e.g. 100ms (no need to increase delay IMHO), total to 2 sec, so 20 iterations.

[github-actions]

🚀 Issue was released in 0.59.0 🚀