Fix for checkpoint rename race condition
Author: tblattner
Pull request: yes

# What does this PR do?
When running distributed training with deepspeed, I was encountered a race condition due to os.rename not being atomic on network filesystems. This rework, changes the logic for renaming to only run on the main processes, or a single main process depending on the save_on_each_node flag. Also added is the use of fsync to try to flush buffers, hopefully ensuring the rename is completed. fsync may have no effect in some filesystems, so a better mechanism may be required to ensure that the rename completed.

Fixes #27925


## Before submitting
- [No] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [Yes] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [Discussed on Github issue] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [Yes] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [No] Did you write any new necessary tests?


## Who can review?
@muellerzr 
@pacman100


Comments:
One thing to note, I attempted to reuse the existing with block: 
```with self.args.main_process_first(desc="Renaming model checkpoint folder to true location", local=self.args.save_on_each_node):```

and include the fsync. Unfortunately fsync did not flush buffers related to the staging directory, so it still failed on other processes. This raises some concerns as to the behavior of fsync on network attached storage using NFS.

Oops, missed this. I looked yesterday :) but I guess you poster after I looked. This is my version:
https://github.com/huggingface/transformers/pull/28373

I don't see why existence check is required if it is happening only once per node.

> Oops, missed this. I looked yesterday :) but I guess you poster after I looked. This is my version: #28373
> 
> I don't see why existence check is required if it is happening only once per node.

Could be a race if the output directory for the checkpoint is used sometime later in the code. If that is not the case, then shouldn't be an issue.

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_28364). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Thanks @tblattner 🤗 

- `if self.is_local_process_zero() if self.args.save_on_each_node else self.is_world_process_zero():`
- `if self.args.should_save`
May I ask What is the difference between the two lines above？Can replace Line1 with Line2

> * `if self.is_local_process_zero() if self.args.save_on_each_node else self.is_world_process_zero():`
> * `if self.args.should_save`
>   May I ask What is the difference between the two lines above？Can replace Line1 with Line2

Definition of should_save looks like they would be equivalent and the second one a little clearer. It will also allow the extra check below for should_save to be removed.

Looks like `fd = os.open(output_dir, os.O_RDONLY)` doesn't work on Windows:
![image](https://github.com/huggingface/transformers/assets/28014244/5449c420-26e8-4252-b108-df7e52679424)


> Looks like `fd = os.open(output_dir, os.O_RDONLY)` doesn't work on Windows: ![image](https://private-user-images.githubusercontent.com/28014244/298395624-5449c420-26e8-4252-b108-df7e52679424.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDU4OTIzMTQsIm5iZiI6MTcwNTg5MjAxNCwicGF0aCI6Ii8yODAxNDI0NC8yOTgzOTU2MjQtNTQ0OWM0MjAtMjZlOC00MjUyLWIxMDgtZGY3ZTUyNjc5NDI0LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAxMjIlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMTIyVDAyNTMzNFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPThmMGMwOTEwYjc0ZGNiMjE1YjAwMjU1ODU2NmE4MmQxM2ZjMWZkOGQ1ZDE4ZGI4NTY5YjY1NmExZWFmN2FlMmImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.7q7GQD1FWee4DlY15fryneYUKq2mUEhepjvXFkLA5tw)

This is indeed an issue. Windows handles directories differently than Linux. I'm not an expert at Windows dev in python, so I don't have a good solution sorry!

I think this is critical, when your training fails at the first checkpoint. Maybe as a workaround we can add a condition for non-Windows systems?
```python
if platform.system() != 'Windows':
    fd = os.open(output_dir, os.O_RDONLY)
    os.fsync(fd)
    os.close(fd)
```
@muellerzr @siddartha-RE 

Yes, quick PR going in a moment.

@Vechtomov @tblattner https://github.com/huggingface/transformers/pull/28637 fixed it.

Unsure how it affects multinode on windows but if a user has this situation and hits it then we can deal with it then as there's not really a clean solution for doing so in python :(