[node] Bump node.js to 4.4.3 #6966
Conversation
This includes a couple of different memory leak and security fixes. npm is bumped to the same version that comes bundled with this version of node rather than the latest possible npm available.
|
@tylersmalley Do you still have that VM set up that you were using to do memory tests? |
|
Hey @epixa - yeah I can throw this on a VM and monitor. What memory leaks are you referring to which this looks to resolve? I do like bumping Node to LTS. |
|
The memory leak was related to ssl and keepalive: nodejs/node#5699 I don't know for sure whether we were encountering it, but given the troubles we've had, I'd prefer to keep on top of the updates. |
|
Yeah, I don't think we are encountering that issue after that last version bump. But I agree, let's get to LTS. I will let you know when the build is up. I would like to have it run for a few days to ensure we are not introducing another issue. Is that timeline ok? |
|
I would also be up for merging while continuing to monitor the VM's memory usage. |
|
I certainly don't mind waiting. Let's check back on Wednesday? |
|
Sounds good. Will report back then. |
|
I still need time to investigate, but wanted to report that the instance crashed due to OOM. Process RSS was reporting almost 1.9GB used at time. I am running the branch with SSL enabled on Kibana and between ES 5.0.0-alpha1. It's on a 2GB VM dedicated to Kibana.
|
|
Interesting... Just to be certain, can you run the same test (with ssl and such) on the parent commit? 7d08096 |
|
To be clear: I want to check the memory usage on one commit removed from this commit, which will be using node 4.3.2. Based on that visualization, we should be able to tell within a few hours whether memory is trending upwards. |
|
I am getting the previous commit up on a VM. Memory will always trend upwards, but I would expect the GC to finally compact when it encroaches on the max-old-space-size. I will set to 256 on this install to see a result quicker. |
|
It appears that the memory usage is simply much higher.
Before 14:00 being the parent commit, and after being this branch. Same exact config and setting |
|
Going to disable SSL and see if it's related. |
|
Confirmed this is only an issue when SSL is enabled between ElasticSearch. |
|
Interesting... On one hand, there's no specific reason to bump node right now. On the other hand, there definitely will be reasons to bump node in the future, so if node is going to start consuming more memory, maybe a major version is the best time to make that jump rather than some patch version in the future. |
|
If we do, we should include at the same time as #6822. Otherwise users can't run on a 2GB VM. |
|
Great catch. I'm still not sure how I want to proceed with this PR, but I'll mark it as blocked for now so we don't get trigger-happy. |
|
@tylersmalley Total shot in the dark here, but if your VM isn't in use at the moment, could you test out that commit only with node 5.11.0 instead? With 6.0 around the corner, this should give us a clearer picture of whether we should expect memory usage to increase permanently in node or if it's just a 4.4 thing. |
|
@epixa, yes will do and report back. |
|
5.11.0 showed the same memory issues as 4.4.3. I tested 6.0, but it requires updating all packages which depend on graceful-fs. I believe the memory growth is related to how we pool SSL connections in elasticsearch-js, so I will investigate that as a possible cause. |
|
Closing in favor of #7148 |
`85.0.0` ➡️ `85.1.0`⚠️ The biggest change in this PR by far is the **removal** of several `EuiFilterSelectItem` CSS classes and styles associated with those classes. EUI has previously exported component-specific CSS classes for usage such as `.euiFilterSelect__items`, `.euiFilterGroup__popoverPanel`, or `.euiAccordionForm`. ❌ **As of our move to CSS-in-JS, we are actively moving away from this architecture**. The goal is to either provide either a component or prop directly to you instead of a CSS class. As a reminder, our classNames are not guaranteed APIs and are subject to change without warning - should you need to tweak or customize EUI's styling, we strongly recommend passing in your own `className`. 💬 Moving forward, EUI will attempted to convert any usages of removed CSS classes and their direct usages in Kibana for you. In most cases, we'll hopefully be able to take the correct path of using a component or prop instead. In cases where we cannot or significant/complex changes are required, we may temporarily convert the CSS to a static CSS-in-JS usage instead and add a TODO asking the relevant team to address this in their own time (for example, the deprecation of `EuiFilterSelectItem` and its conversion to `EuiSelectable`). ## [`85.1.0`](https://github.com/elastic/eui/tree/v85.1.0) - Updated `EuiComboBox`'s `options` to accept `option.append` and `option.prepend` props ([#6953](elastic/eui#6953)) - Updated deprecated `.substr()` usages to `.substring()` ([#6954](elastic/eui#6954)) - Updated `EuiInlineEdit`'s read mode button to include a title tooltip, increasing readability of truncated text ([#6966](elastic/eui#6966)) **Bug fixes** - Fixed `EuiFilterGroup`'s responsive styles ([#6983](elastic/eui#6983)) **Deprecations** - Deprecated `EuiFilterSelectItem`; Use `EuiSelectable` instead ([#6982](elastic/eui#6982)) **CSS-in-JS conversions** - Converted `EuiFilterSelectItem` to Emotion ([#6982](elastic/eui#6982)) - Removed `.euiFilterSelect__items` CSS; Use `EuiSelectable` instead ([#6982](elastic/eui#6982)) - Removed `.euiFilterSelect__note` and `.euiFilterSelect__noteContent` CSS; Use `EuiSelectableMessage` instead ([#6982](elastic/eui#6982)) - Added `focus.transparency` and `focus.backgroundColor` theme tokens ([#6984](elastic/eui#6984)) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Should not be merged until #6822 is merged.
This includes a couple of different memory leak and security fixes.
npm is bumped to the same version that comes bundled with this version
of node rather than the latest possible npm available.