Mailing-List: contact dev-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@lucene.apache.org
Date: Wed, 13 Feb 2013 00:11:15 +0000 (UTC)
From: "Gregg Donovan (JIRA)" <jira@apache.org>
To: dev@lucene.apache.org
Message-ID: <JIRA.12631261.1360245195370.281415.1360714275552@arcas>
In-Reply-To: <JIRA.12631261.1360245195370@arcas>
References: <JIRA.12631261.1360245195370@arcas>
Subject: [jira] [Commented] (SOLR-4413) SolrCore#getIndexDir() contract
 change between 3.6 and 4.1
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/SOLR-4413?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13577200#comment-13577200 ] 

Gregg Donovan commented on SOLR-4413:
-------------------------------------

bq. Could you elaborate on these problems?

*SolrDeletionPolicy*
In Solr 4.1, a core reload sets the new deletion policy to be the previous core's {{IndexDeletionPolicyWrapper}}:
{code:java}
     SolrCore core = new SolrCore(getName(), getDataDir(), config,
         schema, coreDescriptor, updateHandler, prev);
     core.solrDelPolicy = this.solrDelPolicy;
     return core;
{code}

This would cause problems when trying to get the commit points because the commits from the previous directory would be inherited. In our case, we would start with an empty index at {{index/}}. Then, after a reindex, we would have a new index at {{index-<timestamp>/}} at generation 2 (we do one incremental after a reindex before swapping it in). When the ReplicationHandler tried to find the commits for generation 2 during replication, it couldn't find them. {{core.getDeletionPolicy().getCommits()}} would yield the map for {{index/}}, not {{index-<timestamp>/}}.


*Directory*
Solr 4.1 has an optimization to use the {{IndexWriter}} from the previous {{SolrCore}} to create a new {{DirectoryReader}}:
{code:java}
      // use the (old) writer to open the first searcher
      RefCounted<IndexWriter> iwRef = null;
      if (prev != null) {
        iwRef = prev.getUpdateHandler().getSolrCoreState().getIndexWriter(null);
        if (iwRef != null) {
          final IndexWriter iw = iwRef.get();
          newReaderCreator = new Callable<DirectoryReader>() {
            @Override
            public DirectoryReader call() throws Exception {
              return DirectoryReader.open(iw, true);
            }
          };
        }
      }
{code}
In our case, the old core refers to {{index/}} and the core being created is at {{index-<timestamp>/}}, so the {{DirectoryReader}} from the old core points to a directory that is no longer in use. After doing the index.properties update and core reload, the newly created {{SolrIndexSearcher}} would still point to {{index/}}. It would take an empty commit to get a new reader pointed at {{index-<timestamp>/}}.


*UpdateHandler*
The {{UpdateHandler}} has a reference to the {{SolrCoreState}}/{{DefaultSolrCoreState}} which has a reference to the {{SolrIndexWriter}}. So, when we inherited the {{UpdateHandler}} from {{index/}} after the reload pointed to {{index-<timestamp>/}}, commits would still go to {{index/}}.

It's unfortunate to lose all of the stats in {{DirectUpdateHandler2}}, but maybe it makes sense to reset them anyway when the {{Directory}} changes?

*DirectoryFactory*
I should clarify: the reason for not inheriting {{DirectoryFactory}} from the previous core was not actually related to state. Rather, the current logic in 4.1 for whether to inherit the {{DirectoryFactory}} is:
{code:java}
      if (updateHandler == null) {
        initDirectoryFactory();
        solrCoreState = new DefaultSolrCoreState(getDirectoryFactory());
      } else {
        solrCoreState = updateHandler.getSolrCoreState();
        directoryFactory = solrCoreState.getDirectoryFactory();
        this.isReloaded = true;
      }
{code}

I removed inheriting of the {{DirectoryFactory}} of the previous core because {{SolrCore#getNewIndexDir()}} uses {{this.directoryFactory}} to read {{index.properties}} and {{getNewIndexDir()}} was needed to determine whether the new core was using the same directory as the previous core. Perhaps this can be solved by creating a private version of {{SolrCore#getNewIndexDir}} that takes a {{DirectoryFactory}} as an argument? On second look I don't think I see state in {{DirectoryFactory}} that should cause problems when a new directory is used.


> SolrCore#getIndexDir() contract change between 3.6 and 4.1
> ----------------------------------------------------------
>
>                 Key: SOLR-4413
>                 URL: https://issues.apache.org/jira/browse/SOLR-4413
>             Project: Solr
>          Issue Type: Bug
>    Affects Versions: 4.0, 4.1
>            Reporter: Gregg Donovan
>            Assignee: Mark Miller
>             Fix For: 4.2, 5.0
>
>         Attachments: SOLR-4413.patch
>
>
> In [SVN 1420992|http://svn.apache.org/viewvc?view=revision&revision=1420992], {{SolrCore#getIndexDir()}} changed it's implementation from a version that would reflect the value of the index property in {{index.properties}} to one that does not. 
> In 3.6, {{SolrCore#getIndexDir()}} was:
> {code:java}
> public String getIndexDir() {
>   synchronized (searcherLock) {
>     if (_searcher == null)	  	
>       return dataDir + "index/";
>     SolrIndexSearcher searcher = _searcher.get();
>     return searcher.getIndexDir() == null ? dataDir + "index/" : searcher.getIndexDir();
> }
> {code}
> In 3.6, {{SolrIndexSearcher}} would be passed the value of {{SolrCore#getNewIndexDir()}} -- which reads index.properties -- in its constructor and return it when {{SolrIndexSearcher#getIndexDir()}} was called.
>  
> In 4.1, {{SolrCore#getIndexDir()}} is:
> {code:java}
>   public String getIndexDir() {  
>     return dataDir + "index/";
>   }
> {code}
> Clients of {{SolrCore#getIndexDir()}} that were expecting the previous behavior are likely to have issues. E.g.:
> --In {{CoreAdminHandler#handleUnloadAction(SolrQueryRequest, SolrQueryResponse)}} if the {{deleteIndex}} flag is set to true, it calls {{core.getDirectoryFactory().remove(core.getIndexDir())}}. If a value other than {{index/}} is set in {{index.properties}}, the wrong directory will be deleted.
> --In {{CoreAdminHandler#getIndexSize(SolrCore)}}, the existence of {{SolrCore#getIndexDir()}} is checked before {{SolrCore#getNewIndexDir()}}. If a value other than {{index/}} is set in {{index.properties}}, this will return the size of the wrong directory.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org