[Improvement]: Implement heap-based flush mechanism for SortedPosDeleteWriter to prevent OOM
- Dominant language
- Java
- Stars
- 1.2k
- Forks
- 395
- Avg merge
- 4d 10h
- Merged PRs (30d)
- 33
Description
### Search before asking
- [x] I have searched in the [issues](https://github.com/apache/amoro/issues?q=is%3Aissue) and found no similar issues.
### What would you like to be improved?
Currently, `SortedPosDeleteWriter` only flushes buffered position deletes based on a record count threshold. There is a `TODO` comment in the code indicating the need for a heap memory-based flush policy:
```
// TODO Flush buffer based on the policy that checking whether whole heap memory size exceed the
// threshold.
if (records >= recordsNumThreshold) {
flushDeletes();
}
```
**Problem**: When processing large-scale position deletes, the in-memory buffer in `SortedPosDeleteWriter` can grow unbounded (if record threshold is set very high or to `Long.MAX_VALUE`), potentially causing OutOfMemoryError (OOM) issues, especially in memory-constrained environments.
**Current behavior**:
- Only flushes when record count reaches **recordsNumThreshold**
- No protection against heap memory pressure
- Can lead to OOM when processing large delete operations
### How should we improve?
Implement a heap memory-based flush mechanism with the following features:
### 1. New table properties:
- `pos-delete.flush.heap.ratio (default: 0.8)` - Heap usage ratio threshold to trigger flush
- `pos-delete.flush.records (default: Long.MAX_VALUE)` - Record count threshold
- `pos-delete.flush.heap.min-records (default: 1000)` - Minimum records before heap-based flush kicks in
### 2. Implementation details:
- Add `HeapUsageProvider` interface to monitor JVM heap usage
- Implement `shouldFlushByHeap()` method to check if heap usage exceeds threshold
- Modify flush logic to: `if (records >= recordsNumThreshold || shouldFlushByHeap())`
- Ensure backward compatibility through constructor overloads
### 3. Safety guards:
- Prevent frequent small flushes with minimum record count
- Allow disabling heap-based flush by setting invalid ratio (≤0 or ≥1)
- Non-intrusive monitoring (no forced GC)
### Are you willing to submit PR?
- [x] Yes I am willing to submit a PR!
### Subtasks
_No response_
### Code of Conduct
- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
Contributor guide
Research direction
Start at SortedPosDeleteWriter and the TODO around its record-count flush condition; trace its constructors and table-property handling. Define the HeapUsageProvider and heap-based policy alongside the existing record threshold, then verify that minimum-record and invalid-ratio guards, constructor compatibility, and non-forced-GC monitoring are covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- data-engineering, databases
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100