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

Limit memory overhead #6822

Merged
merged 2 commits into from
Apr 27, 2016
Merged

Conversation

tylersmalley
Copy link
Contributor

V8 will not trigger a full mark-sweep & mark-compact until there is memory pressure for the max-old-space-size, which is 1.6GB on a 64bit system. This means that Kibana will, at times, consume over 1.6GB of RSS memory.

screenshot 2016-04-07 14 22 56

We will be setting max-old-space-size to 256MB unless it is ran with --dev, in which case we will use the node defaults.

Over-writing the max-old-space-size with NODE_OPTIONS is still possible.

@tylersmalley
Copy link
Contributor Author

I am going to build and deploy this to DigitalOcean to get a definitive idea of what the usage is. But for now, let's discuss the language used in the documentation and the method for setting the default.

@tylersmalley tylersmalley force-pushed the default-node-arguments branch 3 times, most recently from 37fc9b8 to 0888ec7 Compare April 8, 2016 16:23
@@ -22,7 +22,7 @@ If Not Exist "%NODE%" (
)

TITLE Kibana Server
"%NODE%" %NODE_OPTIONS% "%DIR%\src\cli" %*
"%NODE%" --max-old-space-size=256 %NODE_OPTIONS% "%DIR%\src\cli" %*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to update this batch file to reflect the changes made in bin/kibana. Downloading a Windows VM to verify before adding to the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's two ways. I think the first is the more common (but longer);

@REM Loop through any command line parameters and if we find "--dev" add "--max-old-space-size=256"
:parse
IF "%~1"=="" GOTO endparse
IF "%~1"=="--dev" set NODE_OPTIONS="--max-old-space-size=256 %NODE_OPTIONS%"
SHIFT
GOTO parse
:endparse

And a single line way;

@REM If the command line parameters do not contain "--dev" then add "--max-old-space-size=256"
echo "%*" | findstr /V /C:"--dev" && set NODE_OPTION="--max-old-space-size=256 %NODE_OPTION%"

@tylersmalley tylersmalley changed the title Set default GC limit for V8 Limit memory overhead Apr 8, 2016
@tylersmalley
Copy link
Contributor Author

With max-old-space-size=256 the process' RSS is staying under 300M.

screenshot 2016-04-08 09 36 02

@tylersmalley tylersmalley force-pushed the default-node-arguments branch 3 times, most recently from e133e0c to f91d316 Compare April 8, 2016 22:42
@tylersmalley
Copy link
Contributor Author

@LeeDr - would you mind reviewing/testing the changes made to the batch file in this PR?

@Bargs
Copy link
Contributor

Bargs commented Apr 9, 2016

Why remove the limit in dev? Dev should be as prod like as possible, otherwise we're just asking for pain down the road.

@tylersmalley
Copy link
Contributor Author

@Bargs - the optimize process requires much more memory to run and is not used in production, except for plugin installation which goes through bin/kibana-plugin

@Bargs
Copy link
Contributor

Bargs commented Apr 11, 2016

Hmmm, I'm guessing you've seen the optimizer crash with --max-old-space=256? I just started it up with 256 without trouble but it hasn't been running long. The optimizer gets started with cluster.fork so I think it should get its own heap. I thought we might be able to configure max-old-space separately if it's just the optimizer that needs more memory, but unfortunately that doesn't seem possible.

@tylersmalley
Copy link
Contributor Author

This illustrates the memory usage as compared to 5.0.0-alpha1.

screenshot 2016-04-11 08 42 41

@@ -6,6 +6,7 @@ Kibana is an open source ([Apache Licensed](https://github.com/elastic/kibana/bl

- Elasticsearch master
- Kibana binary package
- 512 MB of available RAM
Copy link
Contributor

Choose a reason for hiding this comment

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

512 here? I assume this is because some amount of memory will of course be used that isn't old space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct, it's just a recommendation. We could probably run on lower, but we do have things which are not old space.


=== Memory management

Kibana is built on Node.js which doesn't tune its heap size based on the amount of memory available. To combat this, we set defaults based on the requirements of Kibana needs, while allowing overhead for additional plugins. These defaults can be overridden at runtime, for example `NODE_OPTIONS="--max-old-space-size=512" bin/kibana`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rashidkpc, thoughts this verbiage?

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@rashidkpc
Copy link
Contributor

In total: LGTM

Tyler Smalley added 2 commits April 26, 2016 13:44
V8 will not trigger a full mark-sweep & mark-compact until there is memory pressure for the max-old-space-size, which is 1.6GB on a 64bit system. This means that Kibana will, at times, consume over 1.6GB of RSS memory.

Bypassing this can be done by starting with `--dev` or explicitally setting `max-old-space-size`. For example: `NODE_OPTIONS="--max-old-space-size=1024" ./bin/kibana`

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@LeeDr
Copy link
Contributor

LeeDr commented Apr 26, 2016

LGTM
I re-tested on Windows with no NODE_OPTIONS setting, and memory seemed to stay around 256M or lower.

With --dev and no NODE_OPTIONS, it went to about 614M while running functional tests.

With --max-old-space-size=50 it crashes with and without --dev.

With --max-old-space-size=5000 without --dev I saw the memory get to about 1G but then dropped down to around 500M while the functional tests were running.

@tylersmalley
Copy link
Contributor Author

@epixa - what do you think about getting this into alpha2? It's ready to merge, but I can hold off if you would like to wait for beta1.

I also did further testing on Linux 32bit today.

@epixa
Copy link
Contributor

epixa commented Apr 27, 2016

Yep, let's do it.

@epixa epixa merged commit 855faff into elastic:master Apr 27, 2016
@tylersmalley tylersmalley deleted the default-node-arguments branch April 27, 2016 15:18
@epixa
Copy link
Contributor

epixa commented Apr 28, 2016

I found a pretty big problem with this:

Optimizing does happen on the server in production in at least one scenario: If you remove a plugin, the next time you start up the server, the optimizer is run on the remaining plugins. I just removed x-pack on an alpha2 build and now I can't start kibana with the default settings because the optimizer makes it crash due to out-of-memory.

@epixa epixa added the reverted label Oct 26, 2016
cee-chen added a commit that referenced this pull request Jun 6, 2023
## Summary

`eui@81.2.0` ⏩ `eui@81.3.0`

✨ Highlights:

- fixes #158644
- Adds a new Timeline icon for use within `EuiMarkdownEditor` (cc
@kqualters-elastic)
- Expandable rows used within `EuiBasicTable` and `EuiInMemoryTable` now
have a faster and snappier expand animation

___

## [`81.3.0`](https://github.com/elastic/eui/tree/v81.3.0)

- Added `timelineWithArrow` glyph to `EuiIcon`
([#6822](elastic/eui#6822))

**Bug fixes**

- Fixed `EuiCodeBlock` potentially incorrectly ignoring lines ending
with a question mark when using the Copy button.
([#6794](elastic/eui#6794))
- Fixed `EuiCodeBlock` to not include line numbers when copying content
([#6824](elastic/eui#6824))
- Fixed the expanded row animation on `EuiBasicTable` causing
cross-browser Safari issues
([#6826](elastic/eui#6826))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants