Skip to content

Commit 4d7e8e0

Browse files
SimplicityGuyclaude
andcommitted
fix(tableinator,graphinator): add automatic recovery from stuck consumer state
When consumers die unexpectedly (due to channel issues, connection problems, or database pressure), the services would get stuck in a limbo state where no consumers were active but files weren't marked as completed. The existing recovery logic only triggered when all files were completed. Changes: - Add check_consumers_unexpectedly_dead() to detect stuck state - Add STUCK_CHECK_INTERVAL (default 30s) for frequent stuck state checks - Refactor periodic_queue_checker() to detect and recover from stuck state - Extract recovery logic into _recover_consumers() for reuse - Update get_health_data() to report unhealthy status when stuck - Add tests for stuck state detection and recovery Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent c3d6057 commit 4d7e8e0

4 files changed

Lines changed: 472 additions & 183 deletions

File tree

graphinator/graphinator.py

Lines changed: 191 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@
5858
os.environ.get("QUEUE_CHECK_INTERVAL", "3600")
5959
) # Default 1 hour - how often to check for new messages when connection is closed
6060

61+
# Interval for checking stuck state (consumers died unexpectedly)
62+
STUCK_CHECK_INTERVAL = int(
63+
os.environ.get("STUCK_CHECK_INTERVAL", "30")
64+
) # Default 30 seconds - how often to check for stuck state
65+
6166
# Driver will be initialized in main
6267
graph: AsyncResilientNeo4jDriver | None = None
6368

@@ -95,9 +100,19 @@ def get_health_data() -> dict[str, Any]:
95100
if active_task is None and len(consumer_tags) > 0:
96101
active_task = "Idle - waiting for messages"
97102

103+
# Check for stuck state: no consumers but work remains (files not completed)
104+
no_active_consumers = len(consumer_tags) == 0
105+
files_incomplete = len(completed_files) < len(DATA_TYPES)
106+
has_processed_messages = any(count > 0 for count in message_counts.values())
107+
is_stuck = no_active_consumers and files_incomplete and has_processed_messages
108+
109+
if is_stuck:
110+
active_task = "STUCK - consumers died, awaiting recovery"
111+
98112
# Determine health status:
99113
# - "starting" if graph driver not yet initialized (startup in progress)
100114
# - "unhealthy" if graph was initialized but is now None (connection lost)
115+
# - "unhealthy" if in stuck state (consumers died unexpectedly)
101116
# - "healthy" if graph is initialized and ready
102117
if graph is None:
103118
# Check if we're still in startup (no consumers registered yet)
@@ -106,6 +121,8 @@ def get_health_data() -> dict[str, Any]:
106121
active_task = "Initializing Neo4j connection"
107122
else:
108123
status = "unhealthy"
124+
elif is_stuck:
125+
status = "unhealthy"
109126
else:
110127
status = "healthy"
111128

@@ -116,6 +133,8 @@ def get_health_data() -> dict[str, Any]:
116133
"progress": current_progress,
117134
"message_counts": message_counts.copy(),
118135
"last_message_time": last_message_time.copy(),
136+
"active_consumers": list(consumer_tags.keys()),
137+
"completed_files": list(completed_files),
119138
"timestamp": datetime.now().isoformat(),
120139
}
121140

@@ -236,122 +255,202 @@ async def close_rabbitmq_connection() -> None:
236255

237256

238257
async def check_all_consumers_idle() -> bool:
239-
"""Check if all consumers are cancelled (idle)."""
258+
"""Check if all consumers are cancelled (idle) AND all files completed."""
240259
return len(consumer_tags) == 0 and len(DATA_TYPES) == len(completed_files)
241260

242261

262+
async def check_consumers_unexpectedly_dead() -> bool:
263+
"""Check if consumers have died unexpectedly (no consumers but files not completed).
264+
265+
This detects the stuck state where:
266+
- No consumers are active (consumer_tags is empty)
267+
- Not all files are completed (some work remains)
268+
- We've processed at least some messages (not just starting up)
269+
270+
Returns:
271+
True if consumers appear to have died unexpectedly
272+
"""
273+
no_active_consumers = len(consumer_tags) == 0
274+
files_incomplete = len(completed_files) < len(DATA_TYPES)
275+
has_processed_messages = any(count > 0 for count in message_counts.values())
276+
277+
return no_active_consumers and files_incomplete and has_processed_messages
278+
279+
243280
async def periodic_queue_checker() -> None:
244-
"""Periodically check all queues for pending messages when connection is closed."""
281+
"""Periodically check queue health and recover from stuck states.
282+
283+
This task handles two scenarios:
284+
1. Normal idle state: All files completed, check for new messages periodically
285+
2. Stuck state: Consumers died unexpectedly, need immediate recovery
286+
287+
The stuck state check runs frequently (every STUCK_CHECK_INTERVAL seconds) to
288+
detect and recover quickly. The normal idle check runs at QUEUE_CHECK_INTERVAL.
289+
"""
245290
global active_connection, active_channel, queues, consumer_tags
246291

292+
last_full_check = 0.0 # Track when we last did a full queue check
293+
247294
while not shutdown_requested:
248295
try:
249-
await asyncio.sleep(QUEUE_CHECK_INTERVAL)
296+
await asyncio.sleep(STUCK_CHECK_INTERVAL)
297+
298+
current_time = time.time()
299+
300+
# Check for stuck state (consumers died but work remains)
301+
if await check_consumers_unexpectedly_dead():
302+
logger.warning(
303+
"⚠️ Detected stuck state: consumers died but files not completed. "
304+
"Attempting recovery...",
305+
active_consumers=len(consumer_tags),
306+
completed_files=list(completed_files),
307+
message_counts=message_counts,
308+
)
309+
await _recover_consumers()
310+
continue
311+
312+
# Normal idle check: only run at QUEUE_CHECK_INTERVAL
313+
time_since_last_check = current_time - last_full_check
314+
if time_since_last_check < QUEUE_CHECK_INTERVAL:
315+
continue
250316

251-
# Only check if all consumers are idle and connection is closed
317+
# Only do full queue check if all consumers are idle and connection is closed
252318
if not await check_all_consumers_idle() or active_connection:
253319
continue
254320

321+
last_full_check = current_time
255322
logger.info("🔄 Checking all queues for new messages...")
323+
await _recover_consumers()
256324

257-
# Temporarily connect to check queue depths
258-
temp_connection = await rabbitmq_manager.connect()
259-
temp_channel = await temp_connection.channel()
325+
except asyncio.CancelledError:
326+
logger.info("🛑 Queue checker task cancelled")
327+
break
328+
except Exception as e:
329+
logger.error(f"❌ Error in periodic queue checker: {e}")
330+
# Continue running despite errors
260331

261-
try:
262-
# Check each queue for pending messages
263-
queues_with_messages = []
264-
for data_type in DATA_TYPES:
265-
queue_name = f"{AMQP_QUEUE_PREFIX_GRAPHINATOR}-{data_type}"
266-
267-
# Use queue.declare with passive=True to get message count without affecting the queue
268-
declared_queue = await temp_channel.declare_queue(
269-
name=queue_name, passive=True
270-
)
271332

272-
if declared_queue.declaration_result.message_count > 0:
273-
queues_with_messages.append(
274-
(data_type, declared_queue.declaration_result.message_count)
275-
)
333+
async def _recover_consumers() -> None:
334+
"""Recover consumers by reconnecting to RabbitMQ and restarting consumption.
276335
277-
if queues_with_messages:
278-
logger.info(
279-
f"📬 Found messages in queues, restarting consumers: {queues_with_messages}"
280-
)
336+
This function handles the actual recovery logic for both:
337+
- Normal recovery after idle period
338+
- Emergency recovery after unexpected consumer death
339+
"""
340+
global active_connection, active_channel, queues, consumer_tags
281341

282-
# Re-establish full connection and start consuming
283-
active_connection = temp_connection
284-
active_channel = temp_channel
342+
# Close any existing broken connection first
343+
if active_connection:
344+
try:
345+
await active_connection.close()
346+
except Exception: # nosec: B110
347+
pass
348+
active_connection = None
349+
active_channel = None
285350

286-
# Declare exchange and queues
287-
exchange = await active_channel.declare_exchange(
288-
AMQP_EXCHANGE,
289-
AMQP_EXCHANGE_TYPE,
290-
durable=True,
291-
auto_delete=False,
292-
)
351+
# Temporarily connect to check queue depths
352+
try:
353+
temp_connection = await rabbitmq_manager.connect()
354+
temp_channel = await temp_connection.channel()
355+
except Exception as e:
356+
logger.error(f"❌ Failed to connect to RabbitMQ for recovery: {e}")
357+
return
293358

294-
# Set QoS - must match batch_size for efficient batch processing
295-
await active_channel.set_qos(prefetch_count=200, global_=True)
359+
try:
360+
# Check each queue for pending messages
361+
queues_with_messages = []
362+
for data_type in DATA_TYPES:
363+
queue_name = f"{AMQP_QUEUE_PREFIX_GRAPHINATOR}-{data_type}"
296364

297-
# Declare and bind all queues
298-
queues = {}
299-
for data_type in DATA_TYPES:
300-
queue_name = f"{AMQP_QUEUE_PREFIX_GRAPHINATOR}-{data_type}"
301-
queue = await active_channel.declare_queue(
302-
auto_delete=False, durable=True, name=queue_name
303-
)
304-
await queue.bind(exchange, routing_key=data_type)
305-
queues[data_type] = queue
306-
307-
# Start consumers for queues with messages
308-
for data_type, _ in queues_with_messages:
309-
if data_type in queues and data_type not in consumer_tags:
310-
# Get appropriate handler for this data type
311-
handler = None
312-
if data_type == "artists":
313-
handler = on_artist_message
314-
elif data_type == "labels":
315-
handler = on_label_message
316-
elif data_type == "masters":
317-
handler = on_master_message
318-
elif data_type == "releases":
319-
handler = on_release_message
320-
321-
if handler:
322-
consumer_tag = await queues[data_type].consume(
323-
handler, consumer_tag=f"graphinator-{data_type}"
324-
)
325-
consumer_tags[data_type] = consumer_tag
326-
# Remove from completed files so it will be processed
327-
completed_files.discard(data_type)
328-
last_message_time[data_type] = time.time()
329-
logger.info(f"✅ Started consumer for {data_type}")
365+
# Use queue.declare with passive=True to get message count without affecting the queue
366+
declared_queue = await temp_channel.declare_queue(
367+
name=queue_name, passive=True
368+
)
330369

331-
# Don't close temp_connection since we're using it as active_connection
332-
else:
333-
logger.info(
334-
"📭 No messages in any queue, connection remains closed"
335-
)
336-
# Close the temporary connection
337-
await temp_channel.close()
338-
await temp_connection.close()
370+
if declared_queue.declaration_result.message_count > 0:
371+
queues_with_messages.append(
372+
(data_type, declared_queue.declaration_result.message_count)
373+
)
339374

340-
except Exception as e:
341-
logger.error(f"❌ Error checking queues: {e}")
342-
# Make sure to close temporary connection on error
343-
try:
344-
await temp_channel.close()
345-
await temp_connection.close()
346-
except Exception: # nosec: B110
347-
pass
375+
if queues_with_messages:
376+
total_messages = sum(count for _, count in queues_with_messages)
377+
logger.info(
378+
f"📬 Found messages in queues, restarting consumers: {queues_with_messages} "
379+
f"(total: {total_messages})"
380+
)
348381

349-
except asyncio.CancelledError:
350-
logger.info("🛑 Queue checker task cancelled")
351-
break
352-
except Exception as e:
353-
logger.error(f"❌ Error in periodic queue checker: {e}")
354-
# Continue running despite errors
382+
# Re-establish full connection and start consuming
383+
active_connection = temp_connection
384+
active_channel = temp_channel
385+
386+
# Declare exchange and queues
387+
exchange = await active_channel.declare_exchange(
388+
AMQP_EXCHANGE,
389+
AMQP_EXCHANGE_TYPE,
390+
durable=True,
391+
auto_delete=False,
392+
)
393+
394+
# Set QoS - must match batch_size for efficient batch processing
395+
await active_channel.set_qos(prefetch_count=200, global_=True)
396+
397+
# Declare and bind all queues
398+
queues = {}
399+
for data_type in DATA_TYPES:
400+
queue_name = f"{AMQP_QUEUE_PREFIX_GRAPHINATOR}-{data_type}"
401+
queue = await active_channel.declare_queue(
402+
auto_delete=False, durable=True, name=queue_name
403+
)
404+
await queue.bind(exchange, routing_key=data_type)
405+
queues[data_type] = queue
406+
407+
# Start consumers for queues with messages
408+
for data_type, msg_count in queues_with_messages:
409+
if data_type in queues and data_type not in consumer_tags:
410+
# Get appropriate handler for this data type
411+
handler = None
412+
if data_type == "artists":
413+
handler = on_artist_message
414+
elif data_type == "labels":
415+
handler = on_label_message
416+
elif data_type == "masters":
417+
handler = on_master_message
418+
elif data_type == "releases":
419+
handler = on_release_message
420+
421+
if handler:
422+
consumer_tag = await queues[data_type].consume(
423+
handler, consumer_tag=f"graphinator-{data_type}"
424+
)
425+
consumer_tags[data_type] = consumer_tag
426+
# Remove from completed files so it will be processed
427+
completed_files.discard(data_type)
428+
last_message_time[data_type] = time.time()
429+
logger.info(
430+
f"✅ Started consumer for {data_type} "
431+
f"(pending: {msg_count})"
432+
)
433+
434+
logger.info(
435+
f"✅ Recovery complete - consumers restarted: {list(consumer_tags.keys())}"
436+
)
437+
# Don't close temp_connection since we're using it as active_connection
438+
else:
439+
logger.info(
440+
"📭 No messages in any queue, connection remains closed"
441+
)
442+
# Close the temporary connection
443+
await temp_channel.close()
444+
await temp_connection.close()
445+
446+
except Exception as e:
447+
logger.error(f"❌ Error during consumer recovery: {e}")
448+
# Make sure to close temporary connection on error
449+
try:
450+
await temp_channel.close()
451+
await temp_connection.close()
452+
except Exception: # nosec: B110
453+
pass
355454

356455

357456
async def check_file_completion(

0 commit comments

Comments
 (0)