Keep notification acknowledgements responsive under snapshot contention #440
|
|
@ -237,6 +237,12 @@ cannot suppress different content. The browser sends its known tokens on later p
|
||||||
`/api/v1/live` can omit unchanged section bodies while still returning current freshness and
|
`/api/v1/live` can omit unchanged section bodies while still returning current freshness and
|
||||||
retry metadata. The client retains omitted data and only rebuilds or persists the sections
|
retry metadata. The client retains omitted data and only rebuilds or persists the sections
|
||||||
that changed. Malformed or oversized tokens are rejected before any upstream work.
|
that changed. Malformed or oversized tokens are rejected before any upstream work.
|
||||||
|
Confirmed notification acknowledgements update this worker's live view immediately, then
|
||||||
|
maintain the shared snapshot outside the async request loop. Bulk acknowledgements remove
|
||||||
|
all successful notification IDs in one SQLite transaction and advance the notification
|
||||||
|
revision once. If shared-store lock admission fails after Gitea confirms the mutation, the
|
||||||
|
API keeps the confirmed result and process-local filtering rather than falsely reporting the
|
||||||
|
upstream write as failed; a later shared refresh reconciles the cache.
|
||||||
|
|
||||||
## Offline mobile shell
|
## Offline mobile shell
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -217,13 +217,16 @@ class LiveSnapshotStore:
|
||||||
connection.commit()
|
connection.commit()
|
||||||
return self.load()
|
return self.load()
|
||||||
|
|
||||||
def remove_notification(self, notification_id: int) -> LiveSnapshotState:
|
def remove_notifications(self, notification_ids: Iterable[int]) -> LiveSnapshotState:
|
||||||
"""Filter a confirmed read from every worker and subsequent stale refresh."""
|
"""Filter confirmed reads from every worker and subsequent stale refreshes."""
|
||||||
|
read_ids = set(notification_ids)
|
||||||
|
if not read_ids:
|
||||||
|
return self.load()
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
connection.execute("BEGIN IMMEDIATE")
|
connection.execute("BEGIN IMMEDIATE")
|
||||||
connection.execute(
|
connection.executemany(
|
||||||
"INSERT OR IGNORE INTO live_read_notification VALUES (?)",
|
"INSERT OR IGNORE INTO live_read_notification VALUES (?)",
|
||||||
(notification_id,),
|
((notification_id,) for notification_id in read_ids),
|
||||||
)
|
)
|
||||||
row = connection.execute(
|
row = connection.execute(
|
||||||
"SELECT value_json, revisions_json FROM live_snapshot WHERE singleton = 1"
|
"SELECT value_json, revisions_json FROM live_snapshot WHERE singleton = 1"
|
||||||
|
|
@ -233,8 +236,9 @@ class LiveSnapshotStore:
|
||||||
if value is not None and isinstance(value.get("notifications"), list):
|
if value is not None and isinstance(value.get("notifications"), list):
|
||||||
previous = value["notifications"]
|
previous = value["notifications"]
|
||||||
retained = [
|
retained = [
|
||||||
item for item in previous
|
item
|
||||||
if not isinstance(item, dict) or item.get("id") != notification_id
|
for item in previous
|
||||||
|
if not isinstance(item, dict) or item.get("id") not in read_ids
|
||||||
]
|
]
|
||||||
if retained != previous:
|
if retained != previous:
|
||||||
value["notifications"] = retained
|
value["notifications"] = retained
|
||||||
|
|
@ -249,6 +253,10 @@ class LiveSnapshotStore:
|
||||||
connection.commit()
|
connection.commit()
|
||||||
return self.load()
|
return self.load()
|
||||||
|
|
||||||
|
def remove_notification(self, notification_id: int) -> LiveSnapshotState:
|
||||||
|
"""Filter one confirmed read from every worker and subsequent stale refresh."""
|
||||||
|
return self.remove_notifications([notification_id])
|
||||||
|
|
||||||
def release_refresh(self, owner: str) -> None:
|
def release_refresh(self, owner: str) -> None:
|
||||||
"""Relinquish a lease that became unnecessary after a coherent recheck."""
|
"""Relinquish a lease that became unnecessary after a coherent recheck."""
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
|
|
|
||||||
32
src/main.py
32
src/main.py
|
|
@ -1735,30 +1735,30 @@ def _live_snapshot_payload(
|
||||||
return payload
|
return payload
|
||||||
|
|
||||||
|
|
||||||
def _remove_notification_from_live_snapshot(thread_id: int) -> None:
|
async def _remove_notifications_from_live_snapshot(thread_ids: list[int]) -> None:
|
||||||
global _live_snapshot_value, _read_notification_ids
|
global _live_snapshot_value, _read_notification_ids
|
||||||
_read_notification_ids = _read_notification_ids | {thread_id}
|
read_ids = set(thread_ids)
|
||||||
try:
|
if not read_ids:
|
||||||
shared = _live_snapshot_store.remove_notification(thread_id)
|
|
||||||
if shared.value is not None or _live_snapshot_value is None:
|
|
||||||
_apply_shared_live_state(shared)
|
|
||||||
except (OSError, sqlite3.Error):
|
|
||||||
# The upstream mutation already succeeded; keep process-local filtering.
|
|
||||||
pass
|
|
||||||
if _live_snapshot_value is None:
|
|
||||||
return
|
return
|
||||||
|
_read_notification_ids = _read_notification_ids | read_ids
|
||||||
|
if _live_snapshot_value is not None:
|
||||||
retained_notifications = _live_snapshot_value.get("notifications")
|
retained_notifications = _live_snapshot_value.get("notifications")
|
||||||
if not isinstance(retained_notifications, list):
|
if isinstance(retained_notifications, list):
|
||||||
return
|
|
||||||
updated = dict(_live_snapshot_value)
|
updated = dict(_live_snapshot_value)
|
||||||
updated["notifications"] = [
|
updated["notifications"] = [
|
||||||
notification
|
notification
|
||||||
for notification in retained_notifications
|
for notification in retained_notifications
|
||||||
if not isinstance(notification, dict) or notification.get("id") != thread_id
|
if not isinstance(notification, dict)
|
||||||
|
or notification.get("id") not in read_ids
|
||||||
]
|
]
|
||||||
if updated["notifications"] != retained_notifications:
|
if updated["notifications"] != retained_notifications:
|
||||||
_live_section_revisions["notifications"] += 1
|
_live_section_revisions["notifications"] += 1
|
||||||
_live_snapshot_value = updated
|
_live_snapshot_value = updated
|
||||||
|
try:
|
||||||
|
await asyncio.to_thread(_live_snapshot_store.remove_notifications, read_ids)
|
||||||
|
except (OSError, sqlite3.Error):
|
||||||
|
# The upstream mutation already succeeded; keep process-local filtering.
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
def _without_read_notifications(snapshot: dict) -> dict:
|
def _without_read_notifications(snapshot: dict) -> dict:
|
||||||
|
|
@ -1986,7 +1986,6 @@ async def _mark_notification_read_result(thread_id: int) -> tuple[int, bool]:
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
return thread_id, False
|
return thread_id, False
|
||||||
_remove_notification_from_live_snapshot(thread_id)
|
|
||||||
return thread_id, True
|
return thread_id, True
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -2016,6 +2015,9 @@ async def read_notifications(batch: NotificationReadBatch) -> JSONResponse:
|
||||||
for thread_id, succeeded in [task.result()]
|
for thread_id, succeeded in [task.result()]
|
||||||
if succeeded
|
if succeeded
|
||||||
}
|
}
|
||||||
|
await _remove_notifications_from_live_snapshot(
|
||||||
|
[thread_id for thread_id in thread_ids if thread_id in succeeded_ids]
|
||||||
|
)
|
||||||
failed = [thread_id for thread_id in thread_ids if thread_id not in succeeded_ids]
|
failed = [thread_id for thread_id in thread_ids if thread_id not in succeeded_ids]
|
||||||
return JSONResponse(
|
return JSONResponse(
|
||||||
{
|
{
|
||||||
|
|
@ -2116,7 +2118,7 @@ async def read_notification(thread_id: int = PathParam(gt=0)) -> JSONResponse:
|
||||||
{"error": "The update could not be marked read. Please retry."},
|
{"error": "The update could not be marked read. Please retry."},
|
||||||
status_code=503,
|
status_code=503,
|
||||||
)
|
)
|
||||||
_remove_notification_from_live_snapshot(thread_id)
|
await _remove_notifications_from_live_snapshot([thread_id])
|
||||||
return JSONResponse({"id": thread_id, "status": "read"})
|
return JSONResponse({"id": thread_id, "status": "read"})
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -102,6 +102,29 @@ def test_store_is_private_and_does_not_persist_upstream_token(tmp_path, monkeypa
|
||||||
assert token.encode() not in path.read_bytes()
|
assert token.encode() not in path.read_bytes()
|
||||||
|
|
||||||
|
|
||||||
|
def test_remove_notifications_filters_batch_with_one_revision_advance(tmp_path):
|
||||||
|
store = LiveSnapshotStore(tmp_path / "live.sqlite3", clock=lambda: 100.0)
|
||||||
|
lease = store.try_acquire_refresh({"notifications"}, lease_seconds=5)
|
||||||
|
assert lease is not None
|
||||||
|
metadata = {section: None for section in ("context", "events", "notifications")}
|
||||||
|
published = store.publish_refresh(
|
||||||
|
lease,
|
||||||
|
value={
|
||||||
|
"notifications": [{"id": 7}, {"id": 8}, {"id": 9}],
|
||||||
|
"sections": {"notifications": "fresh"},
|
||||||
|
},
|
||||||
|
created_at=metadata,
|
||||||
|
failure_count={section: 0 for section in metadata},
|
||||||
|
retry_at=metadata,
|
||||||
|
changed_sections={"notifications"},
|
||||||
|
)
|
||||||
|
|
||||||
|
removed = store.remove_notifications([7, 9, 7])
|
||||||
|
|
||||||
|
assert removed.value["notifications"] == [{"id": 8}]
|
||||||
|
assert removed.revisions["notifications"] == published.revisions["notifications"] + 1
|
||||||
|
|
||||||
|
|
||||||
def test_read_notification_filter_is_shared_with_future_publications(tmp_path):
|
def test_read_notification_filter_is_shared_with_future_publications(tmp_path):
|
||||||
path = tmp_path / "live.sqlite3"
|
path = tmp_path / "live.sqlite3"
|
||||||
first = LiveSnapshotStore(path, clock=lambda: 100.0)
|
first = LiveSnapshotStore(path, clock=lambda: 100.0)
|
||||||
|
|
|
||||||
|
|
@ -61,6 +61,37 @@ async def test_mark_notification_read_api_is_bounded_and_never_cacheable(monkeyp
|
||||||
assert marked == [42]
|
assert marked == [42]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_snapshot_maintenance_does_not_block_the_event_loop(monkeypatch):
|
||||||
|
async def mark(_thread_id):
|
||||||
|
return None
|
||||||
|
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
class BlockingStore:
|
||||||
|
def remove_notification(self, thread_id):
|
||||||
|
calls.append([thread_id])
|
||||||
|
time.sleep(0.08)
|
||||||
|
|
||||||
|
def remove_notifications(self, thread_ids):
|
||||||
|
calls.append(list(thread_ids))
|
||||||
|
time.sleep(0.08)
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "mark_notification_read", mark)
|
||||||
|
monkeypatch.setattr(main, "_live_snapshot_store", BlockingStore())
|
||||||
|
monkeypatch.setattr(main, "_live_snapshot_value", None)
|
||||||
|
started_at = asyncio.get_running_loop().time()
|
||||||
|
|
||||||
|
request = asyncio.create_task(main.read_notification(42))
|
||||||
|
await asyncio.sleep(0.01)
|
||||||
|
heartbeat_delay = asyncio.get_running_loop().time() - started_at
|
||||||
|
response = await request
|
||||||
|
|
||||||
|
assert heartbeat_delay < 0.05
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert calls == [[42]]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_bulk_mark_read_reports_partial_progress_and_retains_only_failures(monkeypatch):
|
async def test_bulk_mark_read_reports_partial_progress_and_retains_only_failures(monkeypatch):
|
||||||
calls = []
|
calls = []
|
||||||
|
|
@ -89,6 +120,33 @@ async def test_bulk_mark_read_reports_partial_progress_and_retains_only_failures
|
||||||
assert main._live_snapshot_value == {"notifications": [{"id": 43}]}
|
assert main._live_snapshot_value == {"notifications": [{"id": 43}]}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_bulk_mark_read_updates_shared_snapshot_once_for_successes(monkeypatch):
|
||||||
|
async def mark(thread_id):
|
||||||
|
if thread_id == 43:
|
||||||
|
raise httpx.HTTPError("upstream unavailable")
|
||||||
|
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
class RecordingStore:
|
||||||
|
def remove_notifications(self, thread_ids):
|
||||||
|
calls.append(tuple(sorted(thread_ids)))
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "mark_notification_read", mark)
|
||||||
|
monkeypatch.setattr(main, "_live_snapshot_store", RecordingStore())
|
||||||
|
monkeypatch.setattr(
|
||||||
|
main,
|
||||||
|
"_live_snapshot_value",
|
||||||
|
{"notifications": [{"id": 42}, {"id": 43}, {"id": 44}]},
|
||||||
|
)
|
||||||
|
|
||||||
|
response = await main.read_notifications(main.NotificationReadBatch(ids=[42, 43, 44]))
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert calls == [(42, 44)]
|
||||||
|
assert main._live_snapshot_value == {"notifications": [{"id": 43}]}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_bulk_mark_read_limits_upstream_concurrency(monkeypatch):
|
async def test_bulk_mark_read_limits_upstream_concurrency(monkeypatch):
|
||||||
active = 0
|
active = 0
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user