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

Replace setInterval #454

Closed
fcaps opened this issue Nov 11, 2023 · 2 comments · Fixed by #487
Closed

Replace setInterval #454

fcaps opened this issue Nov 11, 2023 · 2 comments · Fixed by #487

Comments

@fcaps
Copy link
Collaborator

fcaps commented Nov 11, 2023

using setInterval can be a huge issue described in multiple articles.
just search "nodejs why setInterval is bad".
a common solution is something like https://www.npmjs.com/package/node-cron, or everything that is based on https://nodejs.org/api/events.html#class-eventemitter

@fcaps
Copy link
Collaborator Author

fcaps commented Nov 12, 2023

this issue is more critical than thought, i forgot to shutdown my pc (and containers) for multiple hours, and the container was doing weird stuff.

steps to reproduce:

  • start the container
  • maybe kill the wordpress instance, so it will create issues
  • let it run for some time (depends on your system)
  • try to stop it -> not responsive
  • kill it -> killed, but somehow not correctly
  • start the same container again -> not starting
  • wait some time
  • start it again -> working

Since the website also has issues on prod where we cannot start it after a crash, i guess it's not a local issue.

I would guess it's all the setInterval creating a memory/thread mess, but could be wrong.

@fcaps
Copy link
Collaborator Author

fcaps commented Nov 13, 2023

update:
looking into this even more, i suspect having a "real" cache mechanism is even more useful than just updating a file every x mins.

old:
server starts -> query endpoints of wp/java-api -> saves file on local disk

new:

browser hits endpoint  ->
   -> cache is not stale -> deliver 
   -> cache is stale -> query endpoint -> safe cache -> deliver

Some upside with this approach:

  • no need to have a cronjob running on the server (no setInterval/timeout)
  • container can start without calling endpoints on java-api/wordpress
  • no calls if there is no need to update (if there is no user)

downsides:

  • one user is the "cache-looser" (long http request time)
  • need a mutex/flock to not call the endpoint multiple times if cache is stale
  • some may complain about memory usage (<10MB)
  • some may complain about no file to look at
  • maybe more

packages that can be used:

https://github.com/node-cache/node-cache
maybe a mutex? but have to check it => https://stackoverflow.com/questions/64899813/does-node-cache-uses-locks

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

Successfully merging a pull request may close this issue.

1 participant