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

_ensureIndex on more fields #122

Closed
mitar opened this issue Oct 26, 2015 · 16 comments
Closed

_ensureIndex on more fields #122

mitar opened this issue Oct 26, 2015 · 16 comments

Comments

@mitar
Copy link
Collaborator

mitar commented Oct 26, 2015

Should we have indexes also on:

collection._ensureIndex
  priority: 1
  retryUntil: 1
  after: 1

getWork sorts by those fields.

I have not measured anything, this is just by observing queries.

@mitar mitar changed the title _ensureIndex _ensureIndex on more fields Oct 26, 2015
@vsivsi
Copy link
Owner

vsivsi commented Oct 26, 2015

Probably yes... I'm still living in a mindset where indexing is a developer decision, but I guess I already violated that some time ago. I don't have much time between now and the end of the year, so this kind of thing (not really a bug) will probably need to wait.

@mitar
Copy link
Collaborator Author

mitar commented Oct 26, 2015

Should we add this then just to the documentation? To the section where we are mentioning indexes?

@rhyslbw
Copy link
Contributor

rhyslbw commented Oct 30, 2015

+1 for just adding them in. Is there anytime these wouldn't want to be indexed?

@vsivsi
Copy link
Owner

vsivsi commented Oct 30, 2015

@mitar If you want to implement this, I'm fine with it.

mitar added a commit that referenced this issue Oct 31, 2015
@mitar
Copy link
Collaborator Author

mitar commented Oct 31, 2015

Fixed in dev branch.

@rhyslbw
Copy link
Contributor

rhyslbw commented Oct 31, 2015

👍 FYI @vsiverson and @mitar I'm about to look into implementing Job Collection as a work queue for management tasks in Space.eventSourcing, such as snapshotting and projection rebuilds, so I'll be around if you need to bounce any ideas etc

@aldeed
Copy link

aldeed commented Dec 2, 2015

@mitar @vsivsi It seems like a single key index on status is also needed. This is currently listed as the slowest query on our Compose:

screenshot 2015-12-02 08 46 58

Using package version 1.2.2

@mitar
Copy link
Collaborator Author

mitar commented Dec 2, 2015

Where does this query comes from?

@aldeed
Copy link

aldeed commented Dec 2, 2015

Oh good point, looks like it's from some code we have that checks for stuck jobs periodically. So I guess it's up to you whether you want the package to set up the index. I believe pkgs should ensure indexes for the queries they do, but whether you want to anticipate other common queries, that's personal preference I guess.

@mitar
Copy link
Collaborator Author

mitar commented Dec 2, 2015

Yea, I think this should be done by your code then. I think this is clear that if you are doing some other queries it is on you to create indexes.

@mitar
Copy link
Collaborator Author

mitar commented Dec 2, 2015

I see that we are also doing queries on status for finding stale jobs. But I do create our own indexes.

@mitar
Copy link
Collaborator Author

mitar commented Dec 2, 2015

Hm, but those are no extra indexes, besides what is provided by this package.

But also indexes depends on the number of jobs you have. For not many jobs it might not be so needed.

@aldeed
Copy link

aldeed commented Dec 2, 2015

I don't think the type-status combined index helps when you're querying on status only

@mitar
Copy link
Collaborator Author

mitar commented Dec 2, 2015

I don't think the type-status combined index helps when you're querying on status only

That is my understanding as well.

@vsivsi
Copy link
Owner

vsivsi commented Dec 3, 2015

I agree that the package should only create indexes for queries it generates internally. Maintaining indexes isn't free, so generating indexes that you don't know will be needed is wasteful.

@vsivsi
Copy link
Owner

vsivsi commented Jan 29, 2016

Mitar's additional indexes will be released in 1.3.0 on Atmosphere today.

@vsivsi vsivsi closed this as completed Jan 29, 2016
@vsivsi vsivsi mentioned this issue Jul 13, 2017
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

4 participants