Skip to content

Commit 70aceb9

Browse files
committed
restart inactive rooms instead of deleting
1 parent 37b8df0 commit 70aceb9

File tree

3 files changed

+62
-26
lines changed

3 files changed

+62
-26
lines changed

jupyter_server_documents/rooms/yroom.py

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,17 @@ class YRoom:
7676
_ydoc_subscription: pycrdt.Subscription
7777
"""Subscription to YDoc changes."""
7878

79-
_stopped: bool = False
79+
_stopped: bool
8080
"""
8181
Whether the YRoom is stopped. Set to `True` when `stop()` is called and set
8282
to `False` when `restart()` is called.
8383
"""
8484

85+
_updated: bool
86+
"""
87+
See `self.updated` for more info.
88+
"""
89+
8590
_fileid_manager: BaseFileIdManager
8691
_contents_manager: AsyncContentsManager | ContentsManager
8792

@@ -103,6 +108,7 @@ def __init__(
103108
self._fileid_manager = fileid_manager
104109
self._contents_manager = contents_manager
105110
self._stopped = False
111+
self._updated = False
106112

107113
# Initialize YjsClientGroup, YDoc, and Awareness
108114
self._client_group = YjsClientGroup(room_id=room_id, log=self.log, loop=self._loop)
@@ -511,8 +517,9 @@ def _on_jupyter_ydoc_update(self, updated_key: str, event: Any) -> None:
511517
map_event = cast(pycrdt.MapEvent, event)
512518
if should_ignore_state_update(map_event):
513519
return
514-
515-
# Otherwise, save the file
520+
521+
# Otherwise, a change was made. Set `updated=True` and save the file
522+
self._updated = True
516523
self.file_api.schedule_save()
517524

518525

@@ -663,6 +670,8 @@ def stop(self, close_code: int = 1001, immediately: bool = False):
663670
- Clears the YDoc, Awareness, and JupyterYDoc, freeing their memory to
664671
the server. This deletes the YDoc history.
665672
"""
673+
self.log.info(f"Stopping YRoom '{self.room_id}'.")
674+
666675
# Disconnect all clients with the given close code
667676
self.clients.stop(close_code=close_code)
668677

@@ -734,6 +743,17 @@ def stopped(self) -> bool:
734743
"""
735744
return self._stopped
736745

746+
@property
747+
def updated(self) -> bool:
748+
"""
749+
Returns whether the room has been updated since the last restart, or
750+
since initialization if the room was not restarted.
751+
752+
This initializes to `False` and is set to `True` whenever a meaningful
753+
update that needs to be saved occurs. This is reset to `False` when
754+
`restart()` is called.
755+
"""
756+
return self._updated
737757

738758
def restart(self, close_code: int = 1001, immediately: bool = False):
739759
"""
@@ -745,10 +765,13 @@ def restart(self, close_code: int = 1001, immediately: bool = False):
745765
immediately)` with the given arguments. Otherwise, `close_code` and
746766
`immediately` are ignored.
747767
"""
748-
# Stop if not stopped already, then reset `stopped` state
768+
# Stop if not stopped already
749769
if not self._stopped:
750770
self.stop(close_code=close_code, immediately=immediately)
771+
772+
# Reset internal state
751773
self._stopped = False
774+
self._updated = False
752775

753776
# Restart client group
754777
self.clients.restart()
@@ -759,6 +782,8 @@ def restart(self, close_code: int = 1001, immediately: bool = False):
759782

760783
# Restart `_process_message_queue()` task
761784
self._loop.create_task(self._process_message_queue())
785+
786+
self.log.info(f"Restarted YRoom '{self.room_id}'.")
762787

763788

764789
def should_ignore_state_update(event: pycrdt.MapEvent) -> bool:

jupyter_server_documents/rooms/yroom_file_api.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,6 @@ def stop(self) -> None:
396396
To save the YDoc after stopping, call `await
397397
file_api.save_immediately()` after calling this method.
398398
"""
399-
self.log.info(f"Stopping FileAPI for room '{self.room_id}'.")
400399
if self._watch_file_task:
401400
self._watch_file_task.cancel()
402401
self._stopped = True

jupyter_server_documents/rooms/yroom_manager.py

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,17 @@
1212

1313
class YRoomManager():
1414
"""
15-
A singleton that manages all `YRoom` instances in the server extension. This
16-
automatically stops `YRoom` instances if they have had no connected clients
17-
or active kernel for >10 seconds.
15+
A singleton that manages all `YRoom` instances in the server extension.
16+
17+
This manager automatically restarts updated `YRoom` instances if they have
18+
had no connected clients or active kernel for >10 seconds. This deletes the
19+
YDoc history to free its memory to the server.
1820
"""
1921

2022
_rooms_by_id: dict[str, YRoom]
2123
"""
2224
Dictionary of active `YRoom` instances, keyed by room ID. Rooms are never
23-
deleted from this dictionary, even if stopped due to inactivity.
25+
deleted from this dictionary.
2426
2527
TODO: Delete a room if its file was deleted in/out-of-band or moved
2628
out-of-band. See #116.
@@ -30,8 +32,7 @@ class YRoomManager():
3032
"""
3133
Set of room IDs that were marked inactive on the last iteration of
3234
`_watch_rooms()`. If a room is inactive and its ID is present in this set,
33-
then the room the room should be stopped as it has been inactive for >10
34-
seconds.
35+
then the room should be restarted as it has been inactive for >10 seconds.
3536
"""
3637

3738
_get_fileid_manager: callable[[], BaseFileIdManager]
@@ -79,8 +80,7 @@ def get_room(self, room_id: str) -> YRoom | None:
7980
not exist, this method will initialize one and return it. Otherwise,
8081
this method returns the instance from its cache.
8182
"""
82-
# First, ensure this room stays open for >10 seconds by removing it from
83-
# the inactive set of rooms if it is present.
83+
# First, ensure the room is not considered inactive.
8484
self._inactive_rooms.discard(room_id)
8585

8686
# If room exists, return the room
@@ -144,14 +144,16 @@ def delete_room(self, room_id: str) -> None:
144144
async def _watch_rooms(self) -> None:
145145
"""
146146
Background task that checks all `YRoom` instances every 10 seconds,
147-
stopping any rooms that have been inactive for >10 seconds.
147+
restarting any updated rooms that have been inactive for >10 seconds.
148+
This frees the memory occupied by the room's YDoc history, discarding it
149+
in the process.
148150
149-
- For rooms providing notebooks: This task stops the room if it has no
150-
connected clients and its kernel execution status is either 'idle' or
151-
'dead'.
151+
- For rooms providing notebooks: This task restarts the room if it has
152+
been updated, has no connected clients, and its kernel execution status
153+
is either 'idle' or 'dead'.
152154
153-
- For all other rooms: This task stops the room if it has no connected
154-
clients.
155+
- For all other rooms: This task restarts the room if it has been
156+
updated and has no connected clients.
155157
"""
156158
while True:
157159
# Check every 10 seconds
@@ -161,7 +163,7 @@ async def _watch_rooms(self) -> None:
161163
room_ids = set(self._rooms_by_id.keys())
162164
room_ids.discard("JupyterLab:globalAwareness")
163165

164-
# Iterate through all rooms. If any rooms are empty, stop the room.
166+
# Check all rooms and restart it if inactive for >10 seconds.
165167
for room_id in room_ids:
166168
self._check_room(room_id)
167169

@@ -170,12 +172,15 @@ def _check_room(self, room_id: str) -> None:
170172
"""
171173
Checks a room for inactivity.
172174
175+
- Rooms that have not been updated are not restarted, as there is no
176+
YDoc history to free.
177+
173178
- If a room is inactive and not in `_inactive_rooms`, this method adds
174179
the room to `_inactive_rooms`.
175180
176181
- If a room is inactive and is listed in `_inactive_rooms`, this method
177-
stops the room, as it has been inactive for 2 consecutive iterations of
178-
`_watch_rooms()`.
182+
restarts the room, as it has been inactive for 2 consecutive iterations
183+
of `_watch_rooms()`.
179184
"""
180185
# Do nothing if the room has any connected clients.
181186
room = self._rooms_by_id[room_id]
@@ -192,15 +197,22 @@ def _check_room(self, room_id: str) -> None:
192197
if execution_state not in { "idle", "dead", None }:
193198
self._inactive_rooms.discard(room_id)
194199
return
200+
201+
# Do nothing if the room has not been updated. This prevents empty rooms
202+
# from being restarted every 10 seconds.
203+
if not room.updated:
204+
self._inactive_rooms.discard(room_id)
205+
return
195206

196-
# The room is inactive if this line is reached.
197-
# Stop the room if was marked as inactive in the last iteration,
207+
# The room is updated (with history) & inactive if this line is reached.
208+
# Restart the room if was marked as inactive in the last iteration,
198209
# otherwise mark it as inactive.
199210
if room_id in self._inactive_rooms:
200211
self.log.info(
201-
f"YRoom '{room_id}' has been inactive for >10 seconds. "
212+
f"Room '{room_id}' has been inactive for >10 seconds. "
213+
"Restarting the room to free memory occupied by its history."
202214
)
203-
room.stop()
215+
room.restart()
204216
self._inactive_rooms.discard(room_id)
205217
else:
206218
self._inactive_rooms.add(room_id)

0 commit comments

Comments
 (0)