Skip to content

Commit 90000dc

Browse files
committed
fix(gossip): isolate daemon failures and cap requeue with capacity
Per-message try/except so one failing message does not discard unprocessed siblings or prior children. Requeue children while holding the lock and reserve capacity against concurrent callers. Refs #2248
1 parent 4f969b2 commit 90000dc

2 files changed

Lines changed: 38 additions & 22 deletions

File tree

mempalace/gossip.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ def run_once(
785785
}
786786
children: list[GossipMessage] = []
787787

788-
for idx, message in enumerate(pending):
788+
for message in pending:
789789
if message.is_expired():
790790
report["expired"] += 1
791791
continue
@@ -800,17 +800,16 @@ def run_once(
800800
room_graph=room_graph,
801801
kg_path=kg_path,
802802
)
803-
except Exception as exc:
803+
except Exception:
804804
logger.warning(
805-
"gossip-daemon: message %s failed; re-queueing remaining pending messages",
806-
idx,
805+
"gossip-daemon: message %s failed; moving to retry and continuing",
806+
message.subject,
807807
exc_info=True,
808808
)
809809
report["failed"] += 1
810-
# Re-queue this and all remaining pending messages.
811-
with self._lock:
812-
self._queue.extend(pending[idx:])
813-
return report
810+
# Isolated failure: do not requeue the failed message, do not
811+
# discard children already produced or pending messages.
812+
continue
814813

815814
report["processed"] += 1
816815
report["triples_written"] += node_report["triples_written"]

tests/test_gossip.py

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -545,46 +545,63 @@ def test_daemon_run_once_isolates_message_failures():
545545
daemon.schedule("will", "explode", "now", source_wing="orkid", priority="high")
546546
daemon.schedule("launch", "is", "active", source_wing="orkid", priority="high")
547547

548-
# Make the second message's propagation raise.
549548
original = protocol._propagate_message
549+
calls = {"count": 0}
550550

551551
def _boom_on_second(*a, **kw):
552-
raise RuntimeError("simulated failure")
552+
calls["count"] += 1
553+
if calls["count"] == 2:
554+
raise RuntimeError("simulated failure")
555+
return original(*a, **kw)
553556

554557
protocol._propagate_message = _boom_on_second
555558

556559
report = daemon.run_once()
557560

558-
assert report["processed"] == 1
561+
assert report["processed"] == 2
559562
assert report["failed"] == 1
560-
# The queue must still contain the third message, and possibly a child
561-
# from the first message (no test patches room graph, so children may
562-
# also be requeued).
563+
# The failed message is not requeued; the third message is processed and
564+
# may produce children, and the first message's children are preserved.
563565
with daemon._lock:
564566
assert len(daemon._queue) >= 1
565-
ids = {m.subject for m in daemon._queue}
566-
assert "launch" in ids
567+
# The failed subject should not be in the queue.
568+
assert "will" not in {m.subject for m in daemon._queue}
567569
finally:
568570
Path(kg_path).unlink(missing_ok=True)
569571

570572

571573
def test_daemon_requeue_respects_max_requeue():
572-
"""Children are dropped once the queue reaches max_requeue."""
574+
"""Children are dropped once the queue reaches max_requeue, accounting for
575+
entries added by concurrent callers while run_once() is processing."""
573576
kg_path = _temp_db()
574577
try:
575578
kg = KnowledgeGraph(db_path=kg_path)
576579
protocol = GossipProtocol(kg=kg, config=EXAMPLE_GOSSIP_CONFIG)
577-
# Pre-fill the queue to leave only one slot.
578580
daemon = GossipDaemon(protocol, interval_seconds=60.0, max_requeue=2)
579-
daemon.schedule("pre-existing", "is", "queued", source_wing="orkid")
580-
daemon.schedule("audit", "found", "risk", source_wing="orkid", priority="high")
581581

582+
# Simulate a concurrent schedule that occurs while run_once() processes
583+
# the first message. The scheduled message is not part of the current
584+
# pending batch, so at requeue time it occupies one slot.
585+
original = protocol._propagate_message
586+
scheduled = {"done": False}
587+
588+
def _schedule_concurrent(*a, **kw):
589+
if not scheduled["done"]:
590+
daemon.schedule("concurrent", "is", "active", source_wing="orkid")
591+
scheduled["done"] = True
592+
return original(*a, **kw)
593+
594+
protocol._propagate_message = _schedule_concurrent
595+
596+
daemon.schedule("audit", "found", "risk", source_wing="orkid", priority="high")
582597
report = daemon.run_once()
583598

584-
assert report["processed"] == 2
585-
# At most one child can be enqueued because one slot is already taken.
599+
assert report["processed"] == 1
586600
with daemon._lock:
601+
# concurrent message + at most one child == at most 2.
587602
assert len(daemon._queue) <= 2
603+
assert len(daemon._queue) >= 1
604+
# With max_requeue=2 and one concurrent message, only one child fits.
588605
assert report["children_queued"] <= 1
589606
finally:
590607
Path(kg_path).unlink(missing_ok=True)

0 commit comments

Comments
 (0)