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

Enhance TrinoFileSystem#deleteFiles with files failure count #16006

Conversation

krvikash
Copy link
Contributor

@krvikash krvikash commented Feb 7, 2023

Description

I am proposing to enhance TrinoFileSystem#deleteFiles to track the number of files that failed to delete.

Failure count is needed for the case when we want to throw BulkDeletionFailureException in #15981

See Comments here
#15981 (comment)
#15981 (comment)

Release notes

(X) This is not user-visible or docs only and no release notes are required.

@@ -1231,12 +1236,13 @@ private void executeExpireSnapshots(ConnectorSession session, IcebergTableExecut
long expireTimestampMillis = session.getStart().toEpochMilli() - retention.toMillis();
TrinoFileSystem fileSystem = fileSystemFactory.create(session);
List<String> pathsToDelete = new ArrayList<>();
AtomicLong failedToDelete = new AtomicLong();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't have to be thread safe, if that delete function was called from multi thread it probably wouldn't work as pathsToDelete is not thread safe.

@krvikash krvikash force-pushed the enhance-batch-delete-with-failure-count branch from afb1f78 to a8e21e0 Compare February 7, 2023 10:48
@findepi
Copy link
Member

findepi commented Feb 7, 2023

This still requires answer to #15981 (comment) question

what is org.apache.iceberg.io.BulkDeletionFailureException#numberFailedObjects needed for?

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Mar 2, 2023
@mosabua
Copy link
Member

mosabua commented Mar 2, 2023

Looks like the question from @findepi was answered on the other PR and that was merged. Did you want to proceed with this @krvikash ?

@krvikash
Copy link
Contributor Author

krvikash commented Mar 3, 2023

This still requires answer to #15981 (comment) question

what is org.apache.iceberg.io.BulkDeletionFailureException#numberFailedObjects needed for?

Apart from throwing runtime exception with message format("Failed to delete %d files", numberFailedObjects), I don't see any other usage of numberFailedObjects.

@findepi should we close this PR?

@findepi findepi closed this Mar 3, 2023
@krvikash krvikash deleted the enhance-batch-delete-with-failure-count branch March 3, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants