Optimize Queue.summary query

[grantcarthew]

This issue was raised by @thomasmodeneis in #37.

It looks like the Queue.summary performance leaves a little to be desired. The method was originally added to support testing and made public just as a convenience. Thomas is using it and it needs some love.

Can the RethinkDB query be improved?

Here is the current query code from the queue-summary.js file:

return q.r.db(q.db).table(q.name)
    .hasFields('status')
    .group((job) => {
      return job.pluck('status')
    }).count()

Performance points off the top of my head:

• The .hasFields('status') call is to prevent the state document from adding an undefined entry to the summary. This could be removed or changed. Removed post query as an example.
• There is no index defined in the query.

I am not an "expert" at RethinkDB so there is probably a lot of room for improvement here.

[thomasmodeneis]

I've tested the original query with last night run, there was about 30k rows on the queue.

1.54s round-trip time 1.48s server time 8 shard accesses

Then looking for optimisation I've found on the reference something: https://www.rethinkdb.com/api/javascript/group/ So, Then I created a new index status and changed the query a bit to use the index: r.db('test').table('analyticsQueue').group({index:'status'}).count()

865ms round-trip time 819ms server time 8 shard accesses

Still is not the best performance but is a little better already. :)

Cheers.

[grantcarthew]

Thanks @thomasmodeneis and great work.

I'll add an index on status and add it to the summary query. Can you please test if there is a significant difference between having the hasFields operative and without?

[thomasmodeneis]

Yes, ok, so I will make a fork and introduce the change today and test. I will let you know asap the results. Cheers.

[grantcarthew]

@thomasmodeneis No need to fork mate. You can just comment out the hasFields line from the package in your node_modules directory. Or you could use the Data Explorer in RethinkDB.

[thomasmodeneis]

Oh that would work, but I'm looking to understand better this module and this looks like a good chance for doing it :) Besides I also need to run the tests to check weather my changes are valid or not. Cheers.

[thomasmodeneis]

Hey @grantcarthew I did my changes and got adapted to your code, is a bit weird because you don't use ; anywhere, besides you also add spaces on the closure = > and my editor was complaining saying its supposed to be =>. Never mind, I'm now trying to run the tests with this thing tape, its my first time using this so and I'm having an error, so I changed to node 7.0.0; I've tried with node 6.9.1 and also nothing happens, I can change the assertions to something invalid but nothing happens.

$ tape tests/queue-summary.spec.js 

How do you output anything from this tests when you are building one ? console.log is the way out ?

This is the err when running with node >5.12.0.

$ tape tests/queue-summary.spec.js 
/opt/node-rethinkdb-job-queue/tests/queue-summary.spec.js:17
      let jobs = []
      ^^^

SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode

However it actually works when I run with npm run test, is there a way to run only one specific test and not the whole test suite ?

Cheers.

[thomasmodeneis]

Oh I see now; npm run test-current is the way to run single specs right?

Please let me reformulate my question: Is there a way to run only one of the tests in a single spec without commenting out other tests ?

Also for file indentation I see you used 2 lines, is this correct ?

Cheers.

[thomasmodeneis]

I've submitted this pull https://github.com/grantcarthew/node-rethinkdb-job-queue/pull/39 request with the changes + test fixed. Cheers.

[grantcarthew]

As stated by the badges on the README, I use JavaScript Standard Style. Also, this should answer the testing questions: https://github.com/grantcarthew/node-rethinkdb-job-queue/wiki/Testing

Changing the test-current.js file is probably not the best way of testing a single module. I haven't put much thought into it.

[grantcarthew]

Are you happy with the performance now @thomasmodeneis ?

[grantcarthew]

I'm going to close this now @thomasmodeneis Open it if you think there could be more options to improve it.

Thanks mate.