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

[Discussion][Dev] node-sqlite3 does not support worker_threads #574

Closed
chakflying opened this issue Oct 6, 2021 · 5 comments · Fixed by louislam/node-sqlite3#3
Closed
Labels

Comments

@chakflying
Copy link
Collaborator

chakflying commented Oct 6, 2021

While trying to implement scheduled jobs (for auto clear old statistics), I ran into an issue with sqlite crashing in native code. Turns out sqlite3 > 5.0 breaks when running in worker_threads (see node-sqlite3#1381). So here are my options:

  • Drop the use of worker_threads. For me this isn't ideal. Although there isn't a strong need for running background jobs in another thread for now, any future CPU-intensive work (like generating reports) would benefit greatly from it. It also helps with modularization since we can ensure each background job acquire it's own db connection. But not using threading does have benefit of a more simple implementation.
  • Switch to better-sqlite3, since I tested it seems to work fine on a worker. From the READMEs it also looks like an all-round better library. Searching the repo I see that the project briefly switched to better-sqlite3 before switching back, but I don't know the reasoning behind. Is there any specific issues users encountered?
@chakflying chakflying added the help label Oct 6, 2021
@gaby
Copy link
Contributor

gaby commented Oct 6, 2021

👍🏻 for using better-sqlite3.

I noticed the longer my instance of uptime-kuma runs, the slower it gets. We have over 200 monitors.

@louislam
Copy link
Owner

louislam commented Oct 6, 2021

cd1a3a2

I actually tried to switch to better-sqlite3, because node-sqlite3 seems to be not maintained anymore. (TryGhost/node-sqlite3#1483)

Since Uptime Kuma is using knex.js as the db wrapper, I wrote a better-sqlite3 driver for knex.js in
https://github.com/louislam/redbean-node/blob/master/lib/dialect/better-sqlite3/index.ts.

Unfortunately, I faced more problems with better-sqlite3:

  • Slower performance: Noticeable slow multi queries, the loading time of monitor list was very slow.
  • Memory leak: I noticed that the node process consumed more than 400MB RAM for a few days, and kept going.
  • Prebuilt problems: Very hard to install for non-Docker users, I remembered that I got a lot of reports for that.

I eventually switched back to node-sqlite3 with my own fork.
(https://github.com/louislam/node-sqlite3)

The ultimate solutions should be supporting MySQL in my opinion, but yeah, it is another big task.

Another direction is that if you have any clue on how to add back the worker_threads support of node-sqlite3, you can take a look of my own fork of node-sqlite3 and make a pull request.

@louislam
Copy link
Owner

louislam commented Oct 6, 2021

Just read some history in their repo.

TryGhost/node-sqlite3#1365 (comment)

The big change between v4.2.0 and v5.0.0 is that they upgraded NAN to N-API (TryGhost/node-sqlite3#1304)

But they are using N-API version 3
(https://github.com/mapbox/node-sqlite3/blob/593c9d498be2510d286349134537e3bf89401c4a/package.json#L16-L18)

Which is not support for worker_threads according to this comment: TryGhost/node-sqlite3#1365 (comment)

So either downgrading node-sqlite3 to 4.2.0 or upgrading N-API to 6 could probably solve the issue.

@allgloryforrobots
Copy link

22/12/2023
FATAL ERROR: Error::ThrowAsJavaScriptException napi_throw
1: 00007FF7DDA8433F node::SetCppgcReference+15695
2: 00007FF7DD9FEC26 EVP_MD_meth_get_input_blocksize+78566
3: 00007FF7DDA00B1C node::OnFatalError+252
4: 00007FF7DDA3876C napi_fatal_error+156
5: 00007FFE9B2F19D7
6: 00007FFE9B2F1DA6
7: 00007FFE9B3118E5
8: 00007FFE9B30AB67
9: 00007FF7DDA35792 node_module_register+3586
10: 00007FF7DDAEC610 uv_timer_stop+800
11: 00007FF7DDAEC7FA uv_timer_stop+1290
12: 00007FF7DDAE8A7B uv_update_time+491
13: 00007FF7DDAE85F4 uv_run+900
14: 00007FF7DDAB94C2 node::SpinEventLoop+402
15: 00007FF7DD8FAF74 RSA_meth_get_flags+159588
16: 00007FF7DD8F5FA8 RSA_meth_get_flags+139160
17: 00007FF7DDAD6CCD uv_poll_stop+237
18: 00007FF7DEC74590 inflateValidate+153328
19: 00007FFEE6EC26AD BaseThreadInitThunk+29
20: 00007FFEE7CEAA68 RtlUserThreadStart+40

in worker

@CommanderStorm
Copy link
Collaborator

@allgloryforrobots you are necroposing on an issue which has been closed a long time ago as resolved.
If you want your bug report to not get ignored, writing in an actual issue with full context is the way to go. Currently, your bug report does not hold enough information to debug on

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

Successfully merging a pull request may close this issue.

5 participants