Currently if you were to add your SOLR environment (in our case our Acquia search environment) to a feature and download that code, you will find configuration like this:

$environment = new stdClass();
$environment->api_version = 1;
$environment->env_id = 'acquia_search_server_1';
$environment->name = 'Acquia Search';
$environment->url = 'http://search.acquia.com/solr/CENSORED';
$environment->service_class = 'AcquiaSearchService';
$environment->conf = array(
  'apachesolr_access_always_add_grants' => 0,
  'apachesolr_index_last' => array(
    'node' => array(
      'last_changed' => '1367415686',
      'last_entity_id' => '77769',
    ),
    'redirect' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
    'file' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
    'taxonomy_term' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
    'taxonomy_vocabulary' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
    'user' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
    'wysiwyg_profile' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
    'rules_config' => array(
      'last_changed' => 0,
      'last_entity_id' => 0,
    ),
  ),
  'apachesolr_index_updated' => 1367439249,
  'apachesolr_last_optimize' => 1367436713,
  'apachesolr_read_only' => '0',
   ...

Problem is if you are wanting to put all of your search configuration in code and deploy that across many sites (our case) this will cause feature overrides on all sites. Worse, if you revert that feature on multiple sites you're polluting all of those sites with stale data. This patch fixes that by excluding known array keys stored within the ctools export.

To reproduce:

* Create a search environment.
* Create a feature and add that environment to your features config.
* Download that feature and enable
* Run cron to index some content.
* Go to your feature status page; you will see overrides for these timestamps and entity ids

Also, I'm adding apachesolr_read_only as an exclusion because it's customary for companies to set their dev and/or staging servers to read-only. Removing this from configuration allows each environment to maintain that setting.

Comments

blazindrop’s picture

StatusFileSize
new726 bytes
blazindrop’s picture

Status: Active » Needs review
nick_vh’s picture

Status: Needs review » Needs work
+++ b/apachesolr.moduleundefined
@@ -2787,5 +2787,11 @@ function apachesolr_ctools_environment_export($environment, $indent) {
+  ¶
+  // remove time dependent variables and entity state from code
+  foreach (array('apachesolr_index_last', 'apachesolr_index_updated', 'apachesolr_last_optimize', 'apachesolr_read_only') as $key) {
+  	unset($environment->conf[$key]);
+  }

bunch of spacing issues

Aside of that, it looks good

blazindrop’s picture

Status: Needs work » Needs review
StatusFileSize
new768 bytes

Sorry about the rookie whitespace issues :) Was a vim issue.

sylvain lecoy’s picture

Status: Needs review » Reviewed & tested by the community

Works for me. Thanks !

blazindrop’s picture

Status: Reviewed & tested by the community » Needs work

I'm putting this back to needs work because in testing excluding read only from the ctools export configuration may not be the most reliable way to handle having one server environment (e.g. dev, staging or production) depend on the state of the index (e.g. read/write or read only).

The best way to handle this is to duplicate your original environment via the clone operation. You set that to read only and then include that in your features/ctools exportables. If you leverage the apachesolr_default_environment variable (set in "variable" not "apachesolr_environment_variable") you can write a cloud hook to switch this on db copy (if you are on Acquia managed cloud) or come up with a similar recipe if you are not.

New patch to follow.

blazindrop’s picture

Status: Needs work » Needs review
StatusFileSize
new744 bytes

New patch. Removing 'apachesolr_read_only" from the exclude list; otherwise same as patch in #4.

blazindrop’s picture

Status: Needs review » Needs work

Ok so back to "needs work" :)

Now the issue is even with this patch and the fact we're excluding this configuration from the ctools export, when the module goes to set any of those variables (e.g. index last updates as in below):

mysql> select * from apachesolr_environment_variable
    -> ;
+--------------------------+--------------------------+---------------+
| env_id                   | name                     | value         |
+--------------------------+--------------------------+---------------+
| search_server_1_0 | apachesolr_index_updated | i:1367953222; |
+--------------------------+--------------------------+---------------+
1 row in set (0.00 sec)

... ctools sees this as an override. If you try to revert, your configuration never reverts because ctools doesn't know (I'm assuming) that this variable should be removed. Clearing out this table (thus removing the one variable above) and then doing a revert brings it back to default state. Didn't run into this until I tried to revert across our 20 dev sites.

What I would like to do is create some patches that move these transient variables (e.g. time dependent ones like timestamps) out of the apachesolr_environment_variable table and place in the variables table (of course making them environment specific). Thoughts?

sylvain lecoy’s picture

You need to use Doctrine @Transient meta-data :-)

No really the first patch did it for me and since I don't care about ctools because I think it brings more chaos than tools in projects, I am OK implementing quick fixes with this stuff. In Drupal 8 this will be removed in favor of CMI (Configuration Management Initiative).

bleen’s picture

I think that the only reasonable way to fix this might be to break out those values (like apachesolr_index_updated) into seperate variables. Of course we would need one variable per SOLR env (or maybe one var that is an array keyed on the SOLR env).

In any case, this becomes a pretty big change.

steven merrill’s picture

Here's a quick hack in the meantime:

/**
 * Implements hook_apachesolr_environments_alter().
 *
 * Code needed to prevent overridden Features between environments.
 * ApacheSOLR module puts some time-related data into the Features
 * exports, which would normally cause things to go overridden right
 * away.
 */
function YOURMODULE_apachesolr_environments_alter(&$data) {
  // Load the real data with the real temporal information.
  $environments = apachesolr_load_all_environments();

  // These keys are purely time-based and do not need to be considered.
  $ephemeral_items = array(
    'apachesolr_index_last',
    'apachesolr_index_updated',
    'apachesolr_last_optimize',
  );

  // Update the default configs with the current temporal configs so that these
  // keys do not cause these items to be overridden.
  foreach ($data as $key => $value) {
    $index = &$data[$key];
    foreach($ephemeral_items as $ephemeral_item) {
      if (isset($environments[$key]['conf'][$ephemeral_item])) {
        $index->conf[$ephemeral_item] = $environments[$key]['conf'][$ephemeral_item];
      }
    }
  }
}
bleen’s picture

#11 works until there is a more permanent fix.

smerrill++

bleen’s picture

Issue summary: View changes

clarify revert

blazindrop’s picture

@Steven, nice attempt there in #11 - I like that recipe. I think that takes care of altering the ctools export to try and prevent features from showing an override, but I think the override still gets applied since I see those three ephemeral fields in the database:

mysql> select * from apachesolr_environment_variable\G
*************************** 1. row ***************************
env_id: search_server_1
  name: apachesolr_index_last
 value: a:1:{s:4:"node";a:2:{s:12:"last_changed";s:10:"1389277194";s:14:"last_entity_id";s:6:"216851";}}
*************************** 2. row ***************************
env_id: search_server_1
  name: apachesolr_index_updated
 value: i:1389278702;
*************************** 3. row ***************************
env_id: search_server_1
  name: apachesolr_last_optimize
 value: i:1389242702;
3 rows in set (0.00 sec)

Since these overrides are in this database table none of the settings in code (e.g. field boosts, environment settings, etc.) are not applied. I think a true fix would involve rewriting code to not store these ephemeral values in the ctools-backed configuration.

myselfhimself’s picture

Hello
I have applied #11, hand-edited my feature's feature_name.apachesolr_environments.inc to empty the $environment->conf array, then pushed the modified feature. That removed the overriden state for the feature, on our staging environment. However doing a features update (drush fu feature_name -y), refills the $environment->conf array with ephemeral variables.

blazindrop’s picture

@myselfhimself - I wish I had more time to contribute but like bleen18 said a rewrite of configuration handing is needed to correct this. We wound up removing the environments from features completely and scripting the configuration deployment. Any time this override happened with us none of our other settings were applied and this was a big issue for us to happen on ~ 26 sites.

nick_vh’s picture

So, spoke about this with some folks and it appears that we need to split this off from the variable environment table.

Step 1) Create a new ephemeral config table that is not exported
Step 2) Change the apachesolr_get_last_index_position + apachesolr_set_last_index_position and other ephemeral variable functions so it fetches from this new table.
Step 3) Write update function so that people will be able to use this if they upgrade

rosk0’s picture

Completely agree with #16.

Also want to say that maybe I didn't understood author of #7 completely, but this patch(from #7) fixes the issue in very correct way, at least for current module state. Time dependent variables from `apachesolr_environment_variable` doesn't get included in ctools export, so they could not mark feature as overridden no matter what value they will have.
So patch #7 is a good fix until changes from #16 get in.

blazindrop’s picture

I can explain my comment in #7 further. I actually am deprecating the patch in #7 because it still causes a feature override (read comment #8 as it has the full details). If you apply patch #7 your symptoms will be the rest of your configuration for the environment (content type bias, boosts, etc) will not get applied.

Nick_vh is definitely on the ideal path. For the interim what we did on our 20+ sites is deploy search configuration with a PHP script that we run anytime we have an environment change. The script merely extracts the serialized objects representing the configuration, unserializes the object, sets the new value, reserializes and updates the DB table. Here's an example of a field weight change we had to do across many sites (call me lazy I didn't want to manually update 20+ sites) :)

<?php

$res = db_select('apachesolr_environment_variable', 'v')
  ->fields('v', array('value'))
  ->condition('env_id', 'acquia_search_server_1', '=')
  ->condition('name', 'field_bias', '=')
  ->execute()
  ->fetch();

$config = unserialize($res->value);
$config['ts_author'] = "0.8";
$res = serialize($config);
db_update('apachesolr_environment_variable')
  ->fields(array('value' => $res))
  ->condition('env_id', 'acquia_search_server_1', '=')
  ->condition('name', 'field_bias', '=')
  ->execute();
bforchhammer’s picture

Status: Needs work » Needs review
StatusFileSize
new3.54 KB

I actually am deprecating the patch in #7 because it still causes a feature override

I'm new to this issue, but from what I can see, the "feature override" state is being determined in apachesolr_environment_load_subrecords().

Attached patch tries to fix the problem by making sure that state variables are ignored when comparing in-code with in-database configuration... I haven't tested the patch extensively yet, but it seems to do the job for me. :-)

bforchhammer’s picture

When testing my patch above, note that "reverting" is currently not entirely reliable and may not always work; see #1439564-44: Index bundles Export/Import. You may want to apply the patch over there as well.

alcroito’s picture

I tested #19 and it seems to work for me, but I also applied the patch @bforchhammer mentioned in the thread he posted, so ctools can properly do diffs between code and database, and be able to revert them properly.

nick_vh’s picture

Status: Needs review » Postponed
bforchhammer’s picture

Status: Postponed » Needs review

#1439564: Index bundles Export/Import has been committed so this should not be blocked anymore. Patch still applies and should still work, so no re-roll required.

  • Commit 364f3c6 on 7.x-1.x authored by bforchhammer, committed by Nick_vh:
    Issue #1984502 by blazindrop, bforchhammer: Fixed Don't include time...
nick_vh’s picture

Version: 7.x-1.x-dev » 6.x-3.x-dev
Status: Needs review » Patch (to be ported)

Committed. Not sure if it needs backport to 6.x-3.x Please close it if it doesn't need a backport

Thanks for your contributions! It's very much appreciated and I hope you continue to contribute to our issue queue. We can use all the help we can get.