Check number of available help channels hourly#1941
Conversation
If number of available help channels is incorrect, reload the HelpChannels cog.
dementati
added
a: backend
Numerlor
left a comment
Numerlor
left a comment
There was a problem hiding this comment.
Would this help with the issue? I imagine if the count drops the admins will be notified sooner than the one hour period in most cases
I used the one-hour interval mentioned in the original issue, but perhaps that was referring to actually reloading the cog and not just checking the available channel count. Since getting the category channels in this manner doesn't seem to require an API call, I guess we could do it much more frequently at no cost. |
| """ | ||
| Check number of available help channels periodically. | ||
|
|
||
| If number of available help channels is incorrect, reload cog. |
There was a problem hiding this comment.
| If number of available help channels is incorrect, reload cog. | |
| If the number of available help channels is incorrect, reload the cog. |
onerandomusername
left a comment
onerandomusername
left a comment
There was a problem hiding this comment.
This doesn't seem like it would fix the problem, and may add more problems in the long run.
| @tasks.loop(hours=1) | ||
| async def check_channel_consistency(self) -> None: |
There was a problem hiding this comment.
I believe there is a possible race condition which would extend the amount of bugs.
If a channel is currently being closed while this runs, they would be awaiting an edit coroutine. This would allow the program to switch context and run the waiting task here.
However, the api would still be completing when this task decides to reload the cog, furthering the issue.
Its a race condition that would make the problem worse, imo.
A solution with that could be some asyncio.Locks, in order to not let the context switch to within here before a channel finishes closing and returns.
There was a problem hiding this comment.
I've looked into it when we opened the issue, but the cog is pretty resilient. If the closing process is still on the bot side, that shouldn't affect matters since it'll be canceled and resynced on startup.
If it's waiting on an API call from discord, I think it actually does not cancel until it's done, but I may be mistaken. Either way, the worst case scenario if it doesn't is that we'll move one channel twice, but it'll be to the same place both times, so nothing will break.
| available_help_channels = set(_channel.get_category_channels(self.available_category)) | ||
| if len(available_help_channels) != constants.HelpChannels.max_available: | ||
| log.trace("Unexpected number of available help channels, reloading cog...") | ||
| self.bot.reload_extension("bot.exts.help_channels") |
There was a problem hiding this comment.
Why reload the whole cog? Is it not sufficient to just call init_available() again?
There was a problem hiding this comment.
In such case, I think you'd need to add a lock so it doesn't interfere with claim_channel().
Bluenix2
added
s: waiting for author
Xithrius
added
up for grabs
|
@MarkKoz Hello. What's your status on this PR? Thanks. |
I do have some local changes but nothing was competed. I believe I ran into a difficult concurrency problem which led me to writing a custom lock, but I was having trouble achieving what I needed. I'll try to find it this weekend and see if this is something I can continue or not. |
|
Work has started on the new help system, and given the amount of work this issue will require, I don't think it makes sense to continue. It's not high priority anyway. |
|
Agreed, lets not spend resources on this. |
8 participants
Solves the help channel count inconsistency from #1903.
If number of available help channels is incorrect, reload the HelpChannels cog.