Make the link fetching process asynchronous

[piotrbak]

Is your feature request related to a problem? Please describe.
In 3.16 we introduced the feature that's fetching the links from the homepage. Currently, this process is happening synchronously, whenever the critical images data is cleared.

It's causing potential problems on the slower environments, as we need to wait until the homepage is loaded and fetched synchronously. It can even cause timeouts.

On our testing environment, clearing the data took 13s compared to 2.4s when putting static data into fetch_links() function.

Describe the solution you'd like
We should make fetching process asynchronous, either using AS or WP Cron.

Acceptance Criteria

1. Whenever fetching process needs to happen, asynchronous task will be registered
2. We'll still send homepage URL (if needed both, desktop and mobile, if separate cache for mobiles is activated) synchronously whenever data is cleared.
3. Asynchronous fetching task will fetch the homepage and send links to the prewarmup (if needed both, desktop and mobile, if separate cache for mobiles is activated)
4. rocket_atf_warmup_links_number filter working without regression, should be set back to 10
5. We should do R&D if we still need to do the usleep between sending the requests here.
6. If possible, we should give this task a high priority in the async queue
7. If the task takes too long, we could consider creating asynchronous tasks for each URL in the 3rd point

[MathieuLamiot]

https://wp-media.slack.com/archives/CUT7FLHF1/p1714730269667559

[MathieuLamiot]

@piotrbak Have you been using the alpha1 version? In which case, it should be expected to double this time with this PR that is already merged on develop. (not on alpha1).

To consider:

• Need for the 500ms between requests? https://wp-media.slack.com/archives/CUT7FLHF1/p1714730269667559 Could save some time but not the majority which is likely within the 2 visits to the homepage. Pending dev feedback.
• Do we need to visit the homepage twice? Or once would be enough to gather the links (they might not be the same on mobile and desktop). currently, we do it twice with different agents. Pending product direction.
• What is the grooming/Estimate to move to an async task? Pending dev feedback.

Then, we'll need product direction on the exact behavior we should target for the initial release: no warmup, no mobile warmup, wait for async to be implemented, etc.

[piotrbak]

@MathieuLamiot

1. Let's wait for devs answer here.
2. I'd say we should visit website only once, mobile sounds more reasonable and based on that populate links for both warmups mobile and desktop

For the release:

1. Remove the 500ms delay if possible and reasonable
2. Visit the homepage only once using mobile and populate all the links based on that

For the future enhancement, the objective would be to implement async before the end of the staggered. If applying the the two points will lower the time significantly, we could lower the priority here and do it later. WDYT?

[MathieuLamiot]

The main reason for latency here is the visit of the homepage. So it depends what would be "acceptable". If 10s is OK, then we should be fine in most cases with your plan. But if we're targeting let's say 5s, then removing the delay and only doing one visit won't be enough to meet this criteria for many websites.

So the decision should be based on:

• What threshold do we consider acceptable? (if it's larger than ~8 seconds, I guess we're fine with one visit, no delay and sync, otherwise, we need async anyway).
• What is the complexity to "remove the mobile visit while keep sending the links to the SaaS for desktop and mobile jobs" vs. making the warm-up async (and keeping 2 visits, that would not be an issue anymore). I am not sure the former is easier than the later?

Regarding the schedule, I'll send my directions on Slack for next week in a few.

[piotrbak]

Doing 2 visits doesn't sound like a must to be honest. From our perspective we could remove it anyways to reduce the load, even if it's async task.

What threshold do we consider acceptable?

Removing the delay on our testing website reduced the time from 13s to 8s already. So I think we can win a lot by removing the second visit.

Async fetch would be an enhancement anyways, the thing is only about its priority.

[MathieuLamiot]

Doing 2 visits doesn't sound like a must to be honest.

Right, but here I am looking for the fastest path to reach the target of lowering latency. If we go with async in the first place, then there is no added value in removing the 2nd visit when it comes to lowering latency, so that would become something with a much lower priority.

Removing the delay on our testing website reduced the time from 13s to 8s already. So I think we can win a lot by removing the second visit.

Are those numbers from alpha1 or develop branch? See my message here

[piotrbak]

It's coming from alpha1

[MathieuLamiot]

Then on develop, even without the 2nd visit and no delay, you'll get something above 8s. There is no gain available anymore and it'll be slightly worst because we'll do 10 more requests to the SaaS.

[Mai-Saad]

Note for test: we need to ensure that both mobile/desktop are there in WPR log after fresh install, after clear critical images, after switch theme and change permalinks

[jeawhanlee]

Scope a solution ✅

In Engine\Media\AboveTheFold\WarmUp

• Create a new queue class to extend AbstractASQueue with group rocket-atf-warmup
• Add a new method to register an action - rocket_atf_warmup.

In Engine\Media\AboveTheFold\WarmUp::Controller

• Create a method warm_up_home that does the same thing as warm_up but just the homepage.
• Remove the home url from fetch_links here

In WP_Rocket\Engine\Media\AboveTheFold\Activation::Activation

• Add the queue class dependency.
• Call the new warm_up_home method.
• Call the new method in warm_up and remove the direct controller warm_up call.
• Update Engine\Media\AboveTheFold\Activation::ServiceProvider

In Engine\Media\AboveTheFold\WarmUp::Subscriber

• Add the queue class dependency.
• Replace warm_up calls to warm_up_home
• Call the new method in warm_up
• Add the new event - rocket_atf_warmup in get_subscribed_events.
• Create a new callback method for the above event that calls $this->controller->warm_up()
• Update \Engine\Media\AboveTheFold::ServiceProvider

Update tests

Estimate the effort ✅

[S]

[jeawhanlee]

@MathieuLamiot Since we are touching the warmup, I think the way we fetch links can be a bit optimized based on number to return on the filter. As now if we need 10 links and we have 100 links on the homepage, we will go through the whole 100 before finally slicing the remaining 90 and returning it. is this something we want to look into now?

[MathieuLamiot]

Good catch. Maybe it'd be more appropriate to open a dedicated GH issue? The sprint is already quite full so I'd prefer not adding optional optimizations to the scope.

[Tabrisrp]

LGTM

[MathieuLamiot]

@Tabrisrp @jeawhanlee I might be missing something here but I feel like with the proposed grooming, AC n°2 won't be covered.
Maybe we need to:

• In the controller, create a method warm_up_home() that does the same thing as warm_up but just the homepage, instead of $this->fetch_links()
• In WP_Rocket\Engine\Media\AboveTheFold\Activation::Activation, instead of deleting the call to the controller, replace it with a call to warm_up_home
• In the WarmUp/Subscriber, replace calls to the controller from warm_up to warm_up_home

WDYT?

[jeawhanlee]

But we currently send the home url for warm up here in the fetch_links(), doing it separately seems redundant, is there a reason for this?

[MathieuLamiot]

Ah we would have to remove it from fetch_links then as well.
The idea is that homepage should be warmed up as quickly as possible from what I understand. Async processes with AS can be a bit slower.

[jeawhanlee]

The Grooming has been updated.