j1wonpark commented on code in PR #4276:
URL: https://github.com/apache/amoro/pull/4276#discussion_r3943951575
##########
amoro-format-iceberg/src/main/java/org/apache/amoro/io/writer/SortedPosDeleteWriter.java:
##########
@@ -49,6 +49,27 @@
*/
public class SortedPosDeleteWriter<T> implements Closeable {
private static final long DEFAULT_RECORDS_NUM_THRESHOLD = Long.MAX_VALUE;
+ private static final Runtime RUNTIME = Runtime.getRuntime();
+ // Note: totalMemory/freeMemory only reflect the currently allocated heap,
not max heap.
+ // When the heap has not fully expanded, usedMemory() can be underestimated
and delay flushes.
+ private static final HeapUsageProvider RUNTIME_HEAP_USAGE =
+ new HeapUsageProvider() {
+ @Override
+ public long maxMemory() {
+ return RUNTIME.maxMemory();
+ }
+
+ @Override
+ public long usedMemory() {
+ return RUNTIME.totalMemory() - RUNTIME.freeMemory();
Review Comment:
(blocking) `totalMemory() - freeMemory()` counts garbage that has not been
collected yet, and it is a JVM-wide number shared by every rewrite task running
in the optimizer. With G1 the heap routinely sits above 80% between collections
even when live data is small, so this writer would flush every 1,000 records
regardless of how much it actually buffers, and one task's memory use would
fragment every other task's pos-delete output. I don't think a process-wide
heap ratio can tell us anything about this writer's own buffer.
##########
amoro-format-iceberg/src/main/java/org/apache/amoro/table/TableProperties.java:
##########
@@ -299,6 +299,16 @@ private TableProperties() {}
org.apache.iceberg.TableProperties.WRITE_TARGET_FILE_SIZE_BYTES;
public static final long WRITE_TARGET_FILE_SIZE_BYTES_DEFAULT = 134217728;
// 128 MB
+ public static final String POS_DELETE_FLUSH_HEAP_RATIO =
"pos-delete.flush.heap.ratio";
+ public static final double POS_DELETE_FLUSH_HEAP_RATIO_DEFAULT = 0.8d;
Review Comment:
(blocking) Since `MixedIcebergRewriteExecutor` / `MixedHiveRewriteExecutor`
now pass `table.properties()`, the `0.8` default enables this for every
mixed-format optimizing task on upgrade. Combined with the point above,
existing users would go from one pos-delete file per tree node to many small
ones without opting in. Whatever flush policy we land on should be opt-in.
--
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]
