[PIO-193] Async support for predict method and storage access, blocking code wrapped in blocking construct

[longliveenduro]

I tried to leverage the already present async interface of elasticsearch and armouring the HBase and JDBC call with blocking constructs which will tell the standard scala ExecutionContext to use a separate thread for that.

Let me know what you think.

See https://issues.apache.org/jira/browse/PIO-193
and
https://lists.apache.org/thread.html/f14e4f8f29410e4585b3d8e9f646b88293a605f4716d3c4d60771854@%3Cuser.predictionio.apache.org%3E
and also https://jira.apache.org/jira/browse/PIO-182
for more details

See also actionml/universal-recommender#62 for corresponding UR PR

[takezoe]

Thanks for great work. However I think this way isn't acceptable because it breaks current interfaces. This means that all existing templates won't work after this change.

Async support in entire prediction service is really nice but we need to find another way.

[longliveenduro]

@takezoe yes, you are definitely right. I'm just thinking about and trying some backward compatible tweaks. Will get back!

[dszeto]

Thank you @longliveenduro ! Fingers crossed for your update. :)

[longliveenduro]

@dszeto @takezoe This would be one possible solution to be nearly 100% backward compatible. With "nearly" I mean that an engine implementor has to add "override" to its implementation of predictBase() / predict(), when he recompiles/changes his engine. Because in this suggestion I implement it with a default NotImplementedError stating that the method is deprecated. But this should only be necessary when the engine developer recompiles his engine. With that "override" hint he then also gets a good hint to move on to the new async implementation. Already compiled engines should still work as the simply override the new default impl. of predict/predictBase. predictBaseAsync and predictAsync simply delegate to the old methods and wrapping a blocking block around it to tell the standard scala execution context to execute them in a second, much bigger threadpool. See chapter "Blocking" from https://www.beyondthelines.net/computing/scala-future-and-execution-context/

If you want we could just drop this default implementation of BaseAlgorithm.predictBase / Subclass.predict and would have 100% compatibility but then even a new async implementation would have to implement these old method signatures with a dummy.

Other solution I did think of were using some runtime information (things like checking dynamically if a method is present or not) but I decided not to use that, because these kind of lookups are usually very slow and IMHO breaks type saftey.

[takezoe]

I haven't checked this pull request in details, but first of all, I would like to discuss about our direction about this topic.

Basically, I agree to introduce asynchronousity in whole predict API process and I think your pull request is one of good ideas to achive it. Since we already have Akka-HTTP and async storage access layer, making prediction service asynchronous makes sense. In addition, if we can do it, we can also mark LEventStore's blocking methods as deprecated.

With "nearly" I mean that an engine implementor has to add "override" to its implementation of predictBase() / predict(), when he recompiles/changes his engine.

However, I'm not sure if this is acceptable in our backword compatibility policy. And also we need to update docs and all existing official templates to use new async APIs if we accept this pull request. It's a big change.

@dszeto What do you think about this?