JackieTien97 commented on code in PR #16932:
URL: https://github.com/apache/iotdb/pull/16932#discussion_r2634206864
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/memory/NotThreadSafeMemoryReservationManager.java:
##########
@@ -91,4 +93,25 @@ public void releaseAllReservedMemory() {
bytesToBeReleased = 0;
}
}
+
+ @Override
+ public Pair<Long, Long> releaseMemoryVirtually(final long size) {
+ if (bytesToBeReserved >= size) {
+ bytesToBeReserved -= size;
+ return new Pair<>(size, 0L);
+ } else {
+ long releasedBytesInReserved = bytesToBeReserved;
+ long releasedBytesInTotal = size - bytesToBeReserved;
+ bytesToBeReserved = 0;
+ reservedBytesInTotal -= releasedBytesInTotal;
+ return new Pair<>(releasedBytesInReserved, releasedBytesInTotal);
+ }
+ }
+
+ @Override
+ public void reserveMemoryVirtually(
+ final long releasedBytesInReserved, final long releasedBytesInTotal) {
+ bytesToBeReserved += releasedBytesInReserved;
+ reservedBytesInTotal += releasedBytesInTotal;
Review Comment:
```suggestion
reservedBytesInTotal += releasedBytesInTotal;
reserveMemoryCumulatively(releasedBytesInReserved);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/memory/MemoryReservationManager.java:
##########
@@ -43,4 +45,23 @@ public interface MemoryReservationManager {
* this manager ends, Or the memory to be released in the batch may not be
released correctly.
*/
void releaseAllReservedMemory();
+
+ /**
+ * Release memory virtually without actually freeing the memory. This is
used for memory
+ * reservation transfer scenarios where memory ownership needs to be
transferred between different
+ * FragmentInstances without actual memory deallocation.
+ *
+ * <p>NOTE: When calling this method, it should be guaranteed that
bytesToBeReserved +
+ * reservedBytesInTotal >= size to ensure proper memory accounting and
prevent negative
+ * reservation values.
+ */
+ Pair<Long, Long> releaseMemoryVirtually(final long size);
+
+ /**
+ * Reserve memory virtually without actually allocating new memory. This is
used to transfer
+ * memory ownership from one FragmentInstances to another by reserving the
memory that was
+ * previously released virtually. It updates the internal reservation state
without changing the
+ * actual memory allocation.
+ */
+ void reserveMemoryVirtually(final long releasedBytesInReserved, final long
releasedBytesInTotal);
Review Comment:
```suggestion
void reserveMemoryVirtually(final long bytesToBeReserved, final long
bytesAlreadyReserved);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/memory/FakedMemoryReservationManager.java:
##########
@@ -32,4 +34,13 @@ public void releaseMemoryCumulatively(long size) {}
@Override
public void releaseAllReservedMemory() {}
+
+ @Override
+ public Pair<Long, Long> releaseMemoryVirtually(final long size) {
+ return null;
Review Comment:
return new Pair<>(0, 0)? Otherwise, it may cause NPE.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/memory/MemoryReservationManager.java:
##########
@@ -43,4 +45,23 @@ public interface MemoryReservationManager {
* this manager ends, Or the memory to be released in the batch may not be
released correctly.
*/
void releaseAllReservedMemory();
+
+ /**
+ * Release memory virtually without actually freeing the memory. This is
used for memory
+ * reservation transfer scenarios where memory ownership needs to be
transferred between different
+ * FragmentInstances without actual memory deallocation.
+ *
+ * <p>NOTE: When calling this method, it should be guaranteed that
bytesToBeReserved +
+ * reservedBytesInTotal >= size to ensure proper memory accounting and
prevent negative
+ * reservation values.
+ */
+ Pair<Long, Long> releaseMemoryVirtually(final long size);
Review Comment:
add some java doc about the return value
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/memory/MemoryReservationManager.java:
##########
@@ -43,4 +45,23 @@ public interface MemoryReservationManager {
* this manager ends, Or the memory to be released in the batch may not be
released correctly.
*/
void releaseAllReservedMemory();
+
+ /**
+ * Release memory virtually without actually freeing the memory. This is
used for memory
+ * reservation transfer scenarios where memory ownership needs to be
transferred between different
+ * FragmentInstances without actual memory deallocation.
+ *
+ * <p>NOTE: When calling this method, it should be guaranteed that
bytesToBeReserved +
+ * reservedBytesInTotal >= size to ensure proper memory accounting and
prevent negative
+ * reservation values.
+ */
+ Pair<Long, Long> releaseMemoryVirtually(final long size);
+
+ /**
+ * Reserve memory virtually without actually allocating new memory. This is
used to transfer
+ * memory ownership from one FragmentInstances to another by reserving the
memory that was
+ * previously released virtually. It updates the internal reservation state
without changing the
+ * actual memory allocation.
+ */
+ void reserveMemoryVirtually(final long releasedBytesInReserved, final long
releasedBytesInTotal);
Review Comment:
add some java doc about these two parameters
--
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]
