Role Removal from Inactive Custodians does not work

Kitty-Therapist commented 5 years ago

Describe the bug When an inactive Custodian have their XP wiped out, their roles should also be wiped if they have any. However, it is not removed as they are left as an inactive Custodian without any roles.

To Reproduce Steps to reproduce the behavior:

Have a Custodian not talk in the server for over a month.
Check their pre-prune stats in bot-logs.
Check to see if they have the role.

Expected behavior All of their roles should be removed, and they are left roleless since their XP were wiped.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context This is an issue that we've been aware, but I've been meaning to file this.

Zacatero commented 5 years ago

It was possible this is exclusive to people who earned roles the old way? Perhaps we should test by giving an alt XP and having it buy custodian and record keeper and wait a month to see if it works normally for the current way to get roles, etc

SuperSajuuk commented 5 years ago

I've always wondered why an inactivity system needs to exist. If the database part is an issue, would it not simply be easier to rework the database structure (or indeed use another database solution) so that people can keep the role for as long as needed? 🤔

Zacatero commented 5 years ago

@SuperSajuuk I think it's pretty fair. It's a full month with complete inactivity. No chat messages, no reactions, no nothing. I think if somebody goes a full month with that level of inactivity, I doubt they'd even notice in any reasonable amount of time that their stuff was gone.

SuperSajuuk commented 5 years ago

@SuperSajuuk I think it's pretty fair. It's a full month with complete inactivity. No chat messages, no reactions, no nothing. I think if somebody goes a full month with that level of inactivity, I doubt they'd even notice in any reasonable amount of time that their stuff was gone.

I am aware of that, but the last i recall, one of the reasons the inactivity system exists is just to reduce database load, which is why I suggested changing the DB system around a bit so it's not so much of an issue. I have a custom ranking bot in a server which has about 1500 records and I don't really have any issue handling load (then again, that database is mysql, rather than a flat file I assume is used here).

AEnterprise commented 5 years ago

some background here

megabot v1 was keeping everyone around all the time and had a terrible database population where for streaks, it kept every single date in the streak as string then to promote people to custodian or the next role, it had to loop over every single user in the database to determine if they had a long enough streak, this ment loading every person, with the massive list of streak strings (oh and did i mention command message streak dates where kept seperately as well as something else i don't remember)

this caused massive cpu and ram spikes

for v2 i know the database storing is a lot more sane, not keeping rediculous amounts of data around (and i think it even DMs you at the moment you reach enough xp to get custodian instead of on a timer) and new roles are bought

so i think the current system might handle it fine as it can at the very least now filter on xp amount instead of having to load each user's data and parse a million strings

dougley can probably give more insights on that, these are just the reasons i know from debugging the vps cpu spikes that kept knocking out bots and causing reboots as well as slowing down megabot

Dougley commented 5 years ago

Custodians should maintain a certain degree of activity in the server. We remove inactive custodians because:

Inactive custodians fill up the database (like @AEnterprise mentioned, this can cause huge resource spikes.), the database isn't suited to handle huge datasets, it's embedded and prone to messing up when the dataset becomes too large.
When their statistics get wiped we (try to) remove the roles as well, since people without statistics are not considered proper custodians by MegaBot.

I admit that the database structure we're using is far from ideal, when the project started out the focus was on using as little external services (like Redis, MongoDB) as possible, right now this is less of a problem which is why Redis is used now.

Dougley commented 4 years ago

Fixed since https://github.com/Dougley/MegaBot/commit/e64b1d49b9b5d00698bcac6b47e725764b61c1d6

Dougley / MegaBot

Role Removal from Inactive Custodians does not work #144