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

Test times out due to failed lock acquisition (lock TTL == test timeout) #967

Open
davisjam opened this issue Oct 19, 2016 · 1 comment
Open

Comments

@davisjam
Copy link
Contributor

davisjam commented Oct 19, 2016

I'm running a customized version of node based on node v0.12.7.
The version of node I'm using modifies the timing of events a bit, but is otherwise a faithful implementation of node v0.12.7.

My machine is Ubuntu 14.04.3.

I took the following steps:

  • clone repo
  • npm install
  • npm install --only=dev

I then tried to run the test suite, but it reliably (23/30 attempts) fails when I run "npm test".
It fails with a timeout on the test case 'should honor original delay at fixed backoff'.

I inserted some console.log() to try to find the root cause.
Here's what I could find:

  • Logs in lib/queue/job.js report that the job is registered and its state is set to delayed.
  • In lib/kue.js Queue.prototype.checkJobPromotion, the warlock lock acquisition fails for the entire duration of the test. I note that the default test timeout is 2000 ms (2 seconds), and this is the same as the lockTtl in checkJobPromotion.

If I change lockTtl in lockTtl to 1000 ms, the lock is eventually acquired after about 1200 ms, but then the assert in the test case about test case time fails ((now - start).should.be.approximately(400,120)).

If I add 'this.timeout 0' to the test case, the lock is eventually acquired after 2300 ms, but the same timing-sensitive assert fails.

The question is, why do we need to wait until the previous holder's lock expires to acquire it?

I added logs in warlock.lock and the logs appear to be being dropped appropriately by the previous test case. However, if I remove test case 'should be re tried after failed attempts', the entire test suite passes on 10/10 tries.

If I use a 'vanilla' version of v0.12.7, the test suite passes without a problem. I suspect a race condition.

@behrad
Copy link
Collaborator

behrad commented Oct 21, 2016

I'd be happy if you can create a PR for this to merge in 0.11.x branch

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

No branches or pull requests

2 participants