Fix handling of config files while loading datasets from multiple processes
MEMBER
Ok for this but I think we may want to use the general `filelock` method we are using at other places in the library instead of filecmp (in particular `filelock` take care of being an atomic operation which is safer for concurrent processes)
2020-07-30T08:43:30Z
https://github.com/huggingface/datasets/pull/432#issuecomment-666232833
https://api.github.com/repos/huggingface/datasets/issues/432
MDEyOklzc3VlQ29tbWVudDY2NjIzMjgzMw==
https://api.github.com/repos/huggingface/datasets/issues/comments/666232833/reactions
2020-07-30T08:43:30Z
https://api.github.com/repos/huggingface/datasets/issues/comments/666232833
https://avatars.githubusercontent.com/u/7353373?v=4
https://api.github.com/users/thomwolf/events{/privacy}
https://api.github.com/users/thomwolf/followers
https://api.github.com/users/thomwolf/following{/other_user}
https://api.github.com/users/thomwolf/gists{/gist_id}
https://github.com/thomwolf
thomwolf
MDQ6VXNlcjczNTMzNzM=
https://api.github.com/users/thomwolf/orgs
https://api.github.com/users/thomwolf/received_events
https://api.github.com/users/thomwolf/repos
https://api.github.com/users/thomwolf/starred{/owner}{/repo}
https://api.github.com/users/thomwolf/subscriptions
User
https://api.github.com/users/thomwolf
public
MEMBER
Ok I see.
Why not use filelock in this case then ?
2020-07-30T08:45:57Z
https://github.com/huggingface/datasets/pull/432#issuecomment-666234251
https://api.github.com/repos/huggingface/datasets/issues/432
MDEyOklzc3VlQ29tbWVudDY2NjIzNDI1MQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/666234251/reactions
2020-07-30T08:45:57Z
https://api.github.com/repos/huggingface/datasets/issues/comments/666234251
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
I think we should 🙂
2020-07-30T08:48:02Z
https://github.com/huggingface/datasets/pull/432#issuecomment-666235461
https://api.github.com/repos/huggingface/datasets/issues/432
MDEyOklzc3VlQ29tbWVudDY2NjIzNTQ2MQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/666235461/reactions
2020-07-30T08:48:02Z
https://api.github.com/repos/huggingface/datasets/issues/comments/666235461
https://avatars.githubusercontent.com/u/7353373?v=4
https://api.github.com/users/thomwolf/events{/privacy}
https://api.github.com/users/thomwolf/followers
https://api.github.com/users/thomwolf/following{/other_user}
https://api.github.com/users/thomwolf/gists{/gist_id}
https://github.com/thomwolf
thomwolf
MDQ6VXNlcjczNTMzNzM=
https://api.github.com/users/thomwolf/orgs
https://api.github.com/users/thomwolf/received_events
https://api.github.com/users/thomwolf/repos
https://api.github.com/users/thomwolf/starred{/owner}{/repo}
https://api.github.com/users/thomwolf/subscriptions
User
https://api.github.com/users/thomwolf
public
CONTRIBUTOR
Thanks for approving my patch.

I agree that if copying is needed then some locking mechanism should be put in place. But, I don't think a file should be needlessly copied without a check. So I guess the flow should be, lock => copy if needed => unlock, and add locks wherever else that file is being accessed.

I'll also add that my personal experience with filelock on a different project hasn't been that great, and on some occasions a process somehow got through the lock -- I've never gotten to the bottom of that but it tainted my view of that module. Perhaps it's been fixed (or I just miss used it), but thought you should know to take steps to test it.
2020-07-30T18:19:23Z
https://github.com/huggingface/datasets/pull/432#issuecomment-666578038
https://api.github.com/repos/huggingface/datasets/issues/432
MDEyOklzc3VlQ29tbWVudDY2NjU3ODAzOA==
https://api.github.com/repos/huggingface/datasets/issues/comments/666578038/reactions
2020-07-30T18:19:23Z
https://api.github.com/repos/huggingface/datasets/issues/comments/666578038
https://avatars.githubusercontent.com/u/99543?v=4
https://api.github.com/users/orsharir/events{/privacy}
https://api.github.com/users/orsharir/followers
https://api.github.com/users/orsharir/following{/other_user}
https://api.github.com/users/orsharir/gists{/gist_id}
https://github.com/orsharir
orsharir
MDQ6VXNlcjk5NTQz
https://api.github.com/users/orsharir/orgs
https://api.github.com/users/orsharir/received_events
https://api.github.com/users/orsharir/repos
https://api.github.com/users/orsharir/starred{/owner}{/repo}
https://api.github.com/users/orsharir/subscriptions
User
https://api.github.com/users/orsharir
public
When loading shards on several processes, each process upon loading the dataset will overwrite dataset_infos.json in <package path>/datasets/<dataset name>/<hash>/dataset_infos.json. It does so every time, even when the target file already exists and is identical. Because multiple processes rewrite the same file in parallel, it creates a race condition when a process tries to load the file, often resulting in a JSON decoding exception because the file is only partially written.

This pull requests partially address this by comparing if the files are already identical before copying over the downloaded copy to the cached destination. There's still a race condition, but now it's less likely to occur if some basic precautions are taken by the library user, e.g., download all datasets to cache before spawning multiple processes.