Skip to content

Failed positional-delete load leaves the file stuck in Loading, hanging later loads #3434

Open

Description

@JeonDaehong

Apache Iceberg Rust version

Verified on main at b154603 (the version field only lists releases)

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).

Test (crates/iceberg/src/arrow/caching_delete_file_loader.rs, tests module)
#[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.

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions