Gate librarian cog on a full ping round-trip, not a bare GET
The librarian health check was a plain GET to '/', which only proved
Flask was listening - not that the service could actually take a query,
run it through its internal queue+worker, and answer back. So the cog
could load against a librarian whose worker was wedged or that couldn't
reach the bot on the return leg.
Replace it with a ping that travels the SAME path a real search does, on
both sides:
bot: QueryControl -> OUT_COMM_Q -> scan_queue -> awaiting_q
librarian: POST /ping -> librarian_queue -> worker pulls it off
(no Crossref/DOI search) -> pongs back with the same uuid
bot: /conjurer -> incoming_q -> scan_incoming matches uuid, wakes waiter
The cog enables only when that whole loop closes within 3s. This also
proves the librarian->bot return path, which a GET never did.
Safety: uuid is random per ping; the wait and POST are both bounded so
startup can't stall; a pong that finds no waiter is dropped (never
orphaned into IN_COMM_Q, which would make the cog post a bogus 'no
results' message); and a ping whose pong never returns is swept out of
awaiting_q after PING_TTL_SECONDS so nothing leaks. All awaiting_q writes
stay within scan_queue (append) and scan_incoming (remove) - no locks,
no cross-thread mutation.
Integration tests cover: OK round-trip, timeout when accepted-but-no-pong,
unreachable, non-200, orphan-pong-dropped, and that real results still
reach IN_COMM_Q. Suite: 24 integration + 41 unit green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -4,10 +4,12 @@ import os
|
||||
import re
|
||||
import threading
|
||||
import time
|
||||
import uuid as uuidlib
|
||||
from queue import Empty, Queue
|
||||
from typing import Optional
|
||||
from urllib import request as urequest
|
||||
|
||||
import requests
|
||||
from flask import Flask, abort, jsonify, request
|
||||
from waitress import serve
|
||||
|
||||
@@ -26,6 +28,11 @@ SRCHTITLE = re.compile(rb"StreamTitle=\\*(?P<title>[^;]*);").search
|
||||
|
||||
awaiting_q = []
|
||||
incoming_q = Queue()
|
||||
# A health-check ping whose pong never comes back (dead/dropped librarian) would
|
||||
# otherwise leave its record in awaiting_q forever. scan_incoming sweeps ping
|
||||
# records older than this. Kept well above the ping timeout so a slow-but-alive
|
||||
# round-trip is never swept out from under a waiter.
|
||||
PING_TTL_SECONDS = 30
|
||||
app = Flask(__name__)
|
||||
|
||||
|
||||
@@ -232,9 +239,36 @@ def scan_incoming(stop_event: Optional[threading.Event] = None):
|
||||
if stop_event and stop_event.is_set():
|
||||
logger.info("scan_incoming: stop requested")
|
||||
break
|
||||
# Sweep stale health-check pings first: if a librarian is dead the pong
|
||||
# never arrives, so drop ping records past their TTL. scan_incoming is
|
||||
# the sole remover of awaiting_q, so this needs no lock (scan_queue only
|
||||
# appends). Snapshot with list() so removal during iteration is safe.
|
||||
now = time.monotonic()
|
||||
for record in list(awaiting_q):
|
||||
if (
|
||||
getattr(record, "is_ping", False)
|
||||
and getattr(record, "answered", None) is not None
|
||||
and not record.answered.is_set()
|
||||
and now - getattr(record, "created", now) > PING_TTL_SECONDS
|
||||
):
|
||||
awaiting_q.remove(record)
|
||||
try:
|
||||
answer = incoming_q.get(block=False)
|
||||
logger.info("DATA FOUND")
|
||||
# Health-check pong (shape: {"__pong__": uuid}). Close the waiter's
|
||||
# event and drop its record. It must NEVER fall through to the
|
||||
# result/orphan path below, or the librarian cog would later pull it
|
||||
# off IN_COMM_Q and post a bogus "no results" message to Discord.
|
||||
if isinstance(answer, dict) and "__pong__" in answer:
|
||||
pong_uuid = answer["__pong__"]
|
||||
for record in list(awaiting_q):
|
||||
if getattr(record, "uuid", None) == pong_uuid:
|
||||
event = getattr(record, "answered", None)
|
||||
if event is not None:
|
||||
event.set()
|
||||
awaiting_q.remove(record)
|
||||
logger.info("PONG matched for %s", pong_uuid)
|
||||
continue
|
||||
# Collect matched records and drop them from awaiting_q afterwards -
|
||||
# they used to stay forever (awaiting_q only ever grew), leaking
|
||||
# memory over the bot's uptime and letting a reused UUID re-match a
|
||||
@@ -258,6 +292,55 @@ def scan_incoming(stop_event: Optional[threading.Event] = None):
|
||||
time.sleep(1)
|
||||
|
||||
|
||||
def librarian_ping(address: str, endpoint: str, headers: Optional[dict] = None,
|
||||
timeout: float = 3.0) -> bool:
|
||||
"""Health-check the librarian by round-tripping a ping through the FULL path.
|
||||
|
||||
The ping is a pseudo-query that exercises exactly the same machinery a real
|
||||
search does, on BOTH sides:
|
||||
|
||||
* bot out: a ``QueryControl`` rides ``OUT_COMM_Q`` -> ``scan_queue`` ->
|
||||
``awaiting_q`` just like a real query,
|
||||
* librarian: it is POSTed to the librarian, which must pull it off its OWN
|
||||
internal queue and answer WITHOUT running a search,
|
||||
* bot in: the pong comes back over ``/conjurer`` -> ``incoming_q`` ->
|
||||
``scan_incoming``, which matches it by uuid and sets our event.
|
||||
|
||||
Returns True only when that whole loop closes within ``timeout``. Never
|
||||
blocks longer than roughly ``timeout`` and cannot deadlock: the POST is
|
||||
bounded, the wait is bounded, and a ping whose pong never arrives is swept
|
||||
out of ``awaiting_q`` by ``scan_incoming`` (PING_TTL_SECONDS).
|
||||
"""
|
||||
logger = logging.getLogger("discord")
|
||||
ping_uuid = str(uuidlib.uuid4())
|
||||
answered = threading.Event()
|
||||
query = QueryControl("healthcheck", ping_uuid, "__ping__", None)
|
||||
query.is_ping = True
|
||||
query.answered = answered
|
||||
query.created = time.monotonic()
|
||||
# Enter the bot-side comm queue BEFORE the POST, so the record is already in
|
||||
# awaiting_q by the time the pong can come back (no lost-wakeup race).
|
||||
OUT_COMM_Q.put(query)
|
||||
try:
|
||||
response = requests.post(
|
||||
f"{address}{endpoint}",
|
||||
json={"UUID": ping_uuid},
|
||||
headers=headers or {},
|
||||
timeout=timeout,
|
||||
)
|
||||
except requests.exceptions.RequestException as exc:
|
||||
logger.info("Librarian ping POST failed (%s): %s", ping_uuid, exc)
|
||||
return False # stale record is swept by scan_incoming
|
||||
if response.status_code != 200:
|
||||
logger.info("Librarian ping rejected (%s): HTTP %s", ping_uuid, response.status_code)
|
||||
return False
|
||||
if answered.wait(timeout):
|
||||
logger.info("Librarian ping round-trip OK (%s)", ping_uuid)
|
||||
return True
|
||||
logger.info("Librarian ping timed out after %ss (%s)", timeout, ping_uuid)
|
||||
return False
|
||||
|
||||
|
||||
def get_stream_title(tag: bytes) -> str:
|
||||
title = ""
|
||||
if m := SRCHTITLE(tag):
|
||||
|
||||
Reference in New Issue
Block a user