AcquiaSearchSolrApi:getCores calls lock_acquire for entire request

Created on 4 August 2023, over 1 year ago
Updated 15 March 2024, 10 months ago

From AcquiaSearchSolrApi:getCores:

      $now = REQUEST_TIME;
      while (!lock_acquire('acquia_search_get_search_indexes')) {
        // Throw an exception after X amount of seconds.
        if (($now + self::REQUEST_TIMEOUT) < REQUEST_TIME) {
          throw new \Exception("Couldn't acquire lock for 'acquia_search_get_search_indexes' in less than " . self::REQUEST_TIMEOUT . " seconds.");
        }
      }

There is a logic error in the if condition. It is equivalent to:
if ((REQUEST_TIME + 60) < REQUEST_TIME) {
which will never be true.

If the lock cannot be acquired, this while loop consumes all remaining time until the page request times out.

This is especially problematic due to this code being called indirectly from `acquia_search_init()` using `acquia_search_set_apachesolr_overrides`. On page loads that don't even require Acquia Search, we are having requests time out due to this Acquia Search logic.

πŸ› Bug report
Status

Needs work

Version

4.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States jdleonard Austin, TX, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @jdleonard
  • πŸ‡ΊπŸ‡ΈUnited States jdleonard Austin, TX, USA

    Cross-referencing Acquia support case #00975363

  • Status changed to Needs review over 1 year ago
  • Open on Drupal.org β†’
    Core: 7.x + Environment: PHP 7.4 & MySQL 5.6
    last update over 1 year ago
    Waiting for branch to pass
  • πŸ‡ΊπŸ‡ΈUnited States japerry KVUO

    Pushed internally for review, however you can test the fix with the attached patch as well.

  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States jdleonard Austin, TX, USA

    This patch did not address the issue for us.

    $now = REQUEST_TIME;
    
    ...
    
    if (($now + self::REQUEST_TIMEOUT) < REQUEST_TIME) 

    is equivalent to:
    if (REQUEST_TIME + self::REQUEST_TIMEOUT < REQUEST_TIME)
    which will always be false.

    Perhaps also part of the problem is the use of REQUEST_TIME instead of time(). In the event that this code is reached from a long-running process (i.e. where the request started more than 60 seconds ago), the problem will be compounded.

    Our use case involves operations performed by long-running Drush command executions though I don't believe that our niche use case is the only one that would be affected.

  • πŸ‡ΊπŸ‡ΈUnited States calebtr

    It looks like this patch is included in 7.x-4.6. I don't see the conditional logic referenced in #4 included in AcquiaSearchSolrApi::getCores().

Production build 0.71.5 2024