apache / apache/amoro

[Improvement]: Implement heap-based flush mechanism for SortedPosDeleteWriter to prevent OOM

Open
#4,166 0 comments 0 reactions 0 assignees View on GitHub
type:improvement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.