Implement service bus monitoring with heartbeat detection and automatic recovery - #4601
Implement service bus monitoring with heartbeat detection and automatic recovery#4601Marcus Robinson (marrobi) with Copilot wants to merge 60 commits into
Conversation
…tions Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
|
Copilot can the supervisor be moved into |
Unit Test Results780 tests 780 ✅ 10s ⏱️ Results for commit bf633fd. ♻️ This comment has been updated with latest results. |
|
Copilot can you add something in the supervisor function to check that there is still a heartbeat. |
…ection Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Added heartbeat monitoring to the supervisor function. Each worker process now updates a heartbeat file every iteration, and the supervisor checks these timestamps to detect stuck processes. Stale processes (no heartbeat update for 5 minutes) are automatically terminated and restarted. This ensures the system can recover from processes that are alive but not actively working. Changes include:
Commit: c7072b9 |
|
Copilot this needs to be for deployment status updater, ion the API, not the runner. |
…s updater Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Moved the heartbeat monitoring from the resource processor to the deployment status updater in the API as requested. The implementation now includes:
Commit: 202e726 |
|
Copilot fix the linting issues:
/github/workspace/api_app/service_bus/deployment_status_updater.py:57:1: W293 blank line contains whitespace
|
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Fixed all linting issues by removing trailing whitespace from blank lines in both deployment_status_updater.py and test_deployment_status_update.py. Also incremented the API version from 0.24.3 to 0.24.4 as requested. Commit: 381bd9c |
|
/test-extended 381bd9c |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/15887800326 (with refid (in response to this comment from Marcus Robinson (@marrobi)) |
…/JC-wk/AzureTRE into fix-service-bus-client-lifecycle
…remove duplicate changelog header
…-4464 # Conflicts: # api_app/_version.py # api_app/service_bus/airlock_request_status_update.py # api_app/service_bus/deployment_status_updater.py Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| async with get_credential_async_context(): | ||
| raise RuntimeError("Test Error") | ||
|
|
||
| mock_credential.close.assert_awaited_once() |
There was a problem hiding this comment.
🟡 Changes recommended
Idle consumers can be falsely marked stale, and supervisor lifecycle and recovery timing do not yet meet the intended behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 6
- Review effort level: Balanced
| asyncio.create_task(deploymentStatusUpdater.supervisor_with_heartbeat_check()) | ||
| asyncio.create_task(airlockStatusUpdater.supervisor_with_heartbeat_check()) | ||
| yield |
There was a problem hiding this comment.
Marcus Robinson (@marrobi) PR #4924 also addresses this
| async for msg in receiver: | ||
| complete_message = await self.process_message(msg) |
| self.update_heartbeat() | ||
| task_just_started = True | ||
|
|
||
| await asyncio.sleep(HEARTBEAT_CHECK_INTERVAL_SECONDS) |
| BUG FIXES: | ||
| * Implement service bus consumer monitoring with heartbeat detection, automatic recovery, and /health endpoint integration to prevent operations getting stuck indefinitely ([#4464](https://github.com/microsoft/AzureTRE/issues/4464)) |
|
Copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Addressed all review-thread comments in commit 883cdaf. Targeted service-bus tests and flake8 pass; CodeQL found no alerts. |
Resolves #4464
What is being addressed
VM operations in the TRE UI were getting stuck in "awaiting_action" status indefinitely. The resource processor completed operations successfully (VMs started/stopped in Azure), but the API never received status updates, leaving operations stuck in the UI.
Root cause: Service bus consumers (
DeploymentStatusUpdaterandAirlockStatusUpdater) could fail silently or hang without detection or recovery mechanisms.How is this addressed
Implemented a comprehensive monitoring and auto-recovery system with dual detection:
Key Features
ServiceBusConsumer Base Class: Provides heartbeat monitoring and supervisor functionality for all service bus consumers.
Dual Monitoring:
receive_messages()tasks fail or completeAutomatic Recovery:
Implementation Details
/healthendpointFiles Changed
CHANGELOG.md- Added entry in BUG FIXES sectionapi_app/_version.py- Incremented to 0.26.1service_bus/service_bus_consumer.py- New base class with monitoring (follows PEP 8 import conventions)service_bus/deployment_status_updater.py- Inherits from base classservice_bus/airlock_request_status_update.py- Same integrationmain.py- Uses supervisor functionsservices/logging.py- Enhanced loggingapi/routes/health.py- Consumer status integrationservices/health_checker.py- Health check supportResult
Before: Manual API restart required when consumers failed
After: Self-healing system with zero manual intervention needed and health endpoint monitoring
This eliminates indefinitely stuck operations and makes the TRE service bus system resilient to both transient failures and silent hangs.
Documentation and versioning
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.