Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with init logic #691

Closed
4 tasks done
MinnDevelopment opened this issue May 25, 2018 · 5 comments
Closed
4 tasks done

Issues with init logic #691

MinnDevelopment opened this issue May 25, 2018 · 5 comments
Labels
level: veteran requires deep understanding of java and jda status: completed has been completed but is not yet released type: proposal

Comments

@MinnDevelopment
Copy link
Member

MinnDevelopment commented May 25, 2018

Currently JDA requires all guilds and all of the associated entities to be loaded before starting to fire events. This has serious effects as entire shards can get stuck on a single guild refusing to connect to the gateway. In addition to that JDA can have serious cache issues during outages that can lead to memory issues due to duplicated cache entries that don't get removed.

  • Guilds should not prevent startup
  • Events should fire when all required entities are loaded
  • Cache should be aware of duplicates
  • Properly handle unavailability

We have ideas of adding a GuildReadyEvent in addition to ReadyEvent which would be useful to allow events to fire even if not all guilds are ready yet. The ReadyEvent would be fired when all guilds have been initialized, however events may already have fired for them.

I will redirect the issue traffic of #625 / #602 / #512 to this issue because they are closely related to one-another but I will keep them open for now.

As I don't have a lot of free time at the moment, I cannot give any ETA on when I will find time to work on these proposals. This most likely requires a rewrite of JDA internals.

@MinnDevelopment MinnDevelopment added type: proposal level: veteran requires deep understanding of java and jda labels May 25, 2018
@MinnDevelopment MinnDevelopment added this to the JDA Longtime Goals milestone May 25, 2018
@freyacodes
Copy link
Contributor

The current handling has a memory leak in EntityBuilder#cachedGuildJsons during Discord outages, where a first pass occurs but never a second pass.
image

@MinnDevelopment
Copy link
Member Author

I've started planning how the new init system should be setup.

The first issue I want to tackle is that we have the logic for setting up the session split across the library in several different classes. I believe it would be sensible to focus all of the setup logic for guilds into a single controller (GuildSetupManager or similar). This manager would then be the central facility to handle the setup for each guild.

Currently I believe this is the list of things that would need to rely on the new manager system:

  • GuildLock
  • EntityBuilder
  • ReadyHandler
  • GuildMemberChunkHandler
  • GuildCreateHandler
  • GuildDeleteHandler

All of these classes should instead of keeping their own cache use the new manager system and just pass information to it. The manager system would then advance the setup for the guild and decide if it is ready for usage.

For example:

  1. ReadyHandler receives guilds
  2. The guilds are passed to GuildSetupManager
  3. GuildCreateHandler receives guild data
  4. The guild is updated in the GuildSetupManager
  5. GuildSetupManager decides if it should start chunking
    ...

The logic here is not really planned, I just want to set an example on how it would work differently to the current system.

This way we have a codebase that can easily be found and understood by a contributor, rather than a complex and laid out system which spans across the api. Additionally we can change how the logic works and possibly allow events to be fired before all guilds are ready.

@MinnDevelopment
Copy link
Member Author

This proposal is currently being worked on in experimental/guild-setup

@MinnDevelopment MinnDevelopment added the status: completed has been completed but is not yet released label Sep 2, 2018
@MinnDevelopment
Copy link
Member Author

The new system will be included in v3.8.0

@DV8FromTheWorld
Copy link
Member

I am so hype for the guild setup rewrite to go live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level: veteran requires deep understanding of java and jda status: completed has been completed but is not yet released type: proposal
Projects
None yet
Development

No branches or pull requests

3 participants