search

chilts / mongodb-queue

Message queues which uses MongoDB.
209 stars 92 forks source link

Incorrect indexes? #11

Closed longlostnick closed 8 years ago

longlostnick commented 8 years ago

We've noticed a blocking problem with a large queue and getting new items. It appears that the get query isn't using any of the indexes. Am I missing something?

Index: { deleted: 1, visible: 1, _id: 1 } https://github.com/chilts/mongodb-queue/blob/4fb386e3e1289b43da193265d974da0c31f26aef/mongodb-queue.js#L56

Query: { visible : { $lt : now() }, deleted : { $exists : false } } https://github.com/chilts/mongodb-queue/blob/4fb386e3e1289b43da193265d974da0c31f26aef/mongodb-queue.js#L90

The key order is incorrect, so the query wont use the index.

I can make a pull request to fix this, but wanted to pose the issue first.

chilts commented 8 years ago

You might be right. I added these at the time when I thought I understood that they would be used correctly, but I don't think I was 100% certain. Since you seem to be using it quite heavily, would you be able to investigate which index/indexes need adding/changing?

I'd be happy to hear of what you think would help. I'm not currently using this module in such a heavy way, so I'm not experiencing the same issues (yet!). :)

Thanks for raising this.

longlostnick commented 8 years ago

I'll take a look and see what I can figure out. It may be just as easy as changing that index, but if that one IS used somewhere else it may require adding a new one. Either way I'll figure it out and pull request.

No problem! Great little package that's helped us get by with some basic queuing stuff without having to build out and maintain something bigger like RabbitMQ.

chilts commented 8 years ago

Cool, thanks. Yeah, I really quite like the idea (still) of using MongoDB as an store, especially due to the atomic operations it allows. You're right, perhaps those other indexes are used elsewhere, so might be worth a new index. I'll see if I get time tonight to refresh my memory on why I added those ones.

Perhaps it's because the query is looking for items where deleted doesn't exist, but the index only contains those rows where deleted does exist. By saying this, I'm presuming that the index won't include entries where deleted is non-existant.

Perhaps instead of:

{ deleted: 1, visible: 1, _id: 1 }

It should just be:

{ visible: 1 }

(Having _id in there also doesn't make sense to me now.)

What do you think?

chilts commented 8 years ago

Also, an index on { ack : 1 } might also be useful for both .ping() and .ack(). Again, what do you think?

longlostnick commented 8 years ago

Did you see my pull request? I think I got it figured out. https://github.com/chilts/mongodb-queue/pull/12

It's fine to have deleted first even if it's not in certain entries. Long story short, the main thing is that the order of the keys in the selector need to match the order of the keys in the index.

I went through audited all of the queries and consolidated them down into two indexes that make the most sense.

Keep in mind querying with ack will still use the compound index of { ack: 1, visible: 1, deleted: 1 }.

I'm gonna push it out to my production environment in the next week or so and followup with any findings.

chilts commented 8 years ago

You are awesome. :) Many thanks.

And I'm learning something new about indexes too! I've merged the PR and will re-release to npm shortly.

chilts commented 8 years ago

Released as v2.1.0. I've added you to the changelog in the README to say thanks. :)

longlostnick commented 8 years ago

Thanks! I'll report back if I run into any issues with my implementation.