puniverse / comsat

Fibers and actors for web development
docs.paralleluniverse.co/comsat
Other
598 stars 103 forks source link

comsat-mongodb-allanbanks #13

Closed circlespainter closed 10 years ago

circlespainter commented 10 years ago

Hi!

I implemented a mongodb integration module for comsat based on http://www.allanbank.com/mongodb-async-driver/index.html version 2.0 and a fairly complete essential testsuite for it based on Embed MongoDB https://github.com/flapdoodle-oss/de.flapdoodle.embed.mongo ; I think you might be interested in main comsat inclusion.

The module takes a subclassing approach, so existing code based on the driver should require minimal to no code changes. The overridden methods from the original driver are just the blocking and future ones in order to make them fiber-blocking. The fiber-blocking overrides are implemented by bridging the async, callback-based corresponding methods in the original driver through Quasar's FiberAsync.

I can think of further TODOs both for driver and tests but I think inclusion could be possible already (the 99 JUnit tests covering the overridden methods run all ok here); you can find those ideas in the classes' JavaDocs comments, but shortly:

Incidentally (and luckily), starting with version 2.0, the license for the basic driver functionality has changed to Apache 2.0 http://www.allanbank.com/mongodb-async-driver/license.html which, to the extent of my knowledge, allows for liberal use even in commercial projects (previously commercial use required a commercial licence, which I think was a more strict policy than comsat's). Maybe the author decided such a change because of 10gen's OSS driver starting to consider/implement an async interface in the 3.0.x branch https://github.com/mongodb/mongo-java-driver/tree/3.0.x/async-driver

eitan101 commented 10 years ago

Wow !! Thank you very much. I'll take a look in the code to see if I have something to comment.

eitan101 commented 10 years ago

The following error appears during the tests:

8:38:10.621 [DEBUG] [TestEventLogger]     WARNING: Uninstrumented methods on the call stack (marked with **): 
18:38:10.622 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.Fiber.checkInstrumentation(Fiber.java:1587)
18:38:10.623 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.Fiber.verifySuspend(Fiber.java:1565)
18:38:10.623 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.FiberAsync.run(FiberAsync.java:117)
18:38:10.624 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.mongodb.FiberMongoDatabaseImpl.runCommand(FiberMongoDatabaseImpl.java:70)
18:38:10.624 [DEBUG] [TestEventLogger]      at com.allanbank.mongodb.client.MongoDatabaseImpl.drop(MongoDatabaseImpl.java:152) **
18:38:10.625 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.mongodb.AbstractTestFiberMongo$2.run(AbstractTestFiberMongo.java:89)
18:38:10.625 [DEBUG] [TestEventLogger]      at co.paralleluniverse.strands.SuspendableUtils$VoidSuspendableCallable.run(SuspendableUtils.java:42)
18:38:10.625 [DEBUG] [TestEventLogger]      at co.paralleluniverse.strands.SuspendableUtils$VoidSuspendableCallable.run(SuspendableUtils.java:30)
18:38:10.626 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.Fiber.run(Fiber.java:994)
18:38:10.626 [DEBUG] [TestEventLogger]      at co.paralleluniverse.fibers.Fiber.run1(Fiber.java:989)

The drop method itself is marked as suspendable, but runCommand is not, So the instrumentor doesn't find a reason to instrument it. In order to solve that, the following method has to be added to META-INF/suspendables: com.allanbank.mongodb.client.MongoDatabaseImpl.runCommand

circlespainter commented 10 years ago

I'm not sure why I'm not getting the error when running fork's tests... I seem to have "verifyInstrumentation" enabled and I can see that the "scanSuspendables" task is correctly listing "com.allanbank.mongodb.client.MongoDatabaseImpl.runCommand" in "META-INF/suspendable-supers". Have you got it there?

eitan101 commented 10 years ago

I am also running in your fork. Yes it is listed in the "META-INF/suspendable-supers", but it is not enough. You may not see that because it doesn't fail the test, it's only a warning (but an important one). It can be seen with gradle -i test, or in the test results report located in: ./comsat-mongodb-allanbanks/build/reports/tests/classes/co.paralleluniverse.fibers.mongodb.FiberMongoCollTest.html

circlespainter commented 10 years ago

Thanks! I've found the warning and added the new suspendable method, then the warning has disappeared

circlespainter commented 10 years ago

Hi! I added some additional asserts for the future-based API in order to make sure the addListener+callback functionality works ok.

Still clean merge and the stderr and stdout are still clean apart from some CPU hogging warnings coming from blocking calls in the tests (and ultimately driver's connection) setup stages.