🐛 Allow for cleanup of Redis when asyncio task is not present 🚨 #8620

GitHK · 2025-11-19T10:17:15Z

What do these changes do?

If a task is marked for removal and there is no entry in Redis for more than 1 minute, then it will be removed form Redis.

Why is this necessary? If an instance where and asyncio task was running is rebooted, the entry will remain in Redis. This ensures cleanup.

Related issue/s

How to test

Dev-ops 🚨

When releasing this to any deployment, do the following:

Go to Redis-Commander
Open the redis console on DB number 6
Run below script (which removes all the keys in that database)

EVAL "for _,k in ipairs(redis.call('KEYS','*')) do redis.call('DEL',k) end" 0

…nning-webservers

codecov · 2025-11-19T10:20:49Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.43%. Comparing base (daf0b87) to head (12abbec).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8620      +/-   ##
==========================================
+ Coverage   87.51%   89.43%   +1.91%     
==========================================
  Files        2010     1368     -642     
  Lines       78979    58011   -20968     
  Branches     1378      154    -1224     
==========================================
- Hits        69122    51884   -17238     
+ Misses       9453     6077    -3376     
+ Partials      404       50     -354

Flag	Coverage Δ		*Carryforward flag
integrationtests	`63.95% <ø> (ø)`		Carriedforward from daf0b87
unittests	`87.84% <ø> (+1.49%)`	⬆️

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

Components	Coverage Δ
pkg_aws_library	`∅ <ø> (∅)`
pkg_celery_library	`∅ <ø> (∅)`
pkg_dask_task_models_library	`∅ <ø> (∅)`
pkg_models_library	`∅ <ø> (∅)`
pkg_notifications_library	`∅ <ø> (∅)`
pkg_postgres_database	`∅ <ø> (∅)`
pkg_service_integration	`∅ <ø> (∅)`
pkg_service_library	`∅ <ø> (∅)`
pkg_settings_library	`∅ <ø> (∅)`
pkg_simcore_sdk	`84.58% <ø> (ø)`
agent	`93.44% <ø> (ø)`
api_server	`91.37% <ø> (ø)`
autoscaling	`95.83% <ø> (ø)`
catalog	`92.06% <ø> (ø)`
clusters_keeper	`99.14% <ø> (ø)`
dask_sidecar	`91.72% <ø> (ø)`
datcore_adapter	`97.95% <ø> (ø)`
director	`75.72% <ø> (ø)`
director_v2	`91.29% <ø> (ø)`
dynamic_scheduler	`96.66% <ø> (ø)`
dynamic_sidecar	`90.83% <ø> (ø)`
efs_guardian	`89.83% <ø> (ø)`
invitations	`90.90% <ø> (ø)`
payments	`92.70% <ø> (ø)`
resource_usage_tracker	`92.00% <ø> (ø)`
storage	`86.70% <ø> (+0.24%)`	⬆️
webclient	`∅ <ø> (∅)`
webserver	`86.67% <ø> (+0.01%)`	⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daf0b87...12abbec. 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.

mergify · 2025-11-19T10:45:27Z

🧪 CI Insights

Here's what we observed from your CI run for 12abbec.

❌ Job Failures

Pipeline	Job	Health on `master`	Retries	🔍 CI Insights	📄 Logs
`CI`	`unit-tests`		`0`	View	View

✅ Passed Jobs With Interesting Signals

Pipeline	Job	Signal	Health on `master`	Retries	🔍 CI Insights	📄 Logs
`CI`	`integration-tests`	Base branch is broken, but the job passed. Looks like this might be a real fix 💪		`0`	View	View

…nning-webservers

…ning-webservers

sonarqubecloud · 2025-11-19T15:56:55Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sanderegg

Question: will this work with multiple replicas of the service as well? I guess different replicas might have different tasks in their process

sanderegg · 2025-11-20T07:45:55Z

packages/service-library/src/servicelib/long_running_tasks/_redis_store.py

+                mapping=_to_redis_hash_mapping(
+                    {
+                        _MARKED_FOR_REMOVAL_FIELD: True,
+                        _MARKED_FOR_REMOVAL_AT_FIELD: datetime.datetime.now(


Q: why not have an optional _MARKED_FOR_REMOVAL_AT_FIELD field? if it is null it means there are no such thing. If it exists it means you want to remove it.. no need for the additional boolean.

sanderegg · 2025-11-20T07:46:21Z

packages/service-library/src/servicelib/long_running_tasks/models.py

+                "the entry form Redis"
+            )
+        ),
+    ] = None


Again I do not see the point of having 2 variables here.

sanderegg · 2025-11-20T07:46:46Z

packages/service-library/src/servicelib/long_running_tasks/task.py


        task_to_cancel = self._created_tasks.pop(task_id, None)
        if task_to_cancel is not None:
+            _logger.debug("Removing asyncio task related to task_id='%s'", task_id)


tip: use with log_context

sanderegg · 2025-11-20T07:47:33Z

packages/service-library/src/servicelib/long_running_tasks/task.py

            await self._tasks_data.delete_task_data(task_id)
+        else:
+            task_data = await self._tasks_data.get_task_data(task_id)
+            if task_data.marked_for_removal_at is not None and datetime.datetime.now(  # type: ignore[union-attr]


see? this would simplify these if case... actually the boolean is useless here

yes i tend to agree

sanderegg · 2025-11-20T07:48:44Z

packages/service-library/src/servicelib/long_running_tasks/task.py

+            task_data = await self._tasks_data.get_task_data(task_id)
+            if task_data.marked_for_removal_at is not None and datetime.datetime.now(  # type: ignore[union-attr]
+                tz=datetime.UTC
+            ) - task_data.marked_for_removal_at > datetime.timedelta(  # type: ignore[union-attr]


use assert task_data.marked_for_removal_at # nosec instead of disabling pylint

sanderegg · 2025-11-20T07:49:10Z

packages/service-library/src/servicelib/long_running_tasks/task.py

+            ) - task_data.marked_for_removal_at > datetime.timedelta(  # type: ignore[union-attr]
+                seconds=_TASK_REMOVAL_MAX_WAIT
+            ):
+                _logger.debug(


with log_context here would also tell you that the function finished.

pcrespov

thx. Left some comments for your consideration

BTW is wiping out the entire Redis database 6 necessary for this change?

pcrespov · 2025-11-21T09:37:24Z

packages/service-library/src/servicelib/long_running_tasks/models.py

        bool,
        Field(description=("if True, indicates the task is marked for removal")),
    ] = False
+    marked_for_removal_at: Annotated[


Why is the combination marked_for_removal_at=False and marked_for_removal_at!=None allowed? What does it mean?
or marked_for_removal_at=True and marked_for_removal_at=None ? Does this mena it can be removed anytime? but there is no gurantee of when?

pcrespov · 2025-11-21T09:39:06Z

packages/service-library/src/servicelib/long_running_tasks/task.py

            await self._tasks_data.delete_task_data(task_id)
+        else:
+            task_data = await self._tasks_data.get_task_data(task_id)
+            if task_data.marked_for_removal_at is not None and datetime.datetime.now(  # type: ignore[union-attr]


yes i tend to agree

pcrespov · 2025-11-21T09:43:33Z

packages/service-library/tests/long_running_tasks/utils.py

 TEST_CHECK_STALE_INTERVAL_S: Final[float] = 1
+
+
+def strip_markd_for_removal_at(task_data: TaskData) -> TaskData:


typo: marked

stick to a small set of verbs e.g. CRUD or similar ones. Moreover "strip" is used in strings which has nothing to do with this.

Here could be clear_marked_for_removal

refactor

98bc851

GitHK self-assigned this Nov 19, 2025

GitHK added bug buggy, it does not work as expected t:maintenance Some planned maintenance work and removed bug buggy, it does not work as expected labels Nov 19, 2025

GitHK changed the title ~~Allow for cleanup of Redis is task is not present~~ 🐛 Allow for cleanup of Redis is task is not present Nov 19, 2025

Merge remote-tracking branch 'upstream/master' into pr-osparc-long-ru…

94da882

…nning-webservers

GitHK added this to the Imparable milestone Nov 19, 2025

mypy

8f229cd

GitHK changed the title ~~🐛 Allow for cleanup of Redis is task is not present~~ 🐛 Allow for cleanup of Redis when asyncio task is not present Nov 19, 2025

Andrei Neagu added 4 commits November 19, 2025 13:27

fixed broken tests

3031e9c

Merge remote-tracking branch 'upstream/master' into pr-osparc-long-ru…

8319df0

…nning-webservers

refactor

2f9dcaf

erge remote-tracking branch 'upstream/master' into pr-osparc-long-run…

12abbec

…ning-webservers

GitHK changed the title ~~🐛 Allow for cleanup of Redis when asyncio task is not present~~ 🐛 Allow for cleanup of Redis when asyncio task is not present 🚨 Nov 19, 2025

GitHK marked this pull request as ready for review November 19, 2025 16:01

GitHK requested a review from pcrespov as a code owner November 19, 2025 16:01

GitHK requested review from YuryHrytsuk, bisgaard-itis, giancarloromeo, matusdrobuliak66 and sanderegg November 19, 2025 16:02

sanderegg reviewed Nov 20, 2025

View reviewed changes

pcrespov reviewed Nov 21, 2025

View reviewed changes

		TEST_CHECK_STALE_INTERVAL_S: Final[float] = 1


		def strip_markd_for_removal_at(task_data: TaskData) -> TaskData:

🐛 Allow for cleanup of Redis when asyncio task is not present 🚨 #8620

Are you sure you want to change the base?

🐛 Allow for cleanup of Redis when asyncio task is not present 🚨 #8620

Conversation

GitHK commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

Related issue/s

How to test

Dev-ops 🚨

Uh oh!

codecov bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mergify bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 CI Insights

❌ Job Failures

✅ Passed Jobs With Interesting Signals

Uh oh!

sonarqubecloud bot commented Nov 19, 2025

Quality Gate passed

Uh oh!

sanderegg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pcrespov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GitHK commented Nov 19, 2025 •

edited

Loading

codecov bot commented Nov 19, 2025 •

edited

Loading

mergify bot commented Nov 19, 2025 •

edited

Loading