Problem/Motivation

On real sites with 30000+ nodes to index there are problems with Drupal's database queue. The \Drupal\simple_sitemap\Queue\SimplesitemapQueue::claimItem() query:

      $item = $this->connection->queryRange('SELECT data, item_id FROM {queue} q WHERE name = :name ORDER BY item_id ASC', 0, 1, [':name' => $this->name])->fetchObject();
      if ($item) {
        $item->data = unserialize($item->data);
        return $item;
      }

becomes a bit of a problem. I think it's because even though we have the limit on the query we have to run this for each claim and we're then deleting the item we claimed. This combines to mess up database query optimisations and results in the query taking more time than you'd like.

Proposed resolution

Given the simple_sitemap already has an optimised queue class - \Drupal\simple_sitemap\Queue\SimplesitemapQueue which does not actually take a lease out on the item we can go a step further and not limit the query - and use a generator to return the rows from an unlimited query.

Also given the we're not updating the lease time I think we should consider setting up a persistent lock around sitemap generation so multiple processes don't try and generate the same rows.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#5 Screenshot 2021-03-15 at 21.27.46.png303.67 KBalexpott
#4 Screenshot 2021-03-15 at 13.42.03.png127.09 KBalexpott
#4 Screenshot 2021-03-15 at 13.42.52.png363.3 KBalexpott

Issue fork simple_sitemap-3203626

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott opened merge request !7

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review

Using #3202290: Add performance test script to measure performance on a site with 10000 nodes...

With the patch

time vendor/bin/drush ssg -v                                                                                                                                                                                         300ms  Mon 15 Mar 11:44:37 >  >  [notice] 4632 out of 10001 total items have been processed. queries: 41752, memory: 288.5 MB, non-peak mem: 286.5 MB
>  [notice] 8061 out of 10001 total items have been processed. queries: 30873, memory: 298.5 MB, non-peak mem: 296.5 MB
>  [notice] 10001 out of 10001 total items have been processed. queries: 17477, memory: 298.5 MB, non-peak mem: 292.5 MB

________________________________________________________
Executed in   24.99 secs   fish           external
   usr time   12.81 secs  113.00 micros   12.81 secs
   sys time    1.51 secs  708.00 micros    1.51 secs

Without the patch

time vendor/bin/drush ssg -v                                                                                                                                                                                           25s  Mon 15 Mar 11:51:10 
>  [notice] 3698 out of 10001 total items have been processed. queries: 37041, memory: 242.5 MB, non-peak mem: 240.5 MB
>  [notice] 7348 out of 10001 total items have been processed. queries: 36512, memory: 256.5 MB, non-peak mem: 254.5 MB
>  [notice] 10001 out of 10001 total items have been processed. queries: 26550, memory: 256.5 MB, non-peak mem: 250.5 MB

________________________________________________________
Executed in   28.12 secs   fish           external
   usr time   14.02 secs  118.00 micros   14.02 secs
   sys time    1.62 secs  876.00 micros    1.62 secs

As we can see there are 100103 queries before this patch and only 89822 after. The represents a considerable drop in db traffic.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
FileSize
Screenshot 2021-03-15 at 13.42.52.png363.3 KB
Screenshot 2021-03-15 at 13.42.03.png127.09 KB

Here's some pictures from a production site suffering every hour while generating the sitemap. The majority of queries are the queue queries from the sitemap module...

Database load

Query log and graph

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
FileSize
Screenshot 2021-03-15 at 21.27.46.png303.67 KB

We've tried this fix on a production site and sitemap generation that was taking 30 mins or more is now taking 3 mins and barely affecting the servers.

Database load graph - sitemap generation is running at 21:11!!!

daniel.bosen’s picture

daniel.bosen
he/him
CreditAttribution: daniel.bosen at Thunder commented
Status: Needs review » Reviewed & tested by the community

I tested this patch on a medium-sized project, I did not quite get the improvements as described in comment 5, but the sitemap generation time was still halved. Amazing! I also tested the locking, it is working as expected as well. The MR looks good as well.

I hope this gets merged soon!

gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte for gbyte commented
Assigned: Unassigned » gbyte
gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte for gbyte commented
Assigned: gbyte » Unassigned
Status: Reviewed & tested by the community » Needs work
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Needs review

Thanks for the review @gbyte - I've tried to address your feedback. I've changed everything to use the same lock id. Deleting the queue while rebuilding the queue is likely to produce odd results. Also I've test manually the locking. Will add a test.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs review » Reviewed & tested by the community

Added the test.

  • gbyte committed c27d3c6 on 8.x-3.x authored by alexpott 
    Issue #3203626 by alexpott: Improve queue performance
gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte for gbyte commented
Status: Reviewed & tested by the community » Fixed

Looks good to me thanks! Let's put it into dev to get a few more eyes on it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.