Skip to content

🐛 Webserver: ensure project-logs unsubscription always runs on disconnect/close - #9607

Open
sanderegg wants to merge 2 commits into
ITISFoundation:masterfrom
sanderegg:maintenance/ensure-rabbitmq-subscribed-topics-are-always-properly-unsubscribed
Open

🐛 Webserver: ensure project-logs unsubscription always runs on disconnect/close#9607
sanderegg wants to merge 2 commits into
ITISFoundation:masterfrom
sanderegg:maintenance/ensure-rabbitmq-subscribed-topics-are-always-properly-unsubscribed

Conversation

@sanderegg

Copy link
Copy Markdown
Member

What do these changes do?

Follow-up to the RabbitMQ logs-queue incident : 2 code paths could silently skip unsubscribing a user's session from a project's live-log topic RabbitMQ Queue binding.

Both bugs share the same shape: a fallible RPC-dependent step (a lock-state notification / a dynamic-service-removal or project-state fetch — all of which can time out when RabbitMQ or the dynamic-scheduler is under stress) ran before the log-unsubscription step, with no isolation between them, so a failure in the first step prevented the second from ever running.

  1. _on_user_disconnected (projects/_controller/projects_slot.py, socket disconnect path): the project-locked-state notification and the log-unsubscription were two separate sequential steps — a failure in one can no longer block the other.
  2. close_project_for_user (projects/_projects_service.py, REST close_project + garbage-collector path): now the log unsubscription always runs.

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg added this to the War Pigs milestone Aug 27, 2026
@sanderegg sanderegg self-assigned this Aug 27, 2026
@sanderegg sanderegg added the t:maintenance Maintenance work; used to filter tasks for end-of-sprint reporting in Review (Agreed July 3, Retro) label Aug 27, 2026
@github-actions github-actions Bot added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Aug 27, 2026
@sanderegg sanderegg added the 🤖-automerge marks PR as ready to be merged for Mergify label Aug 27, 2026
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.13%. Comparing base (b722465) to head (40351a6).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9607      +/-   ##
==========================================
- Coverage   88.15%   88.13%   -0.02%     
==========================================
  Files        1559     1559              
  Lines       60601    60601              
  Branches     1583     1583              
==========================================
- Hits        53421    53412       -9     
- Misses       6759     6768       +9     
  Partials      421      421              
Flag Coverage Δ *Carryforward flag
integrationtests 71.92% <ø> (-0.03%) ⬇️
unittests 86.98% <ø> (ø) Carriedforward from b722465

*This pull request uses carry forward flags. Click here to find out more.

Components Coverage Δ
pkg_aws_library 96.83% <ø> (ø)
pkg_celery_library 78.60% <ø> (ø)
pkg_dask_task_models_library 90.70% <ø> (ø)
pkg_models_library 92.62% <ø> (ø)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database 90.13% <ø> (ø)
pkg_service_integration 73.01% <ø> (ø)
pkg_service_library 71.28% <ø> (ø)
pkg_settings_library 90.93% <ø> (ø)
pkg_simcore_sdk 86.42% <ø> (ø)
agent 93.91% <ø> (ø)
api_server 92.97% <ø> (ø)
autoscaling 95.21% <ø> (ø)
catalog 91.90% <ø> (ø)
clusters_keeper 98.61% <ø> (ø)
dask_sidecar 93.38% <ø> (ø)
datcore_adapter 98.08% <ø> (ø)
director 79.17% <ø> (ø)
director_v2 91.92% <ø> (-0.12%) ⬇️
dynamic_scheduler 95.75% <ø> (ø)
dynamic_sidecar 88.66% <ø> (ø)
efs_guardian 89.40% <ø> (ø)
invitations 91.63% <ø> (ø)
payments 92.49% <ø> (ø)
resource_usage_tracker 91.68% <ø> (ø)
storage 88.03% <ø> (ø)
webclient ∅ <ø> (∅)
webserver ∅ <ø> (∅)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b722465...40351a6. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sanderegg
sanderegg marked this pull request as ready for review August 27, 2026 17:41
@sanderegg

Copy link
Copy Markdown
Member Author

@mergify queue

@mergify

mergify Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Waiting for any of
  • check-neutral = unit-tests
  • check-skipped = unit-tests
  • check-success = unit-tests
All conditions
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule default]:
      • any of [🛡 GitHub branch protection]:
        • check-neutral = unit-tests
        • check-skipped = unit-tests
        • check-success = unit-tests
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • github-review-approved [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of [🛡 GitHub branch protection]:
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of [🛡 GitHub branch protection]:
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of [🛡 GitHub branch protection]:
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of [📌 queue requirement]:
    • check-neutral = Mergify Merge Protections
    • check-skipped = Mergify Merge Protections
    • check-success = Mergify Merge Protections

@GitHK GitHK left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pcrespov pcrespov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

@sanderegg
sanderegg force-pushed the maintenance/ensure-rabbitmq-subscribed-topics-are-always-properly-unsubscribed branch from b53335d to 40351a6 Compare August 28, 2026 16:37
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs t:maintenance Maintenance work; used to filter tasks for end-of-sprint reporting in Review (Agreed July 3, Retro)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants