Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize VFS::touch and fix defects and race conditions. #5285

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

teo-tsirpanis
Copy link
Member

SC-53188
SC-53189
SC-54494

This PR updates the implementations of VFS::touch for Azure, GCS and Windows to improve performance and fix race conditions:

  • On Azure we remove checking if the blob exists, and instead use a single conditional request to upload an empty blob if it does not already exist. This reduces round-trips and fixes race conditions if the blob was created between the existence check and the upload.
  • GCS did not even have an existence check, which means that the behavior of touch was incorrect and always overwrote the file. This was fixed by adding a request precondition.
  • On Windows the existence check was removed. We were already doing the right thing and opened the file with CREATE_NEW, but did not mask failures with ERROR_FILE_EXISTS status code. We fix this.

Additionally, the documentation of tiledb_vfs_touch was updated to mention that the file is created only if it does not exist, matching existing behavior in most VFS implmentations, and the expectations from the touch command.

Testing coverage was added.


TYPE: BUG
DESC: Fixed tiledb_vfs_touch to not overwrite existing files on GCS.


TYPE: BUG
DESC: Fixed tiledb_vfs_touch to not overwrite existing files or fail on Azure and Windows respectively, under race conditions.


TYPE: C_API
DESC: Update documentation of tiledb_vfs_touch to specify that existing files are not overwritten, matching the current behavior.

@teo-tsirpanis teo-tsirpanis marked this pull request as ready for review September 5, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant