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

[RTK Query][SSR] Where should resetApiState() be called to avoid memory leak? #4359

Open
truongle-teq opened this issue Apr 18, 2024 · 9 comments

Comments

@truongle-teq
Copy link

truongle-teq commented Apr 18, 2024

Hello!

ISSUE DESCRIPTION

I am using RTK Query and next-redux-wrapper to achive SSR in my project. I am facing the an issue that might be diagnosed as memory leak. We did a load testing in around 2 hours, in which there were upto 25k requests in 20 minutes, to test our auto scaling. After the test, the memory consumption was supposed to slowly drop to its stable amount. Instead, it continued to increase until reaching the maximum capacity and ECS had to restart the container. I think there might be a possibility that caching and rogue timers were left running on the server.

A tip to call store.dispatch(api.util.resetApiState()) can be seen on the official documentation. However, I have no idea where I should call it. If I call it in getServerSideProps, then the server store will be empty during hydration process of NextJS.

Here is what in my getServerSideProps:

export const getServerSideProps = wrapper.getServerSideProps(
  (store) => async (context) => {

    const res = await store.dispatch(
      getData.initiate({
        ...params,
      })
    );

    if (res.status === QueryStatus.fulfilled) {
      await Promise.all(store.dispatch(getRunningQueriesThunk()));
      return {
        props: {},
      };
    }

    return {
      notFound: true,
    };
  }
);

PACKAGE VERSION

"@reduxjs/toolkit": "^1.9.7",
"react-redux": "^8.1.3",
"next": "13.5.6",

QUESTION

Where exactly should I call resetApiState() to clear all the cache and timer on the server?

Hoping for a quick response. Thank you.

@phryneas
Copy link
Member

That's a good question - anywhere in getServerSideProps is probably too early, and I'm not sure if Next.js has a dedicated place for "cleaning up after render".

The big question is also what keeps things in your memory for so long - I would expect that it might be the setTimeout for cache collection? I would assume that reducing your keepUnusedDataFor to something small like 1 could already solve your problem.

@DaniAcu
Copy link

DaniAcu commented Apr 20, 2024

Could you share you _app.tsx setting?

I'm working in a similar stack.

@DaniAcu
Copy link

DaniAcu commented Apr 21, 2024

Right now next-redux-store is doing the store.getState() after runing the 2 callbacks. Maybe is a discussion to move to next-redux-wrapper instead. But I could share with you some work around that I'm using to dispatch the reset.

import api from "./apis/data"

export async function getServerSideProps(...args) {
  const getPropsWithState = wrapper.getServerSideProps(
    (store) => async (context) => {

      const res = await store.dispatch(
        api.getData.initiate({
          ...params,
        })
      );

      if (res.status === QueryStatus.fulfilled) {
        await Promise.all(store.dispatch(getRunningQueriesThunk()));
        return {
          props: {},
        };
      }

      return {
        notFound: true,
      };
    }
  );
  
  const props = await getPropsWithState(...args);
  const request = args.context?.req || args.context?.ctx?.req;
  const store = request.__nextReduxWrapperStore;
  
  store.dispatch(api.util.resetApiState())
  
  return props;
}

@truongle-teq
Copy link
Author

truongle-teq commented Apr 21, 2024

Thanks for the help, let's see if it solves my issue. BTW, here is my _app.tsx: @DaniAcu

export default function App(appProps: AppLayoutProps) {
  const { Component } = appProps;
  const Layout = Component.layout || MainLayout;
  const { store, props } = wrapper.useWrappedStore(appProps);
  const router = useRouter();
  const progressBarOptions = { easing: "ease", speed: 500, showSpinner: false };

  return (
    <Fragment>
      <GTM />
      <DefaultSeo {...DEFAULT_METADATA} />
      <NextNProgress color="#2188DD" options={progressBarOptions} />
      <Provider store={store}>
        <Layout>
          <Component {...props.pageProps} key={router.asPath} />
          <FloatingDocumentRequest />
        </Layout>
      </Provider>
    </Fragment>
  );
}

@truongle-teq
Copy link
Author

That's a good question - anywhere in getServerSideProps is probably too early, and I'm not sure if Next.js has a dedicated place for "cleaning up after render".

The big question is also what keeps things in your memory for so long - I would expect that it might be the setTimeout for cache collection? I would assume that reducing your keepUnusedDataFor to something small like 1 could already solve your problem.

I added keepUnusedDataFor: 1 to my RTKQ config. I'll run another load testing tomorrow to verify if it works. Thanks for your reply.

@phryneas
Copy link
Member

Right now next-redux-store is doing the store.getState() after runing the 2 callbacks. Maybe is a discussion to move to next-redux-wrapper instead. But I could share with you some work around that I'm using to dispatch the reset.

I believe that would clear data from the Redux store before the components could render, though, right? So the hooks wouldn't pick up any data?

@DaniAcu
Copy link

DaniAcu commented Apr 21, 2024

I was thinking the same today 🤔. But as far as I know, the getState() is inmutable because is the last return of the reducer, so the object that came in getState() after calling the clean up will not have the information but if I saved the previous in a variable it will continue having this information, because is not mutating the reference.

And the hooks are getting the information from the initial state which was already generated by the next-redux-wrapper. The app are just propagating the props in the store and in the component tree.

As next doesn't have a after render hook, I think it is the best approach that we could take.

@phryneas
Copy link
Member

But as far as I know, the getState() is inmutable because is the last return of the reducer

store.dispatch(api.util.resetApiState())

actively resets the reducer to empty so the next useSelector() call (which will happen on hook render) will see an empty state.

@DaniAcu
Copy link

DaniAcu commented Apr 22, 2024

You are right! I don't think that Next provide a way to run something after rendering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants