Skip to content

Commit d87388b

Browse files
committed
fix memory conservation bug
1 parent d4035bd commit d87388b

File tree

6 files changed

+36
-23
lines changed

6 files changed

+36
-23
lines changed

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/memory/arbitrator/MemoryArbitratorImpl.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public long reclaimLocally(MemoryPool queryPool, long neededBytes, MemoryPool re
3838
long startTime = System.currentTimeMillis();
3939
long res = queryPool.tryToReclaimLocalMemory(neededBytes, requestingPool);
4040
LOG.info("[{}] reclaim local memory: {} bytes, took {} ms",
41-
Thread.currentThread().getName(),
41+
queryPool,
4242
res,
4343
System.currentTimeMillis() - startTime);
4444
return res;
@@ -65,7 +65,7 @@ public long reclaimGlobally(MemoryPool queryPool, long neededBytes) {
6565
}
6666
}
6767
LOG.info("[{}] reclaim global memory: {} bytes, took {} ms",
68-
Thread.currentThread().getName(),
68+
queryPool,
6969
totalReclaimedBytes,
7070
System.currentTimeMillis() - startTime);
7171
return totalReclaimedBytes;

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/memory/pool/AbstractMemoryPool.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ private long reclaimChildren(long neededBytes, MemoryPool requestingPool) {
9494
long totalReclaimedBytes = 0;
9595
long currentNeededBytes = neededBytes;
9696
for (MemoryPool child : this.children) {
97+
if (child.equals(requestingPool)) {
98+
continue;
99+
}
97100
long reclaimedMemory =
98101
child.tryToReclaimLocalMemory(currentNeededBytes, requestingPool);
99102
if (reclaimedMemory > 0) {
@@ -217,16 +220,15 @@ public MemoryPool getParentPool() {
217220
return this.parent;
218221
}
219222

220-
@Override
221-
public String getName() {
222-
return this.stats.getMemoryPoolName();
223-
}
224-
225223
@Override
226224
public String toString() {
227225
return this.getName();
228226
}
229227

228+
public String getName() {
229+
return this.stats.getMemoryPoolName();
230+
}
231+
230232
@Override
231233
public MemoryPool findRootQueryPool() {
232234
if (this.parent == null) {

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/memory/pool/MemoryPool.java

-2
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ public interface MemoryPool {
9292

9393
long getMaxCapacityBytes();
9494

95-
String getName();
96-
9795
MemoryPool getParentPool();
9896

9997
MemoryPool findRootQueryPool();

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/memory/pool/impl/OperatorMemoryPool.java

+9-2
Original file line numberDiff line numberDiff line change
@@ -189,16 +189,23 @@ public long requestMemoryInternal(long size, MemoryPool requestingPool) throws
189189
// 3. call father
190190
long fatherRes =
191191
getParentPool().requestMemoryInternal(neededMemorySize, requestingPool);
192-
if (fatherRes < 0) {
192+
if (fatherRes <= 0) {
193193
LOG.error("[{}] requestMemory failed because of OOM, request size={}", this,
194194
size);
195+
// if parentRes <= 0, indicating we don't get enough memory bytes.
196+
// But we still need to allocate these memory bytes to operatorPool to ensure
197+
// memory is conserved when parentRes < 0
198+
if (fatherRes < 0) {
199+
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() - fatherRes);
200+
this.stats.setNumExpands(this.stats.getNumExpands() + 1);
201+
}
195202
this.stats.setNumAborts(this.stats.getNumAborts() + 1);
196203
throw new OutOfMemoryException(String.format("%s requestMemory failed " +
197204
"because of OOM, request " +
198205
"size=%s", this, size));
199206
}
200207
// 4. update stats
201-
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() + neededMemorySize);
208+
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() + fatherRes);
202209
this.stats.setNumExpands(this.stats.getNumExpands() + 1);
203210
LOG.debug("[{}] requestMemory success: requestedMemorySize={}", this, fatherRes);
204211
return fatherRes;

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/memory/pool/impl/QueryMemoryPool.java

+12-12
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public long requestMemoryInternal(long bytes, MemoryPool requestingPool) {
105105
}
106106
}
107107
if (requestedMemoryFromManager > 0) {
108-
this.memoryManager.consumeAvailableMemory(bytes);
108+
this.memoryManager.consumeAvailableMemory(requestedMemoryFromManager);
109109
}
110110
return requestedMemoryFromManager == 0 ? requestedMemoryFromArbitration :
111111
requestedMemoryFromManager;
@@ -134,30 +134,30 @@ private long requestMemoryThroughArbitration(long bytes, MemoryPool requestingPo
134134
this.getSnapShot());
135135
long reclaimedBytes =
136136
this.memoryManager.triggerLocalArbitration(this, bytes, requestingPool);
137-
if (reclaimedBytes > 0) {
138-
this.stats.setNumExpands(this.stats.getNumExpands() + 1);
139-
}
140137
// 1. if arbitrate successes, update stats and return success
141138
if (reclaimedBytes - bytes >= 0) {
142139
// here we don't update capacity & reserved & allocated, because memory is
143140
// reclaimed from queryPool itself.
144-
return bytes;
141+
return reclaimedBytes;
145142
} else {
146143
// 2. if still not enough, try to reclaim globally
147144
long globalArbitrationNeededBytes = bytes - reclaimedBytes;
148145
long globalReclaimedBytes = this.memoryManager.triggerGlobalArbitration(this,
149146
globalArbitrationNeededBytes);
150147
reclaimedBytes += globalReclaimedBytes;
151-
// 3. if memory is enough, update stats and return success
152-
if (reclaimedBytes - bytes >= 0) {
153-
// add capacity
148+
// add capacity whether arbitration failed or not.
149+
// NOTE: only bytes gained from global arbitration can be added to stats.
150+
if (globalReclaimedBytes != 0) {
154151
this.stats.setMaxCapacity(this.stats.getMaxCapacity() + globalReclaimedBytes);
155-
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() + bytes);
152+
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() + globalReclaimedBytes);
156153
this.stats.setNumExpands(this.stats.getNumExpands() + 1);
157-
return bytes;
154+
}
155+
// 3. if memory is enough, update stats and return success
156+
if (reclaimedBytes - bytes >= 0) {
157+
return reclaimedBytes;
158158
}
159159
}
160-
// 4. if arbitrate fails, return -1, indicating that request failed.
161-
return -1;
160+
// 4. if arbitrate fails, return -reclaimedBytes, indicating that request failed.
161+
return -reclaimedBytes;
162162
}
163163
}

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/memory/pool/impl/TaskMemoryPool.java

+6
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,12 @@ public long requestMemoryInternal(long bytes, MemoryPool requestingPool) throws
100100
if (parentRes > 0) {
101101
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() + parentRes);
102102
this.stats.setNumExpands(this.stats.getNumExpands() + 1);
103+
} else if (parentRes < 0){
104+
// if parentRes < 0, indicating we don't get enough memory bytes. But we still
105+
// need to allocate these memory bytes to operatorPool to ensure memory is
106+
// conserved.
107+
this.stats.setAllocatedBytes(this.stats.getAllocatedBytes() - parentRes);
108+
this.stats.setNumExpands(this.stats.getNumExpands() + 1);
103109
}
104110
return parentRes;
105111
} catch (InterruptedException e) {

0 commit comments

Comments
 (0)