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

Inefficient guildInfo updating #5

Closed
ahnl opened this issue May 6, 2022 · 6 comments
Closed

Inefficient guildInfo updating #5

ahnl opened this issue May 6, 2022 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@ahnl
Copy link
Member

ahnl commented May 6, 2022

I guess it's because of getMessagesLeaderboard() function, which uses discordUtility.getUserById() function to translate user id to username. The latter function fetches 5 users (number of users displayed on the leaderboard) every 5 seconds (refresh rate) from Discord API seemingly without cache.

Needs investigating.

@ahnl ahnl added the bug Something isn't working label May 6, 2022
@ahnl ahnl self-assigned this May 6, 2022
@ahnl
Copy link
Member Author

ahnl commented May 6, 2022

image

A picture of how long an update takes and here's what it consists of:

    const messagesToday = await database.getMessageCount(mainServer)
    const memberCount = await discordUtility.getMemberCount(mainServer)
    const membersOnline = await discordUtility.getOnlineCount(mainServer)
    const config = await database.getDataCollectionConfig(mainServer)
    const boostStatus = await discordUtility.getBoostStatus(mainServer, config?.allowed ?? [])
    const codingLeaderboard = await getCodingLeaderboard()
    const messagesLeaderboard = await getMessagesLeaderboard()

Esinko added a commit that referenced this issue May 6, 2022
- Use cache in getUserById,
- Remove useless ESLint comments
- Gracefully error in updateGuildInfoCache
@Esinko
Copy link
Member

Esinko commented May 6, 2022

There should now be a cache for user date in getUserById. Could you run the test again?

@Chicken
Copy link
Member

Chicken commented May 6, 2022

The code should probably also take advantage of parallel tasks with Promise.all() instead of doing these asynchronous tasks sequentially.

@Esinko
Copy link
Member

Esinko commented May 6, 2022

A very good point.

@Esinko
Copy link
Member

Esinko commented May 6, 2022

The code should probably also take advantage of parallel tasks with Promise.all() instead of doing these asynchronous tasks sequentially.

I implemented that here

@Esinko
Copy link
Member

Esinko commented May 19, 2022

Should we consider this issue closed @ahnl?

@ahnl ahnl closed this as completed May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants