dataroaring commented on PR #55751:
URL: https://github.com/apache/doris/pull/55751#issuecomment-3284141640
**❓ QUESTION: Memory Check Logic Verification**
**File**: `be/src/olap/tablet.cpp` lines 184-191
```cpp
if (injection ||
!GlobalMemoryArbitrator::is_exceed_soft_mem_limit(GB_EXCHANGE_BYTE)) {
// Trigger compaction
auto st = _engine.submit_compaction_task(tablet_sptr(),
CompactionType::CUMULATIVE_COMPACTION, true);
```
**Question**: Is this logic correct?
**Current Behavior**: Compaction is triggered when:
- Debug injection is active **OR**
- Memory is **NOT** exceeded
**Concern**: Should compaction really be triggered when memory limits are
exceeded? Usually we want to **avoid** heavy operations like compaction when
memory is constrained.
**Please Verify**:
1. Is it intentional to trigger compaction during high memory usage?
2. Does the debug injection need to bypass memory safety checks?
3. Should the logic be: "Only trigger compaction if memory is NOT exceeded
(unless debug injection)"?
**Alternative Logic** (if my concern is valid):
```cpp
bool memory_allows_compaction =
!GlobalMemoryArbitrator::is_exceed_soft_mem_limit(GB_EXCHANGE_BYTE);
if (injection || memory_allows_compaction) {
// This is clearer about the intent
auto st = _engine.submit_compaction_task(...);
```
**Context**: The original code had memory checking for good reason. Want to
ensure this refactoring doesn't inadvertently change the intended memory
management behavior.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
