https://kotlinlang.org logo
Title
j

Jonathan Mew

11/01/2019, 9:52 AM
To keep things simple until we have a need for something more complex, we run controller code as blocking (using ktor). With that in mind, if I'm calling into a suspend function in a 3rd party library that makes network calls, is this a reasonable construct to use?
runBlocking(<http://Dispatchers.IO|Dispatchers.IO>) {
    try {
        thirdParty.doNetworkStuff()
    } catch (e: Exception) {
        log.warn("recoverable error", e)
    }
}
just want to make sure I'm not falling into any obvious gotchas
g

gildor

11/01/2019, 10:09 AM
Dispatchers.IO is most probably redundant for your case
But in general i’m not sure that it’s good approach (except some dirty hack for migration period) to use runBlocking
j

Jonathan Mew

11/01/2019, 10:10 AM
Would you mind expanding on
redundant
? Is it because it's likely the library does that for me under the hood?
g

gildor

11/01/2019, 10:11 AM
because you alredy blocked your current thread by wrapping with
runBlocking
, why do you want to execute this operation on one more thread on IO (so use 2 threads instead of 1)
also wrapping to IO dispatcher is not needed for non-blocking suspend functions, it should be used to wrap some blocking (IO bound) code
j

Jonathan Mew

11/01/2019, 10:15 AM
I guess I should just bite the bullet and push for the calling function to be suspend. Because we haven't invested in coroutine knowledge, I think the sight of a suspend function feels like complexity, and we have generally avoided it.
thanks @gildor
g

gildor

11/01/2019, 10:17 AM
Why do you use coroutines based network library in this case?
Also you limit your scalability when convert non-blocking code to blocking
j

Jonathan Mew

11/01/2019, 10:18 AM
we already use ktor, so when considering which http client to use, it seemed most straightforward to pull in the ktor implementation
g

gildor

11/01/2019, 10:19 AM
Ktor is completely coroutines based, so not sure how can you avoid using suspend functions there and why
j

Jonathan Mew

11/01/2019, 10:19 AM
and yes it will limit scalability to some degree, but we may well not need to scale so much that it becomes an issue, and might have other bottlenecks we hit first. If it becomes the issue that needs fixing, we'll invest in that coroutine knowledge.
g

gildor

11/01/2019, 10:20 AM
If you already use Ktor, you have 2 choices: drop ktor or learn how to use coroutines
I think it’s not the best strategy
especially keeping in mind that use suspend functions is very easy
j

Jonathan Mew

11/01/2019, 10:21 AM
The ktor CRUD methods are not suspend though
g

gildor

11/01/2019, 10:21 AM
You essentially creating unncessary technical depth from the beginning
The ktor CRUD methods are not suspend though
What do you mean?
isn’t every Ktor callback is suspend lambda?
j

Jonathan Mew

11/01/2019, 10:23 AM
fun <T:Table> T.insert(body: T.(InsertStatement<Number>)->Unit): InsertStatement<Number>
for example in Queries.kt
or maybe I'm missing the point
g

gildor

11/01/2019, 10:26 AM
Could you share a link on this function
looks as somethind DB specific, not Ktor
Maybe it’s part of Expose, not Ktor
also yeah, it looks as blocking function
j

Jonathan Mew

11/01/2019, 10:29 AM
Sorry, terrible sleep this week, that's exposed. You're right, ktor drops us into a suspend function, we just switch straight over to dispatchers IO and treat it as blocking code.
g

gildor

11/01/2019, 10:30 AM
yes, sounds as good way to work with blocking block from asyncronous context
j

Jonathan Mew

11/01/2019, 10:30 AM
(again, because it's simpler for the team to work with and it's not causing us issues - we know we're not making the most of it)
g

gildor

11/01/2019, 10:30 AM
just wrap it to IO
I understand how wrapping to IO dispatcher make sense, but than I’m not sure why do you need
runBlocking
j

Jonathan Mew

11/01/2019, 10:32 AM
Generally we have a layered architecture. The (suspend) handler delegates to some service(s) and they generally don't have suspend functions. I think in this case though I'm going to push for an exception.
The runBlocking was to follow that pattern and keep to non-suspend service functions. I think it's a strong enough argument against in this case though.
The worry is that we'll botch something unexpected like exception handling while we haven't invested in better understanding of coroutines. Thanks for your help and advice!
e

Evan R.

11/01/2019, 2:52 PM
@Jonathan Mew exposed calls are not thread safe by default, make sure to use the built-in coroutine support when starting your transactions: https://github.com/JetBrains/Exposed/wiki/Transactions#working-with-coroutines Note however that it’s experimental
j

Jonathan Mew

11/01/2019, 3:56 PM
@Evan R. thanks for that - is it fairly new? I don't remember seeing it when we were starting out. I resorted to handling the threadlocal transaction manager survival myself for use during testing, but haven't hit issues apart from that. I'll have a read.
e

Evan R.

11/01/2019, 4:01 PM
Yeah I think it’s kind of new but it is experimental. I personally ran into a minor issue where attempting to nest
newSuspendedTransaction
calls will throw an exception, but a fix was made for it. Not sure if it’s merged yet though.
Did a little digging. The fix was just merged to master yesterday, I assume it will be available in exposed 1.17.7.
j

Jonathan Mew

11/01/2019, 4:08 PM
Haha... I might hold off rushing into it given we don't have issues currently. Maybe one for when we next upgrade exposed.
👍 1