laskoviymishka commented on code in PR #3435:
URL: https://github.com/apache/iceberg-rust/pull/3435#discussion_r4362960615
##########
crates/iceberg/src/arrow/caching_delete_file_loader.rs:
##########
@@ -271,25 +271,31 @@ impl CachingDeleteFileLoader {
return Self::load_deletion_vector(task,
basic_delete_file_loader).await;
}
- match del_filter.try_start_pos_del_load(task.file_path()) {
- PosDelLoadAction::AlreadyLoaded =>
Ok(DeleteFileContext::ExistingPosDel),
- PosDelLoadAction::WaitFor(notified) => {
- // Positional deletes are accessed synchronously by
ArrowReader.
- // We must wait here to ensure the data is ready
before returning,
- // otherwise ArrowReader might get an empty/partial
result.
- notified.await;
- Ok(DeleteFileContext::ExistingPosDel)
+ loop {
+ match del_filter.try_start_pos_del_load(task.file_path()) {
+ PosDelLoadAction::AlreadyLoaded => {
+ return Ok(DeleteFileContext::ExistingPosDel);
+ }
+ PosDelLoadAction::WaitFor(notified) => {
+ // Positional deletes are accessed synchronously
by ArrowReader.
+ // We must wait here to ensure the data is ready
before returning,
+ // otherwise ArrowReader might get an
empty/partial result.
+ // The wait also ends if that load fails, so check
again.
+ notified.await;
+ }
Review Comment:
For example, say one delete file is shared by 200 scan tasks and that file
is missing or corrupt. With retry, we can end up reopening the same object ~200
times serially before the last task finally fails. If we surface the error,
it’s one open and all waiting tasks fail fast.
Maybe a middle ground is to return the error only to the tasks waiting on
that attempt, but still clear the entry so later calls can retry, similar to
Go’s `singleflight`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
