Repository navigation
Convert inlineCallbacks to async/await #7988
Description
Activity
I should mention that if anyone is interested in helping out here they can poke me here or in #synapse-dev:matrix.org
To give a bit of a higher level update here:
- Most non-test modules have been converted to use async/await, there's also a few PRs outstanding (notably for
runInteraction). - Most tests need to be switched from using
defer.inlineCallbacksstyle to useget_success(). - There's probably some documentation that needs to be updated.
- It would be nice to see if some
ensureDeferredcalls could be removed (since that seems to makeinlineCallbackobjects).
- Most non-test modules have been converted to use async/await, there's also a few PRs outstanding (notably for
The current work on this is done, but I wanted to give another update:
- The vast majority of the code has been converted from
inlineCallbackstoasync. - There are a few remaining bits that could be converted, it is probably worth doing those, but at a much lower effort than we spent so far on this.
- Generally methods that return an awaitable are now
asyncandawaiton other calls instead of passing through awaitables from other calls. - There are remaining uses of
ensureDeferred, which has the unfortunate side-effect of convertingasynctoinlineCallbacks. - The tests need a lot of updating, in particular the use of tests being defined as
inlineCallbacksand then usingyieldshould be replaced withself.get_success, but this requires the test case inheriting fromHomeserverTestCase, which not all tests use. This would be a great way to contribute if someone is interested!
- The vast majority of the code has been converted from
It came up that it was never really written down why we were doing this or how we were doing this. I'll attempt to document what I remember:
Why convert
inlineCallbackstoasync/await:inlineCallbacksmangle stack traces pretty badly,async/awaitshould improve the stack traces for profiling and exceptions (e.g. in Sentry).async/awaitis more modern looking and better understood by people who don't have deep knowledge of Twisted.- Since
async/awaitis a language feature it has better support from other packages, static analyzers, tools, etc. asyncfunctions can provide better type hints for return values.- There's some thought that this might have a small performance boost (although I don't think we ever saw any).
Rules for convert from
inlineCallbackstoasync/await:The result of calling an
asyncfunction is anAwaitable, the result of calling aninlineCallbacksfunction is aDeferred.asyncfunctions useawaitinternally to wait for anotherAwaitable,inlineCallbacksuse yield internally to wait for anotherDeferred.- You can
awaitaDeferred(since it is also anAwaitable). - You cannot
yieldanAwaitable. - You can convert an
Awaitableinto aDeferredviadefer.ensureDeferred. - Note that you can
yieldon a non-Deferred(and it just immediately continues), but callingawaiton a non-Awaitableis a runtime error. - Twisted APIs still expect
Deferreds.
Methodology for converting from
inlineCallbackstoasync/await:Since you can
awaitaDeferredthe easiest way to do this is to start at the outer layers and work inward. By doing this you end up withasyncfunctions which call into code which returnsDeferreds, but this is fine. For Synapse we converted things via:- The REST layer.
- The handler layer.
- The database layer.
In order to avoid doing an entire layer at once you want to start with the modules which are called into the least (preferably only via the layer above!). If there are other callers which have not yet been converted, the call-site is modified to wrap the returned
AwaitablewithensureDeferred.The REST layer needed some magic since this needs to integrate into Twisted, see
_AsyncResourceand sub-classes, in particular this:- Overrides
render(which is a Twisted API fromIResource).- Calls the async function with
defer.ensureDeferredto ensure it gets scheduled with the reactor. - Returns
NOT_DONE_YETso that Twisted doesn't close the connection.
- Calls the async function with
- It then searches for a method called
_async_render_<HTTP METHOD>and calls it. - If that is also async it awaits it.
- Finally it sends the response.
We also had many places which were undecorated functions which returned a
Deferredvia calling something else. While doing this conversion we updated those to beasyncand then internallyawaitthe called function, for clarity.This also involved updating the tests to match the type as well (i.e. if a function was made
asyncand we mock that function somewhere, the mock should also beasync).As this isn't actively being worked on I'm not sure having a tracking issue for this makes sense anymore. The https://patrick.cloke.us/areweasyncyet/ page was updated as of yesterday (and is easy enough to keep up-to-date).
I think most of the remaining bits are converting the tests to use
HomeserverTestCase+get_successinstead of marking the test asinlineCallbacksandyield-ing eachDeferred. This is tedious, although not particularly hard. It also isn't particularly urgent since the gains from it are minor.I think most of the remaining uses in the
synapsepackage probably need to be kept in order to interface with Twisted APIs, although there might be a few more that could go away.
There's a desire to convert code that uses
inlineCallbackstoasync/awaitsince it makes stack traces nicer and helps with profiling (e.g. #7670).Some progress on this can be tracked at https://patrick.cloke.us/areweasyncyet/
I've broken down the Synapse code into modules for tracking of this work:
contrib.cmdclientcontrib.experimentsdocs.log_contextsUpdate the logcontext doc #10353scripts.synapse_port_dbPort synapse_port_db to async/await #6718synapse.apiasync/await is_server_admin #7363 Convert synapse.api to async/await #8031synapse.appasync/await get_user_id_by_threepid #7620 Convert synapse.app and federation client to async/await. #7868 Convert the main methods run by the reactor to async. #8213synapse.appserviceConvert appservice to async. #7973 Convert an errant inlineCallbacks in appservice #8207synapse.cryptoConvert the crypto module to async/await #8003synapse.eventsConvert a synapse.events to async/await #7949synapse.federationConvert synapse.federation.transport.server to async #5689 Port federation_server to async/await #6279 Port receipt and read markers to async/wait #6280 Port much ofsynapse.federation.federation_clientto async/await #6840 Cast a coroutine into a Deferred in the federation base #6996 async/await is_server_admin #7363 Convert synapse.app and federation client to async/await. #7868 Convert federation client to async/await. #7975 Fix unawaited coroutine error in tests. #8072synapse.groupsasync/await is_server_admin #7363 Convert groups local and server to async/await #7600 Convert groups and visibility code to async / await. #7951synapse.handlersPort room rest handlers to async/await #6275 Port receipt and read markers to async/wait #6280 Port SyncHandler to async/await #6484 Port synapse.handlers.initial_sync to async/await #6496 Port handlers.account_validity to async/await. #6504 Port some of FederationHandler to async/await #6517 Port some admin handlers to async/await #6559 Port much ofsynapse.handlers.federationto async/await. #6837 Port PresenceHandler to async/await #6991 Convert auth handler to async/await #7261 Convert some of the federation handler methods to async/await. #7338 async/await is_server_admin #7363 Convert the room handler to async/await. #7396 Convert federation handler to async/await. #7459 Convert search code to async/await. #7460 Update the room member handler to use async/await. #7507 Convert sending mail to async/await. #7557 Convert identity handler to async/await. #7561 Convert user directory handler and related classes to async/await #7640 Convert the registration handler to async/await. #7649 Convert the device message and pagination handlers to async/await #7678 Convert the typing handler to async/await. #7679 Convert directory handler to async/await #7727 Convert the appservice handler to async/await. #7775 Convert E2E key and room key handlers to async/await #7851 Convert _base, profile, and _receipts handlers to async/await #7860 Convert device handler to async/await #7871 Convert the message handler to async/await #7884 Convert room list handler to async/await. #7912 Update the auth providers to be async. #7935 Convert presence handler helpers to async/await. #7939 Fix up types and comments that refer to Deferreds. #7945 Remove hacky error handling for inlineDeferreds. #7950synapse.httpMake the http server handle coroutine-making REST servlets #5475 fix async/await consentresource #5585 Convert the federation agent and related code to async/await. #7874 Convert federation client to async/await. #7975 Convert the SimpleHttpClient to async. #8016 Convert the well known resolver to async #8214synapse.loggingFix the trace function for async functions. #7872synapse.metricsConvert run_as_background_process inner function into an async function #8032synapse.module_apisynapse.notifierConvert the synapse.notifier module to async/await. #7395synapse.pushConvert sending mail to async/await. #7557 Convert push to async/await. #7948 Ensure that remove_pusher is always async #7981synapse.replicationPort replication http server endpoints to async/await #6274 Port synapse.replication.tcp to async/await #6666 Convert replication code to async/await. #7987synapse.restMove rest/admin to use async/await. #6196 Port rest/v1 to async/await #6482 Port rest client v2_alpha to async/await #6483 Convert remote key resource REST layer to async/await. #7020 Convert some of the media REST code to async/await #7110 Convert more of the media code to async/await #7873 Finish converting the media repo code to async / await. #7947 Fix async/await calls for broken media providers. #8027synapse.serversynapse.server_noticesasync/await is_server_admin #7363 Convert synapse.server_notices to async/await. #7394 Fix some comments and types in service notices #7996synapse.spam_checker_apisynapse.stateConvert state resolution to async/await #7942 Add type hints for state. #8140synapse.storageAsync/await for background updates #6647 Convert delete_url_cache_media to async/await. #7241 async/await is_server_admin #7363 async/await get_user_id_by_threepid #7620 Convert storage layer to async/await. #7963 Convert some of the data store to async #7976 Continue converting store to async/await #8042 Convert additional database stores to async/await #8045 Converts event_federation and registration databases to async/await #8061 Convert tags and metrics databases to async/await #8062 Convert account data, device inbox, and censor events databases to async/await #8063 Convert appservice, group server, profile and more databases to async #8066 Convert devices database to async/await. #8069 Convert the roommember database to async/await. #8070 Convert events worker database to async/await. #8071 Convert stream database to async/await. #8074 Convert pusher databases to async/await. #8075 Convert receipts and events databases to async/await #8076 Convert misc database code to async #8087 Convert some of the general database methods to async #8100 Convert runWithConnection to async. #8121 Do not assume calls to runInteraction return Deferreds. #8133 Convert runInteraction to async/await #8156 Convert simple_select_one and simple_select_one_onecol to async #8162 Convert calls of async database methods to async #8166 Convert additional database methods to async (select list, search, insert_many, delete_*) #8168 Convert simple_update* and simple_select* to async #8173 Convert simple_delete to async/await. #8191 Convert stats and related calls to async/await #8192 Convert state and stream stores and related code to async #8194 Convert a grab bag of database code to async/await #8195 Convertevent_push_actions,registration, androommemberdatastores to async #8197 Convert additional databases to async/await #8199 Convert additional databases to async/await part 2 #8200 Convert additional databases to async/await part 3 #8201synapse.streamsConvert streams to async. #8014synapse.utilMake ObservableDeferred.observe() always return deferred. #6291 Fix stacktraces when using ObservableDeferred and async/await #6836 Convert some util functions to async #8035 Remove the unused inlineCallbacks code-paths in the caching code #8119 Convert ReadWriteLock to async/await. #8202 Convert Clock.sleep to async. #8215synapse.visibilityConvert groups and visibility code to async / await. #7951tests.apiConvert additional test-cases to homeserver test case #9396tests.appservicetests.cryptotests.federationtests.handlersConvert test cases to use HomeserverTestCase #9377 Convert additional test-cases to homeserver test case #9396tests.httptests.replicationtests.restConvert test cases to use HomeserverTestCase #9377 Convert additional test-cases to homeserver test case #9396tests.servertests.server_noticestests.statetests.storageDo not yield on awaitables in tests. #8193 Convert storage test cases to HomeserverTestCase. #9736tests.test_federationtests.test_servertests.test_statetests.test_utilsAllow for make_awaitable's return value to be re-used. #8261tests.test_visibilitytests.unittesttests.utiltests.utils