Skip to content

Fix collaborative document cleanup #12595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jtpio merged 1 commit into from
May 18, 2022
Merged

Fix collaborative document cleanup #12595

jtpio merged 1 commit into from
May 18, 2022

Conversation

@davidbrochart
Copy link
Contributor

@davidbrochart davidbrochart commented May 18, 2022

Code changes

In collaborative mode:

  • fixed the condition to know if the room is empty,
  • added a collaborative_document_cleanup_delay option to set the delay to keep a document in memory in the back-end after all clients disconnect (None to keep the document forever, defaults to 60 seconds).

User-facing changes

The new --LabApp.collaborative_document_cleanup_delay=1 option.

Backwards-incompatible changes

None.

@jupyterlab-probot
Copy link

jupyterlab-probot bot commented May 18, 2022

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval fcollonval added this to the 4.0 milestone May 18, 2022
fcollonval
fcollonval approved these changes May 18, 2022
View reviewed changes
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @davidbrochart

@jtpio jtpio merged commit 055099f into jupyterlab:master May 18, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 18, 2022

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

Results table
Test file large_code_notebook large_md_notebook
open
chromium
actual 15211 <- [16244 - 16805 - 17551] -> 21181 3448 <- [3710 - 3790 - 3866] -> 4431
expected 14802 <- [15224 - 15618 - 16023] -> 18437 3517 <- [3680 - 3779 - 3869] -> 4349
Mean relative change 8.5% ± 1.7% 0.4% ± 1.0%
switch-from
chromium
actual 674 <- [713 - 742 - 785] -> 1044 518 <- [551 - 565 - 577] -> 773
expected 656 <- [690 - 707 - 730] -> 928 520 <- [549 - 558 - 576] -> 783
Mean relative change 6.1% ± 2.6% 1.6% ± 2.8%
switch-to
chromium
actual 423 <- [478 - 491 - 498] -> 579 330 <- [858 - 869 - 897] -> 1217
expected 413 <- [467 - 483 - 492] -> 551 820 <- [857 - 868 - 889] -> 1187
Mean relative change 2.3% ± 1.7% -0.0% ± 3.0%
close
chromium
actual 1142 <- [1184 - 1196 - 1231] -> 1383 599 <- [631 - 639 - 655] -> 780
expected 1145 <- [1164 - 1179 - 1193] -> 1316 602 <- [631 - 645 - 662] -> 803
Mean relative change 2.0% ± 0.9% -0.1% ± 1.5%

Changes are computed with expected as reference.

@davidbrochart davidbrochart deleted the fix_document_cleanup branch May 18, 2022 13:24
@davidbrochart davidbrochart mentioned this pull request Sep 26, 2022
Closed
@davidbrochart davidbrochart mentioned this pull request Oct 11, 2022
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fcollonval fcollonval fcollonval approved these changes

Assignees

@davidbrochart davidbrochart

Labels

enhancement

Projects

None yet

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@davidbrochart @fcollonval @jtpio