cshannon commented on code in PR #3151:
URL: https://github.com/apache/accumulo/pull/3151#discussion_r1064018807
##########
core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java:
##########
@@ -84,6 +95,107 @@
public class RFile {
+ private static class RFileMemoryProtection implements NotificationListener {
+
+ private static class FreeMemoryUpdater implements Runnable {
+
+ private final ReentrantLock lock = new ReentrantLock();
+ private final AtomicReference<CountDownLatch> latchRef =
+ new AtomicReference<>(new CountDownLatch(1));
+
+ public void update() {
+ lock.lock();
+ try {
+ latchRef.get().countDown();
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ @Override
+ public void run() {
+ try {
+ while (true) {
+ CountDownLatch latch = latchRef.get();
+ // TODO: Could use latch.await(long, TimeUnit) to update free
memory
+ // when GC does not occur. It's probable that memory allocations
+ // will occur without GC happening, depending on the memory pool
+ // sizes.
+ latch.await();
+ // acquiring a lock here so that we don't miss a call to update()
while
+ // we are getting the current free memory
+ lock.lock();
+ try {
+ FREE_MEMORY.set(Runtime.getRuntime().freeMemory());
Review Comment:
I did a little more research into this topic today and I found some
interesting discussion here:
https://stackoverflow.com/questions/12807797/java-get-available-memory
The most intriguing part to me is that we should not try and use
`Runtime.getRuntime().freeMemory()` but instead to use
`Runtime.getRuntime().maxMemory() - allocatedMemory` where `allocatedMemory` is
`Runtime.getRuntime().totalMemory() - runtime.freeMemory()`
The reasoning being if you use the `-Xmx` property then memory is allocated
in chunks so using `Runtime.getRuntime().freeMemory()` may not actually return
all the free memory.
Regardless, no matter what it is used, it's going to just be an estimation
plus there's the race condition issue so definitely keeping a buffer size to
account for the estimation error, etc that you added is a good idea.
##########
core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java:
##########
@@ -84,6 +95,107 @@
public class RFile {
+ private static class RFileMemoryProtection implements NotificationListener {
+
+ private static class FreeMemoryUpdater implements Runnable {
+
+ private final ReentrantLock lock = new ReentrantLock();
+ private final AtomicReference<CountDownLatch> latchRef =
+ new AtomicReference<>(new CountDownLatch(1));
+
+ public void update() {
+ lock.lock();
+ try {
+ latchRef.get().countDown();
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ @Override
+ public void run() {
+ try {
+ while (true) {
+ CountDownLatch latch = latchRef.get();
+ // TODO: Could use latch.await(long, TimeUnit) to update free
memory
+ // when GC does not occur. It's probable that memory allocations
+ // will occur without GC happening, depending on the memory pool
+ // sizes.
+ latch.await();
+ // acquiring a lock here so that we don't miss a call to update()
while
+ // we are getting the current free memory
+ lock.lock();
+ try {
+ FREE_MEMORY.set(Runtime.getRuntime().freeMemory());
Review Comment:
```suggestion
// Using only runtime.freeMemory() here would give you memory
that is definitely
// available but may not be the amount of memory before an OME
occurs which
// is really the goal. This is due to the fact that
freeMemory() will only
// return the available memory from the currently allocated
memory but the JVM may be
// able to allocate more memory if the -Xmx parameter is used
so the true estimated
// available free memory is actually (maxMemory -
allocatedMemory) where
// allocatedMemory is (totalMemory - freeMemory).
// See
https://stackoverflow.com/questions/12807797/java-get-available-memory
// for more detailed discussion
final Runtime runtime = Runtime.getRuntime();
long allocatedMemory = runtime.totalMemory() -
runtime.freeMemory();
FREE_MEMORY.set(runtime.maxMemory() - allocatedMemory);
```
--
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]
