remote s3 caching uses multipart copy which may cause an ETag mismatch

[woodshop]

DVC version: 0.81.3, ubuntu 18.04 LTS, pip install, python 3.7.4

When specifying a remote object on s3 as a cached output, an ETag mismatch error is sometimes raised during the caching stage. E.g., observe the following debug output.

DEBUG: Removing s3://XXXXX/artifacts/test/train/models/model.01.ckpt.data-00000-of-00002
DEBUG: Created 'copy': s3://XXXXX/s3cache/ee/28f5ee86fc9aaca1aca65e64abde58 -> s3://XXXXX/artifacts/test/train/models/model.01.ckpt.data-00000-of-00002
DEBUG: cache 's3://XXXXX/s3cache/82/ed1912264aec805954939a0c84b5ff' expected '82ed1912264aec805954939a0c84b5ff' actual 'None'
DEBUG: SELECT count from state_info WHERE rowid=?
DEBUG: fetched: [(429,)]
DEBUG: UPDATE state_info SET count = ? WHERE rowid = ?
ERROR: failed to run command - ETag mismatch detected when copying file to cache! (expected: '82ed1912264aec805954939a0c84b5ff', actual: '284b1e7130c3689baad589f9de093810-11')

In this example,
s3://XXXXX/artifacts/test/train/models/model.01.ckpt.data-00000-of-00002 is 81.4 MB. When dvc attempts to cache the file, it produces a multipart copy with a mismatched ETAG.

I believe this is because s3.copy

dvc/dvc/remote/s3.py

Line 172 in 482473a

s3.copy(source, to_info.bucket, to_info.path, ExtraArgs=extra_args)

uses a default chunk size and chunk threshold of 8MB.

I've patched the problem locally for myself with the following lines:

import boto3
s3.copy(
    source, to_info.bucket, to_info.path, ExtraArgs=extra_args,
    Config=boto3.s3.transfer.TransferConfig(
        multipart_threshold=1024**3
   )
)

There are probably more elegant methods to avoid a multipart copy. For instance, https://stackoverflow.com/a/38058798 suggests using put_object.

[woodshop]

BTW, I think there's a hidden assumption in dvc's code that any remote s3 output will use the boto3 library and therefore have the same default chunking. This is not the case for me, where my remote objects are uploaded using an external library that, as far as I can tell, doesn't use any chunking.

[efiop]

Hi @woodshop ! Are you using aws s3 as an endpoint? Or some other solution that is simply s3 compatible?

[woodshop]

Hey @efiop I'm using AWS S3 as the endpoint.

[efiop]

@woodshop Dvc is trying to re-upload the same way as it was before, so we have some logic here https://github.com/iterative/dvc/blob/master/dvc/remote/s3.py#L165 , which, based on existing etag computes what is the size and the number of chunks that it needs to reupload it with. I'm not able to pinpoint right away where that might've gone wrong, but just want to ask whether or not you are sure that no one is writing/modifying that file while you are caching it? Also, which command are you running? dvc run -o s3:/// ...?

[woodshop]

Hey @efiop there is only one process writing to the remote output. No one is writing while caching.

The command (somewhat truncated and anonymized) is

dvc run -v \
    -w "${wdir}" \
    -f "stages/runs/${eid}/train.dvc" \
    -d "../../${data_dir}" \
    -d "../../data/file_lists/train_pv_tfrecords_list.txt" \
    -d "../../data/file_lists/validation_pv_tfrecords_list.txt" \
    -o "s3://XXXXX/artifacts/${eid}/train" \
    --overwrite-dvcfile \
    rm-train \
    <snip>

In my particular case there should be no more than one chunk to re-upload, and the original ETag reflects this. In other words, parts_suffix is empty. However the copy method executes as a multipart upload (by default), and therefore the destination object has an ETag with a hyphenated suffix, whereas the source object does not.

(At least this is what I think is happening.)

[efiop]

@woodshop I think you are right. Ok, so in that case, we need to replace copy with _copy_multipart with with a chunk size == to the size of the file that we are uploading, right? Would you be so kind as to try that out and if it works for you, submit a PR with a fix, please? 🙂 Will be happy to help with everything we can, of course.

[woodshop]

@efiop I'd be happy to submit a PR.

You suggest using _copy_multipart. In this specific case, there will be only one part. Do you foresee any issues with this approach?

Another possibility is to use copy and specify an appropriate sized multipart_threshold via the Config argument, as I pasted above.

I've confirmed that the second approach works for me, I haven't yet tested the first. The second approach requires instantiating boto3.s3.transfer.TransferConfig whereas the first approach does not. Any thoughts about which is better?

[efiop]

@woodshop Sorry, I haven't noticed the Config previously. You are aboslutely right, that would be the best way to handle it. Let's go your way and use Config with multipart_threshhold. 👍I don't think there will be any issues with your approach. Thank you so much for contributing!