1996fanrui opened a new pull request, #27282:
URL: https://github.com/apache/flink/pull/27282
## What is the purpose of the change
## Problem
`RecordsWindowBuffer.addElement()` catches `EOFException` and retries
recursively, causing **StackOverflowError** when retry keeps failing.
## Root Cause
The original code assumes "flush will always free enough space for retry".
This assumption fails when unrecoverable errors occur, leading to infinite
recursion.
## Solution
Use `numKeys == 0` as the termination condition:
```java
while (true) {
LookupInfo<WindowKey, Iterator<RowData>> lookup =
recordsBuffer.lookup(reuseWindowKey);
try {
recordsBuffer.append(lookup, recordSerializer.toBinaryRow(element));
break;
} catch (EOFException e) {
if (recordsBuffer.getNumKeys() == 0) {
// Buffer is empty, retry won't help (unrecoverable error)
throw e;
}
// Buffer has data, flush and retry
flush();
checkState(recordsBuffer.getNumKeys() == 0, "The recordsBuffer
should be empty after flushing.");
}
}
```
## Why This Works
| Scenario | numKeys before catch | Behavior |
|----------|---------------------|----------|
| Buffer full (recoverable) | > 0 | flush → numKeys=0 → retry → success |
| Unrecoverable error (1st attempt) | 0 | throw immediately |
| Unrecoverable error (after flush retry) | 0 | flush → numKeys=0 → retry →
fail again → numKeys still 0 → throw |
Key point: `flush()` clears the buffer, making `numKeys == 0`. If retry
still fails after flush, entering the catch block again with `numKeys == 0`
indicates the problem is not caused by a full buffer, but an unrecoverable
error. In this case, an exception should be thrown instead of continuing to
retry.
## Benefits
1. **Prevents StackOverflowError**: The while loop executes at most twice
(initial attempt + retry after flush), no infinite loop
2. **Preserves normal behavior**: Normal flush + retry still works when
buffer is full
3. **Better diagnostics**: Unrecoverable errors throw a complete
EOFException with full stack trace, making it easy to identify the root cause
4. **Minimal change**: Only checks numKeys, no additional state variables
introduced
## Brief change log
- [hotfix] WindowBuffer implements AutoCloseable to avoid declare close
explicitly
- [FLINK-38746][table-runtime] Fix StackOverflowError in
RecordsWindowBuffer.addElement
## Verifying this change
- Added `RecordsWindowBufferTest`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
--
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]
