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

Conversation

pivotal-eshu
Copy link
Contributor

No description provided.

Copy link
Contributor

@jhuynh1 jhuynh1 left a comment

Choose a reason for hiding this comment

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

Do these tests need to reset the system properties back to their original values?

It looks like this now allows us to no longer get an exception but rather continue to process without locking memory. What would be the side effects of allowing this to happen? Will it just error out at a later operation?

@pivotal-eshu
Copy link
Contributor Author

Tests use RestoreSystemProperties rule to reset the system properties.

This should not cause side effects if not lock memory using the new system property introduced, compared to the unknown side effect if using the original ALLOW_MEMORY_OVERCOMMIT system property setting.

gesterzhou
gesterzhou previously approved these changes Oct 29, 2019
"System memory appears to be over committed by {} bytes. Avoid memory lock.",
size - avail);
"System memory appears to be over committed by {} bytes. So memory will not be locked because -D{} is set to true.",
size - avail, AVOID_MEMORY_LOCK_WHEN_OVERCOMMIT);
} 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.

@pivotal-eshu pivotal-eshu merged commit 52c9d9d into develop Oct 30, 2019
echobravopapa pushed a commit to echobravopapa/geode that referenced this pull request Oct 30, 2019
@pivotal-eshu pivotal-eshu deleted the feature/GEODE-7341 branch October 30, 2019 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants