tillrohrmann commented on a change in pull request #11025:
[FLINK-15919][core][mem] MemoryManager shouldn't allow releasing more memory
than reserved
URL: https://github.com/apache/flink/pull/11025#discussion_r375254671
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/memory/MemoryManager.java
##########
@@ -501,17 +503,21 @@ public void releaseMemory(Object owner, MemoryType
memoryType, long size) {
size,
currentlyReserved == null ? 0 : currentlyReserved,
owner);
+ if (currentlyReserved
!= null) {
+
releasedSize.set(currentlyReserved);
+ }
//noinspection
ReturnOfNull
return null;
} else {
+ releasedSize.set(size);
return
currentlyReserved - size;
}
});
}
//noinspection ReturnOfNull
return reservations == null || reservations.isEmpty() ?
null : reservations;
});
- budgetByType.releaseBudgetForKey(memoryType, size);
+ budgetByType.releaseBudgetForKey(memoryType,
releasedSize.get());
Review comment:
I'd suggest to change this block into:
```
public void releaseMemory(Object owner, MemoryType memoryType, long size) {
checkMemoryReservationPreconditions(owner, memoryType, size);
if (size == 0L) {
return;
}
reservedMemory.compute(owner, (o, reservations) -> {
if (reservations != null) {
reservations.compute(
memoryType,
(mt, currentlyReserved) -> {
long newReservedMemory = 0;
if (currentlyReserved != null) {
if (currentlyReserved < size) {
LOG.warn(
"Trying to
release more memory {} than it was reserved {} so far for the owner {}",
size,
currentlyReserved,
owner);
}
newReservedMemory =
releaseAndCalculateReservedMemory(size, memoryType, currentlyReserved);
}
return newReservedMemory == 0 ? null :
newReservedMemory;
});
}
//noinspection ReturnOfNull
return reservations == null || reservations.isEmpty() ? null :
reservations;
});
}
private long releaseAndCalculateReservedMemory(long memoryToFree, MemoryType
memoryType, long currentlyReserved) {
final long effectiveMemoryToRelease = Math.min(currentlyReserved,
memoryToFree);
budgetByType.releaseBudgetForKey(memoryType, effectiveMemoryToRelease);
return currentlyReserved - effectiveMemoryToRelease;
}
```
----------------------------------------------------------------
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
