ctm / mb2-doc

Mb2, poker software
https://devctm.com
7 stars 2 forks source link

Are we leaking Lobby Sessions? #1368

Closed ctm closed 3 months ago

ctm commented 3 months ago

See if we leak LobbySessions and if so, fix it.

I have no reason to believe that we're leaking Lobby Sessions. In fact, our memory stats strongly suggest we're not. However, I just saw how we're loading them all into memory and a leak would be annoying eventually.

I don't have access to our database here, so this is mostly just a note to myself to take a quick peek and close this issue. I could put it in TODO, but TODO is overflowing.

ctm commented 3 months ago

Yes, yes we are:

mb2=> select player_id, count(player_id) from lobby_sessions group by player_id;
select player_id, count(player_id) from lobby_sessions group by player_id;
 player_id | count 
-----------+-------
        10 |  4034
        37 |     2
(2 rows)

There's nothing special in the code about id 10, but that is indeed my id. There is code, however, that deletes stale lobby sessions, and my guess is it determines staleness accidentally based on player id rather than the lobby session id and since I log in several times a day, that is why there are so many stale ones for id 10.

This is ugly and embarrassing but probably has not affected anything yet, although I can't say that with 100% certainty until I find the exact cause of the problem and, of course, when I do, I'll fix it.

I'll try to do this one before I go running this morning, since after I get back I'll be headed to a place where I won't have access to the database, or perhaps I'll I'll do this one instead of going running this morning.

ctm commented 3 months ago

Yup. Stale sessions are pruned using a timeout, but that timeout is set up when the server starts and I pretty much always log in after doing a restart. 37's duplicate session id has already been deleted.

The obvious fix is to spin up the timeouts via session id rather than player id. I may not get to it before I leave, after all…

ctm commented 3 months ago

Fixed and in master, waiting for a deploy.

ctm commented 3 months ago

Deploying now.