s3: fsspec migration

[isidentical]

Supersedes the first attempt on #5622.

[isidentical]

I'm marking this PR ready to review. The test suite passes on every version, though since we are awaiting a new s3fs release we can't just merge it as is yet (only one line change in setup to point to pypi s3fs instead of github one solve this though, so it won't block reviews).

[isidentical]

Benchmarks from the latest revision, same in general but there seems to be a slight improvement on dvc status -c which might be related with dircache on fsspec: https://gist.github.com/isidentical/9e0246c4fe700c019a49cd6ec6de2440

[skshetry]

@isidentical, now that we are using docker 😩, please turn the following config off so that s3 tests are not run by default:

dvc/tests/conftest.py

Line 22 in 3143975

"s3": True,

[efiop]

🔥

And s3fs is out too https://pypi.org/project/s3fs/ (as well as gcsfs), just waiting on adlfs 🙂

[shcheklein]

hey, guys - quick ask - could we summarize (and may be visualize) before after? It's hard for me to figure out what do specific dvc status -c commands mean in the gist, what do we tests, what we don't test ...

(not a blocker for merge obviously, you decide - but would be really helpful to understand better what's going on and review)

🙏

[isidentical]

It's hard for me to figure out what do specific dvc status -c commands mean in the gist, what do we tests, what we don't test ...

Actually, it is a very simple example of push + pull of big single file / a dataset of a lot of small files with status -c's in the middle of every operation (e.g before push / after push) etc.

(not a blocker for merge obviously, you decide - but would be really helpful to understand better what's going on and review)

I'll try to prepare a proper table with before/after of different scenerios.

[isidentical]

We've discovered a bug that caused exists() calls on s3fs to take 6~ seconds for non-existent files, now this bug is resolved and it is taking 1.8-2.0~ seconds. This is still worse than our current implementation (for both existing and non-existing files, when the calls are made separately. This overhead is compensated when list_hashes_exist performs calls in pararel) (0.6-0.7~ seconds). For the last day, I've been debugging the cause and it appears to be the cost of aiobotocore + aiohttp (and all the underlying stuff).

[efiop]

Outstanding work 🔥

Let's merge and move forward for now, aiobotocore issue is a problem, but we can live with it for now (we also have some time before the next release, etc).

Thank you! 🙏