localgovdrupal / localgov_directories

Searchable, filterable, directories of information, and locations.
1 stars 3 forks source link

It's not possible to enable Venues as part of a site install #201

Closed stephen-cox closed 2 years ago

stephen-cox commented 2 years ago

From https://github.com/localgovdrupal/localgov_microsites_group/issues/108

Enabling directories during a site install using Drush hangs indefinitely. This is because the config change when adding new content types to the Search API data source for the second time triggers a content reindex, which hangs in Drush.

The code in hook_ENTITY_TYPE_insert() that adds the directory item content type to the search index data store is run twice during a site install; first for localgov_directories_page and then localgov_directories_venue. The second time the index configuration is saved a content reindex is triggered. When run through Drush this triggers a second Drush instance to run the batch process, which hangs. It works fine when running a site install through the browser and doesn't cause a problem enabling venues after a site install.

The code the causing the problem is: https://github.com/localgovdrupal/localgov_directories/blob/0d2d1e51458479c06339a47719d69c283c619649/localgov_directories.module#L176-L181

To fix this I think we'll need to edit the configuration directly, rather than through the Index entity.

FYI @finnlewis @ekes

ekes commented 2 years ago

Pretty certain you can suspend the reindex trigger.

stephen-cox commented 2 years ago

Pretty certain you can suspend the reindex trigger.

I thought so too, but have been unable to find how to do this. There doesn't seem to be a method on an Index entity for this and have tried the following:

\Drupal::state()->set("search_api.index.{$index->id()}.has_reindexed", TRUE);

and

\Drupal::state()->set('search_api_use_tracking_batch', FALSE);
ekes commented 2 years ago

This actual doing the indexing, rather than only reseting it, is triggered by the Index::postSave() stuff with the tracker?

Like because of:

      if (!$index_task_manager->isTrackingComplete($this)) {
        // Give tests and site admins the possibility to disable the use of a
        // batch for tracking items. Also, do not use a batch if running in the
        // CLI.
        $use_batch = \Drupal::state()->get('search_api_use_tracking_batch', TRUE);
        if (!$use_batch || Utility::isRunningInCli()) {
          $index_task_manager->addItemsAll($this);
        }
        elseif (!defined('MAINTENANCE_MODE')
            || (!in_array(MAINTENANCE_MODE, ['install', 'update']))) {
          $index_task_manager->addItemsBatch($this);
        }
      }

So it's running in the CLI and IndexTaskManage::addItemsAll()

    $this->taskManager->executeAllTasks($this->getTaskConditions($index));

where IndexTaskManager::addItemsBatch would run

    $this->taskManager->setTasksBatch($this->getTaskConditions($index));

which looks like it sometimes also runs off the batch

Because all the startTracking stopTracking seems to also queue tasks?

stephen-cox commented 2 years ago

It's actually triggered by the saving of the config, which happens in Index:preSave(). The writeChangesToSettings method causes the reindex, although I have to admit I haven't found out how just changing the settings causes this.

stephen-cox commented 2 years ago

Haven't got anywhere turning off the re-indexing, so have a PR #202 that adds the datastore config directly to the config entity. This config then needs re-saving in the index for everything to work properly, it's far from ideal.

ekes commented 2 years ago

For the _db _solr work we've set the index to default disabled.

Doing this here also seems to help. It doesn't try and index when there's no backed yet. Sensible for installing.

It's run me into another problem we've had.

  Configuration objects provided by <em class="placeholder">localgov_directories_venue</em> have unmet dependencies: <em class="placeholder">field.field.node.localgov_directories_venue.localgov_directory_files (media.type.document)</em> 

Venue depends on configuration in optional in localgov_media

stephen-cox commented 2 years ago

This is fixed with #206