apache / apache/iceberg

Spark: serializable-isolation tests can run until external CI/resource limits

Open
#16,359 0 comments 0 reactions 0 assignees View on GitHub
build
Dominant language
Java
Stars
9.2k
Forks
3.5k
Avg merge
2d 11h
Merged PRs (30d)
132

Description

## Summary

Some `spark-extensions` serializable-isolation concurrency tests wait on `Future.get()` without a timeout while worker threads loop up to `Integer.MAX_VALUE`.

If the expected `ValidationException` is not thrown, the test has no in-method upper bound and can continue running until CI timeout, disk exhaustion, or manual interruption.

## Example

Representative source:

https://github.com/apache/iceberg/blob/6976e020b894f6a6777704df2b8c4458cb291ae9/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMerge.java#L1538-L1628

The merge thread loops nearly unboundedly:

```java
for (int numOperations = 0; numOperations < Integer.MAX_VALUE; numOperations++) {
...
sql("MERGE INTO ... WHEN MATCHED THEN UPDATE SET dep = 'x'", commitTarget());
barrier.incrementAndGet();
}
```

The test then waits without a timeout:

```java
assertThatThrownBy(mergeFuture::get)
.isInstanceOf(ExecutionException.class)
.cause()
.isInstanceOf(ValidationException.class)
.hasMessageContaining("Found conflicting files that can contain");
```

Cleanup is in `finally`, so it is not reached unless `mergeFuture.get()` returns or throws.

## CI evidence

This is not only theoretical. In PR #16303, CI was retriggered multiple times before the root cause was understood.

The failure was later identified as a legitimate runaway-loop issue: the test kept running until GitHub Actions reported `No space left on device`.

## Affected area

The pattern appears in serializable-isolation row-level operation tests, including:

- `TestMerge.testMergeWithSerializableIsolation`
- `TestDelete.testDeleteWithSerializableIsolation`
- `TestUpdate.testUpdateWithSerializableIsolation`

These tests are inherited by the corresponding copy-on-write / merge-on-read test classes where present.

## Suggested fix

Add an explicit upper bound so these tests fail fast with useful diagnostics instead of relying on CI timeouts or disk exhaustion.

Possible options:

- use `Future.get(timeout, unit)`;
- add a JUnit timeout;
- bound the operation loop to a reasonable retry count;
- ensure both futures are cancelled and the executor is shut down on timeout.

Contributor guide

Open the contributing guide

Research direction

Start with TestMerge.testMergeWithSerializableIsolation at the referenced section, then compare the corresponding tests in TestDelete and TestUpdate. Run the affected serializable-isolation tests and trace the Future.get() and finally cleanup paths. Done means a failed validation cannot leave an unbounded worker loop running until CI or disk limits, and both futures and the executor are cleaned up on timeout.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spark
Domain
testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.