Skip to content

Commit bfe75c0

Browse files
committed
complete OOM UT and fix bugs
1 parent 54d1fd8 commit bfe75c0

File tree

6 files changed

+53
-19
lines changed

6 files changed

+53
-19
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public void gcQueryMemoryPool(MemoryPool pool) {
8080
LOG.info("Manager gc query memory pool {}", pool);
8181
queryMemoryPools.remove(pool);
8282
long reclaimedMemory = pool.getAllocatedBytes();
83-
pool.releaseSelf(String.format("GC query memory pool %s", pool));
83+
pool.releaseSelf(String.format("GC query memory pool %s", pool), false);
8484
currentAvailableMemoryInBytes.addAndGet(reclaimedMemory);
8585
}
8686

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

+16-10
Original file line numberDiff line numberDiff line change
@@ -96,35 +96,41 @@ public long tryToReclaimLocalMemory(long neededBytes) {
9696
* called when one layer pool is successfully executed and exited.
9797
*/
9898
@Override
99-
public void releaseSelf(String reason) {
100-
this.memoryActionLock.lock();
99+
public void releaseSelf(String reason, boolean isTriggeredInternal) {
101100
try {
102-
if (this.isBeingArbitrated.get()) {
103-
this.condition.await();
101+
if (!isTriggeredInternal) {
102+
this.memoryActionLock.lock();
103+
if (this.isBeingArbitrated.get()) {
104+
this.condition.await();
105+
}
104106
}
105107
LOG.info("[{}] starts to releaseSelf because of {}", this, reason);
106108
this.isClosed = true;
107-
// update father
108-
Optional.ofNullable(this.parent).ifPresent(parent -> parent.gcChildPool(this, false));
109+
// gc self from father
110+
Optional.ofNullable(this.parent).ifPresent(parent -> parent.gcChildPool(this, false,
111+
isTriggeredInternal));
112+
// gc all children
109113
for (MemoryPool child : this.children) {
110-
gcChildPool(child, true);
114+
gcChildPool(child, true, isTriggeredInternal);
111115
}
112116
LOG.info("[{}] finishes to releaseSelf", this);
113117
} catch (InterruptedException e) {
114118
LOG.error("Failed to release self because ", e);
115119
Thread.currentThread().interrupt();
116120
} finally {
117-
this.memoryActionLock.unlock();
121+
if (!isTriggeredInternal) {
122+
this.memoryActionLock.unlock();
123+
}
118124
// Make these objs be GCed by JVM quickly.
119125
this.parent = null;
120126
this.children.clear();
121127
}
122128
}
123129

124130
@Override
125-
public void gcChildPool(MemoryPool child, boolean force) {
131+
public void gcChildPool(MemoryPool child, boolean force, boolean isTriggeredInternal) {
126132
if (force) {
127-
child.releaseSelf(String.format("[%s] releaseChildPool", this));
133+
child.releaseSelf(String.format("[%s] releaseChildPool", this), isTriggeredInternal);
128134
}
129135
// reclaim child's memory and update stats
130136
this.stats.setAllocatedBytes(

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

+16-3
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,22 @@ public interface MemoryPool {
3434

3535
Object tryToAcquireMemoryInternal(long bytes);
3636

37-
void releaseSelf(String reason);
38-
39-
void gcChildPool(MemoryPool child, boolean force);
37+
/**
38+
* Release all self's resources. Called by user or called automatically by itself when OOM.
39+
*
40+
* @param reason: release reason, for logging.
41+
* @param isTriggeredInternal: if true, it is called automatically. if false, called by user.
42+
*/
43+
void releaseSelf(String reason, boolean isTriggeredInternal);
44+
45+
/**
46+
* Called by `releaseSelf` to release children's resource.
47+
*
48+
* @param child: child pool
49+
* @param force: if false, called to gc self from father
50+
* @param isTriggeredInternal: passed from upper caller `releaseSelf`
51+
*/
52+
void gcChildPool(MemoryPool child, boolean force, boolean isTriggeredInternal);
4053

4154
long getAllocatedBytes();
4255

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ public void bindMemoryConsumer(MemoryConsumer memoryConsumer) {
5454
}
5555

5656
@Override
57-
public void releaseSelf(String reason) {
58-
super.releaseSelf(reason);
57+
public void releaseSelf(String reason, boolean isTriggeredInternal) {
58+
super.releaseSelf(reason, isTriggeredInternal);
5959
// since it is already closed, its stats will not be updated. so here we can use its
6060
// stats out of memoryActionLock.
6161
this.memoryAllocator.returnMemoryToManager(getAllocatedBytes());
@@ -125,7 +125,7 @@ public Object requireMemory(long bytes) {
125125
LOG.warn("[{}] detected an OOM exception when request memory, will ABORT this " +
126126
"query and release corresponding memory...",
127127
this);
128-
findRootQueryPool().releaseSelf(String.format(e.getMessage()));
128+
findRootQueryPool().releaseSelf(String.format(e.getMessage()), true);
129129
return null;
130130
} finally {
131131
this.memoryActionLock.unlock();

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ public TaskMemoryPool(MemoryPool parent, String poolName, MemoryManager memoryMa
3737
}
3838

3939
@Override
40-
public synchronized void releaseSelf(String reason) {
40+
public void releaseSelf(String reason, boolean isTriggeredInternal) {
41+
super.releaseSelf(reason, isTriggeredInternal);
4142
this.memoryManager.removeCorrespondingTaskMemoryPool(Thread.currentThread().getName());
42-
super.releaseSelf(reason);
4343
}
4444

4545
@Override

hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/memory/MemoryManageTest.java

+15
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,19 @@ public void testOOM() {
114114
ByteBuf memoryBlock = (ByteBuf) query2Task1Operator1MemoryPool.requireMemory(requireBytes);
115115
Assert.assertNull(memoryBlock);
116116
}
117+
118+
@Test
119+
public void testReleaseMemory() {
120+
121+
}
122+
123+
@Test
124+
public void testLocalArbitration() {
125+
126+
}
127+
128+
@Test
129+
public void testGlobalArbitration() {
130+
131+
}
117132
}

0 commit comments

Comments
 (0)