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

Close s3 filehandle on destruction #9758

Merged
merged 1 commit into from Nov 22, 2023
Merged

Conversation

samansmink
Copy link
Contributor

Fixes #9727

S3 filehandle would not close automatically for s3 filehandles meaning that the multipart uploads would never be finished when the filehandle was not explicitly closed.

@Mytherin Mytherin merged commit a9e050d into duckdb:main Nov 22, 2023
42 of 43 checks passed
@Mytherin
Copy link
Collaborator

Thanks! LGTM

@brianwyka
Copy link
Contributor

@samansmink , is it possible that there are any non closed S3FileHandle references in CSV/Parquet reads, or was it only a problem for writes/uploads?

@samansmink
Copy link
Contributor Author

@brianwyka That would be possible, but i don't see how that would cause issues: the problem here was that on close, the filehandle would need to make an additional request to finalize the multipart upload. For reading there is no such thing, the filehandle can just be discarded and calling Close doesn't really do anything

@brianwyka
Copy link
Contributor

Okay thanks, was trying to see if it would help out with this issue I opened up: #9712

krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Dec 14, 2023
Merge pull request duckdb/duckdb#9772 from hawkfish/skiplist-rand
Merge pull request duckdb/duckdb#9767 from samansmink/fix-vcpkg-patching-on-windows
Merge pull request duckdb/duckdb#9757 from Tmonster/712-unicode-bugs
Merge pull request duckdb/duckdb#9758 from samansmink/fix-issue-9727
Merge pull request duckdb/duckdb#9759 from samansmink/update-vcpkg-2023.10.19
Merge pull request duckdb/duckdb#9761 from samansmink/add-nightly-deploy-script
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.

export database doesn't create sql files when exporting to s3 parquet
3 participants