xintongsong commented on a change in pull request #10921: [FLINK-15692][state
backend] Enable limiting RocksDB memory consumption by default
URL: https://github.com/apache/flink/pull/10921#discussion_r369906968
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java
##########
@@ -71,12 +74,26 @@
protected abstract B getStateBackend() throws Exception;
+ protected abstract long getManagedMemorySize();
Review comment:
@StephanEwen
I find it difficult to follow the entire conversation, but I can try to
explain what's the problem of having on-heap managed memory for RocksDB state
backend test cases:
- RocksDB state backend (or to be specific
`RocksDBOperationUtils#allocateSharedCachesIfConfigured`) calls
`MemoryManager#getSharedMemoryResourceForManagedMemory` for reserving and
allocating managed memory, with 1.0 as the `fractionToInitializeWith`.
- In `getSharedMemoryResourceForManagedMemory`, we first compute the memory
size with the fraction 1.0, then reserve that amount of memory.
- While `computeMemorySize` returns memory size including both on-heap and
off-heap, we reserve only off-heap memory.
- If the memory manager contains both on-heap and off-heap memory, no matter
eagerly allocated/reserved or not, we will try to reserve more off-heap memory
than it has.
- E.g., memory manager contains 50mb on-heap budget and 100mb off-heap
budget, in `getSharedMemoryResourceForManagedMemory` we will first compute the
memory size as 150mb, then try to reserve 150mb off-heap memory which is larger
than the off-heap budget.
Currently memory manager created from the testing util class
`MemoryManagerBuilder` will always contain some on-heap memory, which is added
in the constructor of `MemoryManagerBuilder`. When @Myasuka asked yesterday
offline, I suggested to check whether any budget is set in
`MemoryManagerBuilder#build`, and only add the default on-heap budget if no
other budget is set. In this way, we can keep the batch test cases to use
on-heap managed memory for less testing overhead, while have RocksDB state
backend test cases to explicitly use off-heap managed memory only.
An alternative could be make `MemoryManager#computeMemorySize` returns only
size of off-heap memory. However, since we do not have on-heap managed memory
in production, I'm in favor of keep the concern of excluding on-heap memory in
testing codes only.
@tillrohrmann @Myasuka
I'm not sure whether its necessary to set managed memory to 0 for
non-RocksDB cases. Since managed memory is always lazily allocated/reserved, I
think streaming test cases with non-RocksDB state backend should not reserve
managed memory at all, and batch test cases should only allocate on-heap
managed memory if needed (which is the same behavior as before).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
