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

Fix: sites with a lot of forms can cause slow queries on pages with form list views #7483

Merged
merged 83 commits into from
Aug 26, 2024

Conversation

glaubersilva
Copy link
Contributor

@glaubersilva glaubersilva commented Aug 8, 2024

Resolves GIVE-1079

Description

This PR solves the performance issue introduced on the Give 3.14 related to the long page load which was only happening on pages with lots of forms that had lots of donations and the progress bar enabled.

Before we didn't face this problem due to the use of meta keys that were used to store the aggregated values for the form stats, since Give 3.14 we started to calculate this data in real-time to keep the data accurate.

The problem with this new approach is that it can be time and resource-consuming in some sites with huge amounts of data.

So, to solve the problem a new approach to load the form stats data was implemented. Now, we load the admin form list views and the form grid view pages without loading the form stats - these data are loaded through async requests to the server only after the page gets loaded.

Also, we just load the data of the forms visible on the screen, when the user scrolls down the form list we load the other stats by demand when it gets visible on the screen as well.

This performance issue doesn't happen in all sites. So, if the customer (or the support team) wishes to disable the async approach to the admin form list views or to the form grid is possible to do that by adding the following options on the wp-config.php file:

/**
 * ADMIN FORM LIST VIEWS OPTIONS
 */

define('GIVE_IS_ALL_STATS_COLUMNS_ASYNC_ON_ADMIN_FORM_LIST_VIEWS', false);
//define('GIVE_IS_GOAL_COLUMN_ASYNC_ON_ADMIN_FORM_LIST_VIEWS', false);
//define('GIVE_IS_DONATIONS_COLUMN_ASYNC_ON_ADMIN_FORM_LIST_VIEWS', false);
//define('GIVE_IS_REVENUE_COLUMN_ASYNC_ON_ADMIN_FORM_LIST_VIEWS', false);


/**
 * FORM GRID OPTIONS
 */

define('GIVE_IS_ALL_PROGRESS_BAR_STATS_ASYNC_ON_FORM_GRID', false);
//define('GIVE_IS_PROGRESS_BAR_AMOUNT_RAISED_ASYNC_ON_FORM_GRID', false);
//define('GIVE_IS_PROGRESS_BAR_DONATIONS_COUNT_ASYNC_ON_FORM_GRID', false);

And if the customer (or support team) wishes to use the meta key aggregated values like it was before Give 3.14, it is possible to do that using the following snippet:

<?php

/**
 * It will use the meta keys that store the aggregated values instead of the data retrieved in real-time from DB.
 */
add_filter('givewp_use_cached_form_stats_meta_keys_on_admin_form_list_views', '__return_true');
add_filter('givewp_use_cached_form_stats_meta_keys_on_form_grid', '__return_true');

Please note that some servers have rules to prevent too many requests at the same time for the same client as described here in this Cloudflare forum thread: https://community.cloudflare.com/t/error-429-message/625392

So, what can happen is that some servers can return a 429 error when some rules like that are enabled. It also will return a Retry-After header with the time the client needs to wait until trying to do a new request, usually 60 minutes. More details here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429#response_containing_retry-after_header

So, to try to avoid this error, a throttling technique was implemented to limit the requests made at once when the form list pages get loaded. By default, only one async request will be run per time.

But is possible to disable the throttling by adding the following in the wp-config.php file:

define('GIVE_ASYNC_DATA_THROTTLING', false);

However, it's recommended to let the throttling feature be enabled. Also, It's important to understand that even with this feature enabled the client can get 429 errors due to highly restrictive rate-limiting rules applied to the Server/Firewall - for these cases, the rules should be adjusted or removed.

Affects

  • The legacy admin form list view.
  • The admin form list view.
  • The form grid list view (shortcode and block).

Visuals

Demo video with more details and context:

https://www.loom.com/share/e9b69a066260435293118b3ad19545a5?sid=4e1437bf-ec62-4d57-9333-4cab66222df7

IMPORTANT: A couple of constants and the snippet discussed in the 👆 video above have changed after the recording, so consider only the values presented in this PR's "description" section.

10 minutes to load the legacy form list view page BEFORE the changes implemented on this PR:

10-minutes-to-load-all-forms

3 seconds to load the legacy form list view AFTER the changes implemented on this PR:

image

Testing Instructions

  1. Setup a site with a lot of data that has the problem - you can find a site sample in the attached Jira task;
  2. Setup the Give version of this branch on the problematic site;
  3. Access the new and legacy form list views on the admin area;
  4. Access a page with the form grid;
  5. Make sure all pages from the previous steps got loaded in a reasonable time;

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@glaubersilva glaubersilva self-assigned this Aug 8, 2024
@glaubersilva
Copy link
Contributor Author

@jonwaldstein Thanks for all the great suggestions. This is ready for re-review.

Copy link
Contributor

@jonwaldstein jonwaldstein left a comment

Choose a reason for hiding this comment

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

@glaubersilva great job man! this is really working well and it was built in such a pluggable way 🔌 Let's battle test this with QA! 🌈

Copy link
Member

@rickalday rickalday left a comment

Choose a reason for hiding this comment

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

Passed manual QA tests.

@jonwaldstein jonwaldstein merged commit a5ef4c9 into develop Aug 26, 2024
20 checks passed
@jonwaldstein jonwaldstein deleted the fix/progress-bar-slow-query-GIVE-1079 branch August 26, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants