Add cleanup to SimpleEventIsolation #1032
Conversation
cleaunp will clear unused locks to prevent memory overflow
✔️ Changelog found.Thank you for adding a description of the changes |
|
@evgfilim1, should not be reviewed until changelog is added. |
My bad) I always forget to do it) |
Codecov Report
@@ Coverage Diff @@
## dev-3.x #1032 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 302 302
Lines 8140 8165 +25
=========================================
+ Hits 8140 8165 +25
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -91,6 +91,8 @@ async def close(self) -> None: # pragma: no cover | |||
|
|
|||
|
|
|||
| class BaseEventIsolation(ABC): | |||
| _locks: Dict[Hashable, Any] | |||
There was a problem hiding this comment.
Why this attribute is added to the base class?
This attribute is needed only for in-memory events isolation
| async def close(self) -> None: | ||
| self._locks.clear() | ||
|
|
||
| def _cleanup(self, key: Hashable) -> None: | ||
| if self._locks[key]._waiters is None: # type: ignore[attr-defined] |
There was a problem hiding this comment.
getting access to the protected attribute is a bad decision, but you can inherit from the base class and add the necessary public attribute, for example:
from asyncio import Lock, Future
from typing import Optional, Deque
class _SimpleEventIsolationLock(Lock):
_waiters: Optional[Deque[Future]] = None
@property
def has_waiters(self) -> bool:
return bool(self._waiters)or implement the dunder method __bool__ and use lock directly in the conditions
|
|
||
|
|
||
| @pytest.mark.parametrize("isolation", [pytest.lazy_fixture("redis_isolation")]) | ||
| class TestRedisIsolation: |
There was a problem hiding this comment.
I don't think so the original test should be separated into three, but you can implement new one test that prove your changes, it can alsobe test that you already implement here.
Description
Cleaunp will clear unused locks to prevent memory overflow.
❗️ MUST BE REVIEWED ❗️
Type of change
How has this been tested?
pytest .\tests\test_fsm\storage\ -vv -s --redis redis://localhost:6379/0Test Configuration
Checklist: