Solr
  1. Solr
  2. SOLR-2156

Solr Replication - SnapPuller fails to clean Old Index Directories on Full Copy

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: replication (java)
    • Labels:
      None

      Description

      We are working on the Solr trunk ....and have a Master and Two slaves configuration .....
      Our indexing consists of Periodic Full and Incremental index building on the master and replication on the slaves.

      When a Full indexing (clean and rebuild) is performed, we always end with an extra index folder copy, which holds the complete index and hence the size just grows on, on the slaves.

      e.g.
      drwxr-xr-x 2 tomcat tomcat 4096 2010-10-09 12:10 index
      drwxr-xr-x 2 tomcat tomcat 4096 2010-10-11 09:43 index.20101009120649
      drwxr-xr-x 2 tomcat tomcat 4096 2010-10-12 10:27 index.20101011094043
      rw-rr- 1 tomcat tomcat 75 2010-10-11 09:43 index.properties
      rw-rr- 1 tomcat tomcat 422 2010-10-12 10:26 replication.properties
      drwxr-xr-x 2 tomcat tomcat 68 2010-10-12 10:27 spellchecker

      Where index.20101011094043 is the active index and the other index.xxx directories are no more used.

      The SnapPuller deletes the temporary Index directory, but does not delete the old one when the switch is performed for the full copy.

      The below code should do the trick.

      boolean fetchLatestIndex(SolrCore core) throws IOException

      { .......... }

      finally {
      if(deleteTmpIdxDir)

      { delTree(tmpIndexDir); }

      else

      { // Delete the old index directory, as the flag is set only after the full copy is performed delTree(indexDir); }

      }
      .........
      }

        Issue Links

          Activity

          Hide
          Jayendra Patil added a comment -

          Attached the Fix.

          Show
          Jayendra Patil added a comment - Attached the Fix.
          Hide
          Noble Paul added a comment -

          The patch looks good..

          Show
          Noble Paul added a comment - The patch looks good..
          Hide
          Jayendra Patil added a comment -

          There are different conditions in which the flag gets set to false.

          If the Index is stale (Common Index files of slave does not exist in master
          or are of different size, or with different timestamp - isIndexStale method)
          or more newer than the master (slave generation > master generation) a full
          index download is performed.

          In our case we do a clean build so the files on on slave don't exist on
          master and hence the flag is set to false.

          Show
          Jayendra Patil added a comment - There are different conditions in which the flag gets set to false. If the Index is stale (Common Index files of slave does not exist in master or are of different size, or with different timestamp - isIndexStale method) or more newer than the master (slave generation > master generation) a full index download is performed. In our case we do a clean build so the files on on slave don't exist on master and hence the flag is set to false.
          Hide
          Bill Bell added a comment -

          Question:

          Are we setting "successfulInstall" and "deleteTmpIdxDir" properly when replicating config files in method fetchLatestIndex ?

          Shall we replace:

                   if (isFullCopyNeeded) {
                      modifyIndexProps(tmpIndexDir.getName());
                    } else {
                      successfulInstall = copyIndexFiles(tmpIndexDir, indexDir);
                    }
          

          With:

          
                   if (isFullCopyNeeded) {
                      successfulInstall = modifyIndexProps(tmpIndexDir.getName());
                      deleteTmpIdxDir =  false;
                    } else {
                      successfulInstall = copyIndexFiles(tmpIndexDir, indexDir);
                    }
          
          

          THis might be related to the same issue.

          Show
          Bill Bell added a comment - Question: Are we setting "successfulInstall" and "deleteTmpIdxDir" properly when replicating config files in method fetchLatestIndex ? Shall we replace: if (isFullCopyNeeded) { modifyIndexProps(tmpIndexDir.getName()); } else { successfulInstall = copyIndexFiles(tmpIndexDir, indexDir); } With: if (isFullCopyNeeded) { successfulInstall = modifyIndexProps(tmpIndexDir.getName()); deleteTmpIdxDir = false ; } else { successfulInstall = copyIndexFiles(tmpIndexDir, indexDir); } THis might be related to the same issue.
          Hide
          Alexander Kanarsky added a comment -

          Bill, this was mentioned in SOLR-1264 (see last comment there) and reopened as SOLR-1983 (still not fixed).

          -Alexander

          Show
          Alexander Kanarsky added a comment - Bill, this was mentioned in SOLR-1264 (see last comment there) and reopened as SOLR-1983 (still not fixed). -Alexander
          Hide
          Bill Bell added a comment -

          You added the additional patch after the issue was closed. It will not be picked up.

          This is an IMPORTANT issue that needs to be added to the trunk. Can a committer please add it?

          https://issues.apache.org/jira/secure/attachment/12448101/SOLR-1264-additional.patch

          Show
          Bill Bell added a comment - You added the additional patch after the issue was closed. It will not be picked up. This is an IMPORTANT issue that needs to be added to the trunk. Can a committer please add it? https://issues.apache.org/jira/secure/attachment/12448101/SOLR-1264-additional.patch
          Hide
          Alexander Kanarsky added a comment -

          > You added the additional patch after the issue was closed. It will not be picked up.

          Yes, that's (in my understanding) exactly why the SOLR-1983 was opened
          I do not know why Hoss did not move the patch there, though. I moved it to SOLR-1983 now.

          Show
          Alexander Kanarsky added a comment - > You added the additional patch after the issue was closed. It will not be picked up. Yes, that's (in my understanding) exactly why the SOLR-1983 was opened I do not know why Hoss did not move the patch there, though. I moved it to SOLR-1983 now.
          Hide
          Yonik Seeley added a comment -

          Thanks, I just committed this!

          Show
          Yonik Seeley added a comment - Thanks, I just committed this!
          Hide
          Grant Ingersoll added a comment -

          Bulk close for 3.1.0 release

          Show
          Grant Ingersoll added a comment - Bulk close for 3.1.0 release

            People

            • Assignee:
              Yonik Seeley
              Reporter:
              Jayendra Patil
            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development