globalsign / mgo

The MongoDB driver for Go
Other
1.97k stars 230 forks source link

Cleanup txn-queues even for only Asserted documents. #286

Closed jameinel closed 5 years ago

jameinel commented 6 years ago

This has a threshold of how many transactions are ready to be removed from the transaction queue. Once we reach that threshold, we go ahead and update the document, even though it wouldn't require updates otherwise. In performance testing, it can be seen that having a document whose queue grows too long can significantly impact how long it takes to apply a new transaction. (We have to load the information about all the other transactions that asserted against this document, and check if any of them need to be applied.)

As an example, looping over 600 transactions to be applied, and timing how long it takes to apply them with various AssertionCleanupLength gives:

1 1.18s 2 1.17s 5 1.28s 10 1.39s 20 1.81s 50 2.94s 100 4.81s 0 22.99s # unlimited

So keeping the queues small significantly impacts transaction speed.

jameinel commented 6 years ago

I rebased this onto 'development', but if you need me to target another location, feel free to ask.

jameinel commented 6 years ago

Travis seems to be saying there is a problem with "golint", but I don't see how that is something that I introduced. If you have pointers of how I can fix it, please let me know.

maitesin commented 6 years ago

@jameinel

Can you rebase it again with development? I just fixed the build on it

jameinel commented 6 years ago

@maitesin thanks. I believe I just rebased and pushed again.

jameinel commented 5 years ago

I believe I've managed to fix all the tests that I had broken when rebasing this patch over to the globalsign development branch. Is there anything else I can do to push this along?

domodwyer commented 5 years ago

Looks legit - thanks for (another!) great PR @jameinel

jameinel commented 5 years ago

@domodwyer thanks for the review. Any chance to actually get this landed?

jameinel commented 5 years ago

I just realized that my commit that cleaned up the code comment didn't actually get pushed because I had amended a commit and thus 'git push' refused. This should be fixed, let me know if there is anything else I can do.

eminano commented 5 years ago

@jameinel Sorry for the delay. We'll merge into development as soon as the build is done. Thanks for your PR! :)

jameinel commented 5 years ago

Thanks @eminano, looks like I managed to not break the build with the comment fix \o/