Bug #128104 for DBIx-Class: Async support integration

On 12/26/2018 08:10 AM, Mark Zealey via RT wrote: Show quoted text
Hi! Thanks for your interest in trying to patch in some async stuf. The short answer to your question is "it depends": the way the question is phrased makes me think that you are *not* on the right track - _construct_resuls is already 100% CPU bound and is separated from the fetched via the ::Cursor mechanism. We could try to duke out the details in this ticket, or alternatively if you prefer the velocity you can put together a different ::ResultSet-based subclass as a proof of concept and then we see which hooks are necessary to provide so that as much of the functionality is incorporated into DBIC itself while at the same time keeping DBIC 100% synchronous in its core dist. An extra piece that *may* be useful to you is this gist I put together way back when ( the most important implementation-blueprint is at the very very end from line 451 onward): https://gist.github.com/ribasushi/032fbb8b58e2b0d62fae Let me know how I can help further!

Thanks for this. I've pushed an example repo to https://github.com/mzealey/dbic-async which is working for dbix class resultset async ->all at the moment on pg (with cancellation) and mysql (can't see it supports cancellation). Example schema config in the readme and test script at root. I think a hook in Storage::DBI::_prepare_sth to allow options passed to 2nd parameter of ->prepare*() would simplify the code reasonably significantly as then we could just wrap the return value with the required ::Async::Request layer. I managed to avoid having to hack _construct_results by making the changes in the ResultSet->all fn. This is probably the correct way to do it although I'm not sure I like the ->all_p and then ->all idea; probably rename the latter to fetch_all and avoid having any ->all fn on the cursor to avoid issues. I'm not sure quite on the best way to handle the async sth object - currently wrapping it in a custom class and forwarding most calls through to DBI::st via AUTOLOAD nastyness; but it may be an idea to inherit from DBI::st & add in Promises either via MI or something else. I don't think having to instantiate a new DBIC instance for each async request is much of a problem - I'm currently thinking you would only use async in special cases (ie queries you know will be long-running), although perhaps adding an async connection manager layer for dbs where connection takes a while would be a later extension. As per comment in README.md code it's annoying having to set default_result_class in the class rather than the instance but I imagine there is a good reason for that and it's not the end of the world. Mark On 26/12/2018 15:20, Peter Rabbitson via RT wrote: Show quoted text