Memory leak/CPU issue traced to this change... Why?

help
1

Ran some updates on our codebase and our Galaxy containers all went down - 100% CPU usage on all containers and memory steadily climbing to 100% before crashing.

Traced the problem down to this change and I cannot, for the life of me, figure out why. It seems like the most straightforward change:

// index.js - old code

/* eslint-disable react/jsx-filename-extension */
import React from 'react';
import { render } from 'react-dom';
import { Meteor } from 'meteor/meteor';
import { Router } from 'react-router-dom';
import { createBrowserHistory } from 'history';
import ContextProvider from '../../ui/components/Context';
import App from '../../ui/layouts/App';
import ErrorHandler from '../../ui/components/ErrorHandler';

const history = createBrowserHistory();

Meteor.startup(() => {
	render(
		<ErrorHandler>
			<Router history={history}>
				<ContextProvider>
					<App />
				</ContextProvider>
			</Router>
		</ErrorHandler>,
		document.getElementById('app')
	);
});



// index.tsx - new code

import { createBrowserHistory } from 'history';
import { Meteor } from 'meteor/meteor';
import React from 'react';
import { createRoot } from 'react-dom/client';
import { Router } from 'react-router-dom';
import ContextProvider from '../../ui/components/Context';
import ErrorHandler from '../../ui/components/ErrorHandler';
import App from '../../ui/layouts/App';

const history = createBrowserHistory();

const container = document.getElementById('app');
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const root = createRoot(container!);

Meteor.startup(() => {
	root.render(
		<ErrorHandler>
			<Router history={history}>
				<ContextProvider>
					<App />
				</ContextProvider>
			</Router>
		</ErrorHandler>
	);
});

Can anyone see what footgun I am shooting myself with? Another thing that is weird is that this is client side code (mounting the React tree) - why is it having an impact on the servers?

2

I have almost no experience with React. But as far as I know a recent thing in React is server components that are used for SSR. So perhaps something like this is being set up on the server. SSR and memory leaks usually go hand in hand :slight_smile:

I would start my seeing if that file, index.tsx, is loaded only on the client or on the server as well.

3

That’s odd. What version of React are you using?
Were you able to revert/rollback to the previous version of the app for now?

4

So this was a step in upgrading from react 16.8 to the latest one - 18.2

But React 18.2 works fine other than this file*. The documentation says that it’s running React 17 until this root change is applied - so I guess it may be that React 18 is an issue and React 17 is not?

*What I mean by this is that the project has React 18.2 installed, and currently everything is working smoothly with the old code.

5

I see. Indeed, if you don’t use createRoot, it uses React 17 behind the scenes.

I just created a Meteor app using React, it defaults to React 18, and I deployed it to Galaxy with no issues. URL: https://test-react-app.meteorapp.com/

The main.jsx file is below, I think it’s the same as yours if we compare how it creates the app. :thinking:

import React from 'react';
import { createRoot } from 'react-dom/client';
import { Meteor } from 'meteor/meteor';
import { App } from '/imports/ui/App';

Meteor.startup(() => {
  const container = document.getElementById('react-target');
  const root = createRoot(container);
  root.render(<App />);
});
6

Interesting. I’m not using SSR (or trying to anyway!).

Just had a look (by inserting a console.log in the culprit file) - and there was no log on the server so I assume it isn’t running anything server side.

The plot thickens!

7

That code looks basically the same as mine. So weird!

But it clearly isn’t a galaxy issue, as the change is definitely causing the issue. Still can’t figure out why.

Note - there didn’t seem to be any crashes on the staging (free Galaxy tier). I suspect the number of clients connected may have a role to play?

1 Like
8

I can’t think of anything that could be affected by the number of clients here.

What about React Router? Is it updated to version 6.x?

9

No I’m running React-Router ^5.1.2.

What is your thought process behind asking about that?

10

“What is your thought process behind asking about that?”

React Router 6 no longer uses History.

11

I don’t use TS. Is that “!” at the end of “container” something in TS?!

12

If I’m in your position, here is what I will do:

  • upgrade to React 17 (no code changes), deploy, check
  • upgrade to React 18 (no code changes), deploy, check
  • change code for React 18, deploy, check

Weird that this is affecting the server without SSR

13

Yup it tells Typescript that this value will not be null, otherwise TS will scream at you (rightly to be fair).

14

So I did something similar in trying to figure out why this was happening. Steps I used:

  1. Upgrade code to React 18 - noticed the error.
  2. Undid all the changes - confirmed the upgraded code is cause of error.
  3. Updated code one bit at a time (painfully slow process!) and deployed until I found the specific change causing the error.

So now it is at the stage where all the code from step 1 is deployed, minus this single file. Without this file update (effectively running React 17), everything is fine, with this file change, the memory leak shows itself.

Would you suggest still going through your sequence of steps? What would I be looking for?

15

Our code in React 17 now does not work in React 18 (no code changes). Although we have SSR that is the culprit in our case but there might be enough changes between React 17 and 18 that might matter

16

Really odd. I’ll give it a shot downgrading the projects code from 18 to 17 and report back.