As designed \XFES\Service\Optimizer::optimize createIndex is not called with default index configuration

X

Xon

Well-known member
Affected version
2.1.1
The optimize() function is called with an empty $settings array during actionOptimize() and AbstractSource::truncate(), which results in the index being created with mappings and then index wide config being applied. If custom index configuration which mappings are dependent on is being supplied, this results in an error.
PHP: 
public function optimize(array $settings = [], $updateConfig = false)
{
   $config = [];

   if ($this->es->indexExists())
   {
      $config = $this->es->getIndexInfo();
      $this->es->deleteIndex();
      $this->db()->emptyTable('xf_es_index_failed');
   }

   if ($settings)
   {
      if (isset($config['settings']))
      {
         $config['settings'] = array_replace_recursive($config['settings'], $settings);
      }
      else
      {
         $config['settings'] = $settings;
      }
   }

   // if we create an index in ES6+, we must force it to be single type
   $this->es->forceSingleType($this->es->majorVersion() >= 6);

   $config['mappings'] = $this->getExpectedMappingConfig();

   $this->es->createIndex($config);

Instead of getIndexInfo(), this sort of code should occur before the indexExists() check;
PHP: 
if (!$settings)
{
   /** @var \XFES\Service\Configurer $configurer */
   $configurer = $this->service('XFES:Configurer', $this->es);
   $analyzerConfig = $configurer->getAnalyzerConfig();

   /** @var \XFES\Service\Analyzer $analyzer */
   $analyzer = $this->service('XFES:Analyzer', $this->es);
   $settings = $analyzer->getAnalyzerFromConfig($analyzerConfig);
}

This way extending getAnalyzerFromConfig() is all that is required to ensure custom index configuration is persisted across index rebuilds
 
We're actually somewhat concerned that the proposed change here may have some unintended consequences. The current approach here with getIndexInfo is to try to maintain the index configuration from ES itself (replication, shards, etc), except for the mappings (and settings if provided).

We think the proposed changes may wipe out things that might have been done to the index config outside of XFES which is what the current approach was trying to maintain.

Ultimately we'd need a very specific reproduction case, including the specific ES release details, to be able to comment further but as it stands we feel the current approach is correct.
 
My use case is in MultiPrefix I add a custom field mapping with a custom analyzer. The custom analyzer must exist before you define the field (ie after index creation), which in XF2 just doesn't work if you click the 'optimize' button but does work for normal setup

Keep in mind, optimize() blows away the entire index anyway. So keeping index configuration simply doesn't happen if there is no $settings passed into the function.
 
If no settings are passed in, then the settings would come from the existing index, which might include customizations such as sharding or replication changes and various other custom settings, but also the full analyzer configuration, which could (theoretically) include something beyond what XF has done itself. The proposed change would lose all the existing custom stuff.

If I'm understanding properly, then provided your custom analyzer was already specified on the index, it would be maintained by the existing code and created with the index, alongside the mappings. If Elasticsearch is rejecting the mapping despite the analyzer being setup at the same time, that seems like a definite bug on their end.

At this time, I don't see reason to change anything here.
 
Back
Top Bottom