[I] Failed positional-delete load leaves the file stuck in Loading, hanging later loads [iceberg-rust]

["JeonDaehong (via GitHub)" <gi...@apache.org>]

JeonDaehong opened a new issue, #3434:
URL: https://github.com/apache/iceberg-rust/issues/3434

   ### Apache Iceberg Rust version
   
   v0.10.0
   
   ### Describe the bug
   
   If loading a positional delete file fails in `CachingDeleteFileLoader` (for example, the file can't be opened yet), a later `load_deletes` for the same file on the same loader never completes.
   
   `try_start_pos_del_load` marks the file `PosDelState::Loading`, and only the success path calls `finish_pos_del_load`. After a failure the entry stays `Loading`, so the next caller gets `PosDelLoadAction::WaitFor` and waits for a notification that never comes. `ArrowReader` clones share this state, so retrying `read` on a clone hangs too.
   
   This is the positional half of #3274 (equality deletes, fixed in #3275). #3433 adds another retryable failure on this path: the metadata request for a delete file of unknown size.
   
   ### To Reproduce
   
   1. Load a position delete file whose path doesn't exist yet: `load_deletes` fails with `Failed to open file ...`.
   2. Create the file and call `load_deletes` again on the same loader: it doesn't complete (stopped after 3s).
   3. A new `CachingDeleteFileLoader` reads the same file fine.
   
   The same happens with an unknown size on #3433 (the first call fails with `Failed to stat delete file`).
   
   <details>
   <summary>Test (crates/iceberg/src/arrow/caching_delete_file_loader.rs, tests module)</summary>
   
   ```rust
   #[tokio::test]
   async fn retry_after_failed_pos_del_load() {
       use std::time::Duration;
   
       let tmp_dir = TempDir::new().unwrap();
       let file_scan_tasks = setup(tmp_dir.path());
       let task = &file_scan_tasks[0];
       let pos = task
           .deletes()
           .iter()
           .find(|d| d.file_type() == DataContentType::PositionDeletes)
           .unwrap()
           .clone();
       let path = pos.file_path().to_string();
       let moved = format!("{path}.moved");
       std::fs::rename(&path, &moved).unwrap();
   
       let loader = CachingDeleteFileLoader::new(FileIO::new_with_fs(), 10, Runtime::current());
       let first = loader
           .load_deletes(std::slice::from_ref(&pos), task.schema_ref())
           .await
           .unwrap();
       assert!(first.is_err());
   
       std::fs::rename(&moved, &path).unwrap();
       let second = tokio::time::timeout(
           Duration::from_secs(3),
           loader.load_deletes(std::slice::from_ref(&pos), task.schema_ref()),
       )
       .await;
       assert!(second.is_ok(), "second load on the same loader did not complete");
   }
   ```
   
   On `main` this fails at the last assertion after 3s.
   
   </details>
   
   ### Expected behavior
   
   A failed load shouldn't block later loads of the same file: retrying should either load it or return the error again.
   
   ### Willingness to contribute
   
   I can contribute a fix for this bug independently


-- 
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: issues-unsubscribe@iceberg.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org