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

Perf : Save ratings count in notices entities #408

Merged
merged 1 commit into from
Jul 6, 2021

Conversation

JalilArfaoui
Copy link
Member

For each display of notices list in back-office and every call of the API with notices info or notices list, we are counting the number of ratings for each type of rating … No wonder why this was getting slow and consuming up to 1 GB of memory just to show 20 notices in back-office !

What I did here is to store the count of badged, display, unfold, clicked, liked, 'dislikedanddismissedevents directly inNoticeentities, and update the corresponding count every time a rating isPOST`ed.

Showing the same page now consumes approx 15-20MB of memory (tested with production database) … back to normal.

For now, with the added index, the rating POST is quick enough (400 ms), so I left the bus synchronous, but later, it would be possible to make it asynchronous, and event to group updates.

Note that this breaks a bit the single source of truth, but it also shows the way to CQRS if you think about it :-) …

@MaartenLMEM @Etienne-DisMoi One nice side effect is that you can now sort the notices by ratings in the back-office. (I also changed some table headers to emojis to save some space)

It would be possible to do the same with relayersCount if this proves useful.

@JalilArfaoui JalilArfaoui self-assigned this Jul 5, 2021
@JalilArfaoui JalilArfaoui force-pushed the perf/save_ratings_count_in_notices_entities branch 2 times, most recently from f970037 to d3d7eaf Compare July 5, 2021 21:03
Copy link
Member

@gregoirelacoste gregoirelacoste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, big performance gain indeed!
it will now be necessary to implement methods in front to increment these new fields?

@JalilArfaoui
Copy link
Member Author

it will now be necessary to implement methods in front to increment these new fields?

@gregoirelacoste nope, no change on the API side, same fields are filled,

@JalilArfaoui JalilArfaoui force-pushed the perf/save_ratings_count_in_notices_entities branch from d3d7eaf to 560429a Compare July 6, 2021 20:41
@JalilArfaoui JalilArfaoui merged commit f1e3111 into master Jul 6, 2021
@JalilArfaoui JalilArfaoui deleted the perf/save_ratings_count_in_notices_entities branch July 6, 2021 20:59
@JalilArfaoui
Copy link
Member Author

@MaartenLMEM @Etienne-DisMoi It’s merged and available on https://staging-notices.bulles.fr … Can you validate functionnally ?

@JalilArfaoui
Copy link
Member Author

This is now in production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error 500 when clicking on Expired notices
2 participants