clockworklabs / SpacetimeDB

Multiplayer at the speed of light
https://spacetimedb.com
Other
4.12k stars 100 forks source link

Module hotswapping #1147

Open coolreader18 opened 3 weeks ago

coolreader18 commented 3 weeks ago

Description of Changes

This needs some testing. I feel like there's something obvious I'm missing here but this like, should work. At some point, I should write some smoketests about how clients behave when the module disconnects.

Expected complexity level and risk

3 (touches the websocket message actor - though I'm confident about that part. moreso just that I'm not sure that I preserve all the module lifecycle stuff.)

Testing

jdetter commented 3 weeks ago

Bot test resulted in the client not receiving any more transactions after the database update was deployed. The client doesn't get disconnected but I can't call any reducers and it appears that I no longer receive any updates

coolreader18 commented 2 weeks ago

Currently with this branch, calling update_database may invoke the __update__ reducer and thus send an event to subscribers before the new module is fully inserted into the database. I'm not sure of a good way to prevent this.

kim commented 2 weeks ago

Currently with this branch, calling update_database may invoke the __update__ reducer and thus send an event to subscribers before the new module is fully inserted into the database. I'm not sure of a good way to prevent this.

I believe this is fine, or at least after #1186 lands. __update__ runs in a transaction, so if it succeeds it is technically correct to send the event.

coolreader18 commented 2 weeks ago

So why are we starting it immediately now?

I think that was your change @kim - .start() used to be called after some other setup was done, but if we're always calling it immediately after constructing the module, we may as well just not have the mechanism for it.

kim commented 1 week ago

I think that was your change @kim - .start() used to be called after some other setup was done

Hm I don’t know that I did that — but we do need to start it, because we’re almost certainly going to call a reducer, no?

coolreader18 commented 1 week ago

It used to be .start()ed after calling init on it, but not anymore.