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

Support VirtualLock on Windows #8480

Closed
clintongormley opened this Issue Nov 14, 2014 · 14 comments

Comments

Projects
None yet
7 participants
@clintongormley
Copy link
Member

clintongormley commented Nov 14, 2014

On *nix boxes, we have the mlockall option, but that isn't supported by Windows. Instead, Windows has VirtualLock

Could/should we support this option? Could we make the bootstrap.mlockall setting use VirtualLock on Windows boxes?

@clintongormley

This comment has been minimized.

Copy link
Member Author

clintongormley commented Nov 14, 2014

@costin @gmarz @Mpdreamz any ideas about this?

@Mpdreamz

This comment has been minimized.

Copy link
Member

Mpdreamz commented Nov 14, 2014

@gmarz

This comment has been minimized.

Copy link
Member

gmarz commented Nov 14, 2014

@clintongormley I've wondered this myself. I knew VirtualLock existed, but have never done anything with it. I'll start looking into it.

@gmarz

This comment has been minimized.

Copy link
Member

gmarz commented Dec 22, 2014

So after looking into this a bit, it does look possible to leverage VirtualLock. However, it's important to note that VirtualLock works a bit more like mlock than it does mlockall, and thus is more tedious to implement as it requires more information and setup upfront (size of region to lock, memory needs to be allocated, etc). Unfortunately, there is no equivalent VirtualLockAll in Windows.

In short, there are two key steps that need to be taken before calling VirtualLock:

  • Increase working set size of the process using SetProcessWorkingSetSize. By default, Windows limits the working set size of a process to ~30 pages.

This limit is intentionally small to avoid severe performance degradation. Applications that need to lock larger numbers of pages must first call the SetProcessWorkingSetSize function to increase their minimum and maximum working set sizes.

I've started implementing this in my own branch, and so far the results look good. It'll probably be much easier to discuss this once we have some code to look at, so I'll open a PR once I'm confident with the changes and we can take it from there.

@clintongormley

This comment has been minimized.

Copy link
Member Author

clintongormley commented Dec 22, 2014

nice work @gmarz

@gmarz

This comment has been minimized.

Copy link
Member

gmarz commented Jan 7, 2015

Need to make a slight amendment to my previous comment above.

After digging a bit further, I've learned that there is no need for VirtualAlloc. It's only useful for allocating and locking new memory, where in this case, we need to lock the current memory already allocated by the JVM (Xms).

The only way I've found this possible is to walk the JVM address space using VirtualQueryEx and lock each page individually.

gmarz added a commit to gmarz/elasticsearch that referenced this issue Jan 23, 2015

gmarz added a commit to gmarz/elasticsearch that referenced this issue Apr 29, 2015

gmarz added a commit to gmarz/elasticsearch that referenced this issue May 1, 2015

gmarz added a commit to gmarz/elasticsearch that referenced this issue May 8, 2015

@gmarz gmarz closed this in #10887 May 8, 2015

@kevinkluge kevinkluge removed the discuss label May 8, 2015

gmarz added a commit that referenced this issue May 8, 2015

VirtualLock implementation for Windows (mlockall equivalent)
Closes #8480

Conflicts:
	src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented May 11, 2015

I reverted both 1.x and master commits since tests are failing

@s1monw s1monw reopened this May 11, 2015

@rmuir

This comment has been minimized.

Copy link
Contributor

rmuir commented May 11, 2015

I think its just a silly test bug? The failing test asserts that mlockall didn't succeed on windows, but now it can since its supported:

public void testMlockall() {
    if (Constants.WINDOWS) {
        assertFalse("Memory locking is not available on Windows platforms", Natives.LOCAL_MLOCKALL);
    }
    ...
@gmarz

This comment has been minimized.

Copy link
Member

gmarz commented May 11, 2015

@rmuir Yup. I'm fixing this now.

gmarz added a commit to gmarz/elasticsearch that referenced this issue May 11, 2015

@rmuir

This comment has been minimized.

Copy link
Contributor

rmuir commented May 11, 2015

I think we should just remove that whole assert.

@rmuir

This comment has been minimized.

Copy link
Contributor

rmuir commented May 11, 2015

+1, my browser was a bit out of date.

@gmarz

This comment has been minimized.

Copy link
Member

gmarz commented May 11, 2015

I've put back the change and fixed the tests in 1.x and master.

@gmarz gmarz closed this May 11, 2015

@javadevmtl

This comment has been minimized.

Copy link

javadevmtl commented Jun 8, 2015

I would gladly love to test this. Currently getting 100% usage in Windows with mapped files :)
I have 128GB server and twice made it up to 100% mem usage and unresponssive ES node.

Some details here: https://discuss.elastic.co/t/100-windows-memory-usage-but-no-oom/2089/4
And here: http://serverfault.com/questions/697511/setting-memory-mapped-file-limit-for-windows

@gmarz

This comment has been minimized.

Copy link
Member

gmarz commented Jun 9, 2015

Hey @javadevmtl this is available in 1.6, which we just released today.

Would be great if you could test this and provide any feedback. Really interested in seeing what effect it has on the issue your experiencing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.