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

knex oracle dialect sets UV_THREADPOOL_SIZE without knowing the effect #3715

Open
tsondergaard opened this issue Mar 9, 2020 · 1 comment

Comments

@tsondergaard
Copy link
Contributor

tsondergaard commented Mar 9, 2020

lib/dialects/oracledb/index.js has this code:

  // Node.js only have 4 background threads by default, oracledb needs one by connection
  if (this.driver) {
    process.env.UV_THREADPOOL_SIZE = process.env.UV_THREADPOOL_SIZE || 1;
    process.env.UV_THREADPOOL_SIZE =
      parseInt(process.env.UV_THREADPOOL_SIZE) + this.driver.poolMax;
  }

If the libuv threadpool has been initialized before this code is executed, this does not have the desired effect as UV_THREADPOOL_SIZE is only read once, when libuv initializes the threadpool. I don't think it is correct for the knex oracle dialect to do this. First of all because changing UV_THREADPOOL_SIZE at an arbitrary time during runtime is unreliable, but also because it is something for the actual oracle driver to work out.

I think the knex oracle driver should limit itself to logging a debug message if (UV_THREADPOOL_SIZE || 4) < this.driver.poolMax.

Background: This is not a theoretical problem. It was discovered precisely because the code above was executed too late to have an effect in the project that I am working on.

@briandamaged
Copy link
Collaborator

I agree that this is something that should be investigated further. I'm also concerned about how this code will behave in multi-client / multi-pool scenarios. (Speaking of which... the (UV_THREADPOOL_SIZE || 4) < this.driver.poolMax check probably would not be sufficient in this scenario. The check would work for each client individually, but not collectively)

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

2 participants