Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEODE-7341: Provide a way to avoid memory lock if over committed #4210

Merged
merged 5 commits into from Oct 30, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -135,11 +135,6 @@
public class InternalDistributedSystem extends DistributedSystem
implements LogConfigSupplier {

/**
* True if the user is allowed lock when memory resources appear to be overcommitted.
*/
private static final boolean ALLOW_MEMORY_LOCK_WHEN_OVERCOMMITTED =
Boolean.getBoolean(GEMFIRE_PREFIX + "Cache.ALLOW_MEMORY_OVERCOMMIT");
private static final Logger logger = LogService.getLogger();

private static final String DISABLE_MANAGEMENT_PROPERTY =
Expand Down Expand Up @@ -172,6 +167,16 @@ public class InternalDistributedSystem extends DistributedSystem

private final StatisticsManager statisticsManager;
private final FunctionStatsManager functionStatsManager;
/**
* True if the user is allowed lock when memory resources appear to be overcommitted.
*/
private final boolean allowMemoryLockWhenOvercommitted =
Boolean.getBoolean(GEMFIRE_PREFIX + "Cache.ALLOW_MEMORY_OVERCOMMIT");
/**
* True if memory lock is avoided when memory resources appear to be overcommitted.
*/
private final boolean avoidMemoryLockWhenOvercommitted =
pivotal-eshu marked this conversation as resolved.
Show resolved Hide resolved
Boolean.getBoolean(GEMFIRE_PREFIX + "Cache.AVOID_MEMORY_LOCK_WHEN_OVERCOMMIT");

/**
* The distribution manager that is used to communicate with the distributed system.
Expand Down Expand Up @@ -724,22 +729,7 @@ private void initialize(SecurityManager securityManager, PostProcessor postProce
// included the available memory calculation.
long avail = LinuxProcFsStatistics.getAvailableMemory(logger);
long size = offHeapMemorySize + Runtime.getRuntime().totalMemory();
if (avail < size) {
if (ALLOW_MEMORY_LOCK_WHEN_OVERCOMMITTED) {
logger.warn(
"System memory appears to be over committed by {} bytes. You may experience instability, performance issues, or terminated processes due to the Linux OOM killer.",
size - avail);
} else {
throw new IllegalStateException(
String.format(
"Insufficient free memory (%s) when attempting to lock %s bytes. Either reduce the amount of heap or off-heap memory requested or free up additional system memory. You may also specify -Dgemfire.Cache.ALLOW_MEMORY_OVERCOMMIT=true on the command-line to override the constraint check.",
avail, size));
}
}

logger.info("Locking memory. This may take a while...");
GemFireCacheImpl.lockMemory();
logger.info("Finished locking memory.");
lockMemory(avail, size);
}

try {
Expand Down Expand Up @@ -817,6 +807,34 @@ private void initialize(SecurityManager securityManager, PostProcessor postProce
reconnectAttemptCounter.set(0);
}

void lockMemory(long avail, long size) {
if (avail < size) {
if (avoidMemoryLockWhenOvercommitted) {
logger.warn(
"System memory appears to be over committed by {} bytes. Avoid memory lock.",
pivotal-eshu marked this conversation as resolved.
Show resolved Hide resolved
pivotal-eshu marked this conversation as resolved.
Show resolved Hide resolved
size - avail);
} else if (allowMemoryLockWhenOvercommitted) {
logger.warn(
"System memory appears to be over committed by {} bytes. You may experience instability, performance issues, or terminated processes due to the Linux OOM killer.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add to this middle message that because of ALLOW_MEMORY_OVERCOMMIT being true it will be locked even though it appears to be over committed. In my previous review I suggested a change. I think this is helpful because the reader of the message may not be the same user that set this sys prop to true and may not realize that they can just set it to false to prevent this warning.

size - avail);
lockMemory();
} else {
throw new IllegalStateException(
String.format(
"Insufficient free memory (%s) when attempting to lock %s bytes. Either reduce the amount of heap or off-heap memory requested or free up additional system memory. You may also specify -Dgemfire.Cache.ALLOW_MEMORY_OVERCOMMIT=true on the command-line to override the constraint check.",
pivotal-eshu marked this conversation as resolved.
Show resolved Hide resolved
avail, size));
}
} else {
lockMemory();
}
}

void lockMemory() {
logger.info("Locking memory. This may take a while...");
pivotal-eshu marked this conversation as resolved.
Show resolved Hide resolved
GemFireCacheImpl.lockMemory();
logger.info("Finished locking memory.");
}

private void startSampler() {
if (statsDisabled) {
return;
Expand Down
@@ -0,0 +1,95 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more contributor license
* agreements. See the NOTICE file distributed with this work for additional information regarding
* copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License. You may obtain a
* copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package org.apache.geode.distributed.internal;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchThrowable;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;

import java.util.Properties;

import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;


public class InternalDistributedSystemTest {

@Rule
public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();

@Test
public void lockMemoryAllowedIfAllowMemoryOverCommitIsSet() {
System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "Cache.ALLOW_MEMORY_OVERCOMMIT", "true");
InternalDistributedSystem system =
spy(new InternalDistributedSystem.Builder(new Properties()).build());
doNothing().when(system).lockMemory();

system.lockMemory(100, 200);

verify(system).lockMemory();
}

@Test
public void lockMemoryAvoidedIfAvoidMemoryLockWhenOverCommitIsSet() {
System.setProperty(
DistributionConfig.GEMFIRE_PREFIX + "Cache.AVOID_MEMORY_LOCK_WHEN_OVERCOMMIT", "true");
InternalDistributedSystem system =
spy(new InternalDistributedSystem.Builder(new Properties()).build());

system.lockMemory(100, 200);

verify(system, never()).lockMemory();
}

@Test
public void lockMemoryAvoidedIfAvoidAndAllowMemoryLockWhenOverCommitBothSet() {
System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "Cache.ALLOW_MEMORY_OVERCOMMIT", "true");
System.setProperty(
DistributionConfig.GEMFIRE_PREFIX + "Cache.AVOID_MEMORY_LOCK_WHEN_OVERCOMMIT", "true");
InternalDistributedSystem system =
spy(new InternalDistributedSystem.Builder(new Properties()).build());

system.lockMemory(100, 200);

verify(system, never()).lockMemory();
}


@Test
public void lockMemoryThrowsIfMemoryOverCommit() {
pivotal-eshu marked this conversation as resolved.
Show resolved Hide resolved
InternalDistributedSystem system =
spy(new InternalDistributedSystem.Builder(new Properties()).build());

Throwable caughtException = catchThrowable(() -> system.lockMemory(100, 200));

assertThat(caughtException).isInstanceOf(IllegalStateException.class);
verify(system, never()).lockMemory();
}

@Test
public void locksMemoryIfMemoryNotOverCommit() {
InternalDistributedSystem system =
spy(new InternalDistributedSystem.Builder(new Properties()).build());
doNothing().when(system).lockMemory();

system.lockMemory(200, 100);

verify(system).lockMemory();
}
}