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

[BUG] AsyncTransferManager eatingup exception #10890

Closed
dhwanilpatel opened this issue Oct 24, 2023 · 4 comments
Closed

[BUG] AsyncTransferManager eatingup exception #10890

dhwanilpatel opened this issue Oct 24, 2023 · 4 comments
Labels
bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing Storage:Remote

Comments

@dhwanilpatel
Copy link
Contributor

Describe the bug
In AsyncTransferManager, at one place we are only throwing the exception. Considering it will be run in different thread, this exception might be lost from invoking thread, we should invoke returnFuture.completeExceptionally from there, so invoking thread's action listener will be invoked and appropriate action would be taken.

returnFuture.completeExceptionally(throwable);
} else {
try {
uploadRequest.getUploadFinalizer().accept(true);
} catch (IOException e) {
throw new RuntimeException(e);
}
returnFuture.complete(null);
}

@dhwanilpatel dhwanilpatel added bug Something isn't working untriaged labels Oct 24, 2023
@shwetathareja
Copy link
Member

@vikasvb90 to take a look.

@shwetathareja shwetathareja added Storage:Remote Indexing Indexing, Bulk Indexing and anything related to indexing and removed untriaged labels Oct 30, 2023
@vikasvb90
Copy link
Contributor

@dhwanilpatel I don't see the exception getting masked. I tested exceptions both from corrupted as well as IO block and I can see them propagated to S3BlobContainer here which is invoking onFailure of the completionListener. In fact, I can also see abort operation like deleteObject in single chunk upload getting invoked.

Can you mention a scenario where you saw this getting masked?

@vikasvb90
Copy link
Contributor

Also, it will be incorrect to invoke returnFuture from that block or any part failure block because if we do that then we will never be able to clean up uploaded data which is happening in the subsequent chained handle blocks which is dependent on the exception to trigger cleanup.

@Bukhtawar
Copy link
Collaborator

We decided to close it for now and pick it up with the broader Virtual Thread migration effort.

[Storage Triage - attendees 1 2 3 4 5 6 7 8 9] 10

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Storage Project Board May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing Storage:Remote
Projects
Status: ✅ Done
Development

No branches or pull requests

5 participants