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

Need to explicit destroy knex in order to stop the script #534

Closed
Kostanos opened this issue Oct 4, 2017 · 17 comments
Closed

Need to explicit destroy knex in order to stop the script #534

Kostanos opened this issue Oct 4, 2017 · 17 comments

Comments

@Kostanos
Copy link

Kostanos commented Oct 4, 2017

Hi, I initialize Model this way:

const Knex = require('knex');
const Model = require('objection').Model;
const knex = new Knex({
  client: 'pg',
  pool: { min: 1, max: 10 },
  connection: {},
  searchPath: 'knex,public',
});
Model.knex(knex);

The problem I have is when I run tests with mocha, mocha never ends until I run:

knex.destroy();

And even if I run it, mocha keep waiting for extra 5-10 seconds, and after actually ends.
Which makes me believe, that Objection or knex still have some process running to keep pool connections alive.

Any suggestions how could I cut this "waiting" time in mocha?

@koskimas
Copy link
Collaborator

koskimas commented Oct 4, 2017

Hi!

I use after(() => knex.destroy()) and that's enough to let the process die immediately. Neither objection nor knex start any processes.

Have you tried setting the minimum pool size to zero?

@newhouse
Copy link
Contributor

newhouse commented Oct 4, 2017

@Kostanos As food for thought, in my own mocha tests, I leverage the after hook in the mocha config to kill my process. E.g.

...
// After hook
after(function(done) {
  return doTeardownStuff()
    .catch(err => {
      console.log(err);
      return Promise.resolve();
    })
    .then(() => {
      // Kill the process after some time passes, because I know that I am done.
      setTimeout(function() {process.exit();}, 2000);
    });
});
...

It's a little bit "hacky" but it works fine and I think is safe enough for my scenario.

@koskimas
Copy link
Collaborator

koskimas commented Oct 4, 2017

I have never had to kill the process, nor have I ever had problems with mocha tests not finishing. I think there is something "wrong" with your setups. Check out objection's tests. They have no such problem.

@Kostanos
Copy link
Author

Kostanos commented Oct 4, 2017

Just tried with min=0, same result :(

Thank you for killing suggestion, it works, at least now test doesn't consume extra actually >20 seconds, as I see in pipeline.

BTW. the draining of knex with DEBUG=*

  knex:pool INFO pool postgresql:pg:client0 - draining +7ms
  knex:pool INFO pool postgresql:pg:client0 - force destroying all objects +0ms

I'm checking other possible scenarios, I have pg-restify server, which I also put down with:

this.restify.close();

But the issue only happens when I run test that uses ModelBase I showed above.

In tests where ModelBase is not used, the script exists fast.

@koskimas
Copy link
Collaborator

koskimas commented Oct 4, 2017

I'm pretty sure this has nothing to do with objection, but just to make sure, could you create a small standalone project that reproduces this? I'm not able to reproduce this.

@koskimas
Copy link
Collaborator

koskimas commented Oct 4, 2017

Maybe you have transactions that are never committed or rolled back? Those keep a connection alive.

@Kostanos
Copy link
Author

Kostanos commented Oct 4, 2017

Will try later about small project.
It is more complex here, for sure it may be provoked by some other dependency, even the same pg-restify + objection may have conflicts, as both of them open connections on their own.

No pending transactions for sure, checked it.

The reason why I pointed to objection+knex, is because if I don't destroy knex with knex.destroy(), the test never ends at all, and with DEBUG=* I see knex keeps pinging connection in pool.

Anyway, for now I'm ok with process.exit, will go forward with next tasks.

If you prefer, we can close this task until I have more data to reproduce it.

Thank you for your time and help!

@koskimas
Copy link
Collaborator

koskimas commented Oct 5, 2017

This can easily be an issue in knex. Objection doesn't start queries or do anything funky without the user explicitly telling it. I'll close this now, but I'd like to know what caused this if you get to the bottom of this.

@koskimas koskimas closed this as completed Oct 5, 2017
@tmaxim-gpsw
Copy link

For me explicitly setting pool.min = 0 and knex.destroy() in jasmine.onComplete did the trick. My issue was similar with yours: Testing process was not exiting. What's odd is that pool.min should be 0 by default but I still needed to explicitly set it in my knexfile. Any thoughts?

@koskimas
Copy link
Collaborator

@tmaxim-gpsw No idea. I have never had that issue. Knex github or gitter is probably a better place to get the answer for this.

@elhigu
Copy link
Contributor

elhigu commented Oct 27, 2017

I'm not sure if related, but default behaviour changed in mocha https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#default-behavior

That it does not force exit after tests has been ran, so I suppose it will wait until generic-pool evicts idle connections before exitting...

Supply the --exit flag to revert to pre-v4.0.0 behavior

I would go for explicitly destroying knex after tests has been complete.

@koskimas
Copy link
Collaborator

koskimas commented Oct 27, 2017

I would go for explicitly destroying knex after tests has been complete.

@elhigu The issue seems to be that knex.destroy() isn't enough.

@elhigu
Copy link
Contributor

elhigu commented Oct 27, 2017

Uh right... I missed that. knex.destroy() waits nowadays that connections are closed gracefully... so I would expect that testcode has something fishy going on in this case... maybe an open transaction or something like that.

@horia141
Copy link

Seeing the same problem. It was fixed by using pool: {min: 0}. I'll add knex.destroy() just in case.

@cztomsik
Copy link

cztomsik commented Mar 7, 2019

add idleTimeoutMillis: 500 too if you want to use knex in jest

@Kostanos
Copy link
Author

Kostanos commented May 6, 2019

I've added idleTimeoutMillis, but still have the issue.
This is my knex/objection initialization file:

const info = require('debug')('ws:info:db');

const Knex = require('knex');
const Model = require('objection').Model;

info('Connecting..');
const knex = Knex({
  client: 'pg',
  pool: { min: 0, max: 10, idleTimeoutMillis: 500, },
  connection: {},
  searchPath: 'knex,public',
  // debug: true,
});

Model.knex(knex);

module.exports = Model;

and here how I close it:

const ModelBase = require('./lib/models/ModelBase');

      ModelBase.knex().destroy()
      .then(() => {
        // TODO:  never enters here on tests :(
        debug('Knex closed');
        if (this.wsServer){
          debug('Closing WS Server');
          this.wsServer.close(() => {
            this.server.close(resolve);
          });
        } else resolve();
      });

BTW. it only happens if I run at least one query. if I don't run any queries, no timeout on tests (the connections closes without issues)

tegandbiscuits pushed a commit to shiftdsm/Galavanting-Gnome that referenced this issue May 29, 2019
I don't really know what the implications are of this, but I saw it
suggested (here)[Vincit/objection.js#534] and
it worked and doesn't require manually running `db.destroy()` after each
test.
tegandbiscuits pushed a commit to shiftdsm/Galavanting-Gnome that referenced this issue May 29, 2019
* Set timeout for test  pool

I don't really know what the implications are of this, but I saw it
suggested (here)[Vincit/objection.js#534] and
it worked and doesn't require manually running `db.destroy()` after each
test.

* Lint pass

* Skipping app test

I will go back and test these later.
@DoWhileGeek
Copy link

I still have the issue with idleTimeoutMillis: 500

Cruikshanks added a commit to DEFRA/water-abstraction-system that referenced this issue Sep 28, 2023
When trying to work with the DB in the tests we were getting

```
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.
```

Googling led us to [Having trouble running Jest, unhandled Promises](Vincit/objection.js#1015) and [Need to explicit destroy knex in order to stop the script](Vincit/objection.js#534). The second issue included a comment that setting the pool min size to 0 and idleTimeoutMillis to 500 would solve the problem. We tried it and it worked!
Cruikshanks added a commit to DEFRA/water-abstraction-system that referenced this issue Sep 28, 2023
When trying to work with the DB in the tests we were getting

```
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.
```

Googling led us to [Having trouble running Jest, unhandled Promises](Vincit/objection.js#1015) and [Need to explicit destroy knex in order to stop the script](Vincit/objection.js#534). The second issue included a comment that setting the pool min size to 0 and idleTimeoutMillis to 500 would solve the problem. We tried it and it worked!
Cruikshanks added a commit to DEFRA/water-abstraction-system that referenced this issue Oct 3, 2023
When trying to work with the DB in the tests we were getting

```
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.
```

Googling led us to [Having trouble running Jest, unhandled Promises](Vincit/objection.js#1015) and [Need to explicit destroy knex in order to stop the script](Vincit/objection.js#534). The second issue included a comment that setting the pool min size to 0 and idleTimeoutMillis to 500 would solve the problem. We tried it and it worked!
Cruikshanks added a commit to DEFRA/water-abstraction-system that referenced this issue Oct 8, 2023
When trying to work with the DB in the tests we were getting

```
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.
```

Googling led us to [Having trouble running Jest, unhandled Promises](Vincit/objection.js#1015) and [Need to explicit destroy knex in order to stop the script](Vincit/objection.js#534). The second issue included a comment that setting the pool min size to 0 and idleTimeoutMillis to 500 would solve the problem. We tried it and it worked!
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

8 participants