🐛 Fixes possible issue with GC closing projects #6492
Conversation
GitHK
added
the
a:webserver
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6492 +/- ##
==========================================
- Coverage 84.57% 82.55% -2.03%
==========================================
Files 10 602 +592
Lines 214 30589 +30375
Branches 25 265 +240
==========================================
+ Hits 181 25253 +25072
- Misses 23 5275 +5252
- Partials 10 61 +51
Continue to review full report in Codecov by Sentry.
|
GitHK
requested review from
matusdrobuliak66,
pcrespov and
sanderegg
as code owners
GitHK
added
bug
|
| await self._registry.set_key_alive( | ||
| self._resource_key(), _get_service_deletion_timeout(self.app) | ||
| ) | ||
| # when the tab is closed the alive key is also removed immediately, |
There was a problem hiding this comment.
So from my understanding, this issue is somehow connected to session IDs (where there is probably some issue how we work with them cross product/cross tabs) did you test it and are you sure this removal will not cause other side-effect issue? Are you able to reproduce this issue? (its happening so often that I guess it should be reproducable?)
There was a problem hiding this comment.
You can reproduce this everywhere. Open a project and cose the tab its alive key will still be present in Redis. This makes no sense to me. But I might be wrong.
There was a problem hiding this comment.
If you are not sure, why are you changing it without verifying? What if this change makes the GC more unstable?
pcrespov
left a comment
pcrespov
left a comment
There was a problem hiding this comment.
If you are not sure, why are you changing it without verifying? What if this change makes the GC more unstable?
sanderegg
left a comment
sanderegg
left a comment
There was a problem hiding this comment.
Please test the following:
- start a service
- close the tab
- wait the GC interval
- re-open the study in a new tab --> this should work
I have the feeling what you did here will make this fail.
| await self._registry.set_key_alive(self._resource_key(), 1) | ||
|
|
||
| async def remove_socket_id(self) -> None: | ||
| async def remove_socket_id_after_disconnection(self) -> None: |
There was a problem hiding this comment.
I don't why you change the naming of that function.
In effect you are moving logic naming into a deeper location.
| ) | ||
| # when the tab is closed the alive key is also removed immediately, | ||
| # there is no reason to keep it active | ||
| await self._registry.set_key_alive(self._resource_key(), 1) |
There was a problem hiding this comment.
the key alive has a TTL.
I checked that this function _get_service_deletion_timeout (which is badly named) returns the default TTL.
Did you actually check what happens when you close the tab now? it will instantly remove the service when the GC runs.
This kind of disconnection happen a lot in a train for example... so I think here you might be messing around with it. Did you test that use-case?
|
outdated |
4 participants
What do these changes do?
I'm not sure why the GC would not remove the alive key as soon as the tab is closed or the user disconnects. The key is set to it's maximum timeout.
Related issue/s
How to test
Dev-ops checklist