Skip to content

core: disconnect clients #3275

Merged
Shubham8287 merged 14 commits into
masterfrom
shub/update-disconnect-clients
Oct 1, 2025
Merged

core: disconnect clients #3275
Shubham8287 merged 14 commits into
masterfrom
shub/update-disconnect-clients

Conversation

@Shubham8287
Copy link
Copy Markdown
Contributor

@Shubham8287 Shubham8287 commented Sep 23, 2025

Description of Changes

The AutoMigrateStep::DisconnectAllUsers step is implemented as follows:

  1. The spacetimedb::db::update::update_database function returns a response of type UpdateDatabaseResult::UpdatePerformedWithClientDisconnect.

  2. Upon receiving this response, the host_controller::update_module proceeds to drop the watch::Sender<ModuleHost> field within the core::host_controller::Host and disconnect clients.

companion private PR - https://github.com/clockworklabs/SpacetimeDBPrivate/pull/2095

API and ABI breaking changes

NA

Expected complexity level and risk

Diff code is simple but It depends on the subcription logic to behave correctly.

Testing

Manually.

@Shubham8287 Shubham8287 force-pushed the shub/update-disconnect-clients branch from a6e35ec to ec9f02a Compare September 23, 2025 15:42
Comment thread crates/core/src/db/update.rs Outdated
@Shubham8287 Shubham8287 requested review from gefjon and kim September 23, 2025 15:57
Comment thread crates/core/src/subscription/module_subscription_manager.rs Outdated
@Shubham8287 Shubham8287 changed the base branch from shub/add-columns to master September 24, 2025 12:49
@Shubham8287 Shubham8287 force-pushed the shub/update-disconnect-clients branch from 3a0e6bd to 49cfb51 Compare September 24, 2025 12:59
@Shubham8287 Shubham8287 force-pushed the shub/update-disconnect-clients branch from 16f51b1 to 068d84f Compare September 24, 2025 20:29
gefjon
gefjon previously approved these changes Sep 25, 2025
Copy link
Copy Markdown
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks! This seems like the correct way to disconnect clients.

Comment thread crates/core/src/db/update.rs
@gefjon gefjon self-requested a review September 25, 2025 14:03
@gefjon gefjon dismissed their stale review September 25, 2025 14:04

Actually, I realized I'm not confident that we correctly call the client_disconnected reducers here.

@gefjon
Copy link
Copy Markdown
Contributor

gefjon commented Sep 25, 2025

How does client_disconnected get called for the disconnected clients?

@Shubham8287
Copy link
Copy Markdown
Contributor Author

Shubham8287 commented Sep 25, 2025

How does client_disconnected get called for the disconnected clients?

Dropping watch::sender leads to exit of ws_client_actor which was awaiting on ws_main_loop.

tokio::spawn(client.disconnect());

client.disconnect() imeplementation calls ModuleHost::disconnect_client() which does the job.

Shubham8287 and others added 3 commits September 25, 2025 19:58
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
@Shubham8287
Copy link
Copy Markdown
Contributor Author

Shubham8287 commented Sep 26, 2025

How does client_disconnected get called for the disconnected clients?

I fixed disconnect_redudcer not being called by explicitly calling it while updating module.
I also updated smoketest to test the disconnect reducer calls .

Comment thread crates/core/src/host/module_host.rs Outdated
Comment thread crates/core/src/host/module_host.rs Outdated
@Shubham8287 Shubham8287 requested a review from gefjon September 26, 2025 17:12
Comment thread crates/datastore/src/locking_tx_datastore/mut_tx.rs Outdated
@Shubham8287 Shubham8287 requested a review from gefjon September 29, 2025 16:42
@bfops bfops added the release-any To be landed in any release window label Sep 29, 2025
@Shubham8287 Shubham8287 added this pull request to the merge queue Oct 1, 2025
Merged via the queue into master with commit bfc9741 Oct 1, 2025
26 of 28 checks passed
@Shubham8287 Shubham8287 deleted the shub/update-disconnect-clients branch October 1, 2025 16:30
bfops pushed a commit that referenced this pull request Oct 8, 2025
# Description of Changes
The `AutoMigrateStep::DisconnectAllUsers` step is implemented as
follows:

1. The `spacetimedb::db::update::update_database` function returns a
response of type
`UpdateDatabaseResult::UpdatePerformedWithClientDisconnect`.

2. Upon receiving this response, the `host_controller::update_module`
proceeds to drop the `watch::Sender<ModuleHost>` field within the
`core::host_controller::Host` and disconnect clients.

# API and ABI breaking changes
NA

# Expected complexity level and risk
3.
Diff code is simple but It depends on the subcription logic to behave
correctly.

# Testing
Manually.

---------

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
bfops pushed a commit that referenced this pull request Oct 8, 2025
# Description of Changes
The `AutoMigrateStep::DisconnectAllUsers` step is implemented as
follows:

1. The `spacetimedb::db::update::update_database` function returns a
response of type
`UpdateDatabaseResult::UpdatePerformedWithClientDisconnect`.

2. Upon receiving this response, the `host_controller::update_module`
proceeds to drop the `watch::Sender<ModuleHost>` field within the
`core::host_controller::Host` and disconnect clients.

# API and ABI breaking changes
NA

# Expected complexity level and risk
3.
Diff code is simple but It depends on the subcription logic to behave
correctly.

# Testing
Manually.

---------

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-any To be landed in any release window

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants