Solr
  1. Solr
  2. SOLR-2578

ReplicationHandler Backups -- clean up old backups

    Details

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

      Description

      It would be nice when performing backups if there was an easy way to tell ReplicationHandler to only keep so many and then delete the older ones.

      1. SOLR-2578_3x.patch
        10 kB
        James Dyer
      2. SOLR-2578.patch
        13 kB
        Hoss Man
      3. SOLR-2578.patch
        10 kB
        James Dyer

        Activity

        Hide
        James Dyer added a comment -

        Thanks for pointing this out, Neil. I opened SOLR-3168 for this bug and will commit the fix shortly.

        Show
        James Dyer added a comment - Thanks for pointing this out, Neil. I opened SOLR-3168 for this bug and will commit the fix shortly.
        Hide
        Neil Hooey added a comment -

        Just tested, and it works if you change this line:

        -   if( i > numberToKeep-1 ) {
        +   if( i++ > numberToKeep-1 ) {
        
        Show
        Neil Hooey added a comment - Just tested, and it works if you change this line: - if ( i > numberToKeep-1 ) { + if ( i++ > numberToKeep-1 ) {
        Hide
        Neil Hooey added a comment -

        This doesn't seem to work for me on trunk.

        It only deletes snapshots if you set numberToKeep=1.

        I checked the related commit, and the code is missing an increment of i:

        // In: solr/core/src/java/org/apache/solr/handler/SnapShooter.java
        
        int i=1;
        for(OldBackupDirectory dir : dirs) {
          if( i > numberToKeep-1 ) {
            SnapPuller.delTree(dir.dir);
          }
        }   
        
        Show
        Neil Hooey added a comment - This doesn't seem to work for me on trunk. It only deletes snapshots if you set numberToKeep=1. I checked the related commit, and the code is missing an increment of i: // In: solr/core/src/java/org/apache/solr/handler/SnapShooter.java int i=1; for (OldBackupDirectory dir : dirs) { if ( i > numberToKeep-1 ) { SnapPuller.delTree(dir.dir); } }
        Hide
        Uwe Schindler added a comment -

        Bulk close after 3.5 is released

        Show
        Uwe Schindler added a comment - Bulk close after 3.5 is released
        Hide
        James Dyer added a comment -

        Thanks for committing this. I have updated the wiki.

        Show
        James Dyer added a comment - Thanks for committing this. I have updated the wiki.
        Hide
        Hoss Man added a comment -

        James: thanks for the 3x patch, but the svn merge from trunk went clean so i just went that route.

        Committed revision 1202969.
        Committed revision 1203003.

        Show
        Hoss Man added a comment - James: thanks for the 3x patch, but the svn merge from trunk went clean so i just went that route. Committed revision 1202969. Committed revision 1203003.
        Hide
        Mark Miller added a comment -

        so I plan on committing to trunk

        +1 - only took a quick scan, but looks nice.

        Show
        Mark Miller added a comment - so I plan on committing to trunk +1 - only took a quick scan, but looks nice.
        Hide
        James Dyer added a comment -

        Here's a patch for 3.x

        Show
        James Dyer added a comment - Here's a patch for 3.x
        Hide
        Hoss Man added a comment -

        Updated patch to trunk and cleaned up formatting (ie: removed tabs, excessive newlines)

        Path looks decent to me, and has a test that passes so I plan on committing to trunk unless anyone objects

        (I haven't yet assessed how hard backporting will be – not an area of the code i'm super knowledgeable on)

        Show
        Hoss Man added a comment - Updated patch to trunk and cleaned up formatting (ie: removed tabs, excessive newlines) Path looks decent to me, and has a test that passes so I plan on committing to trunk unless anyone objects (I haven't yet assessed how hard backporting will be – not an area of the code i'm super knowledgeable on)
        Hide
        Robert Muir added a comment -

        3.4 -> 3.5

        Show
        Robert Muir added a comment - 3.4 -> 3.5
        Hide
        James Dyer added a comment -

        This patch adds the functionality with a new parameter: numberToKeep . The unit test has been enhanced to do 2 backups and then check to see if the first one was automatically deleted (numberToKeep=1).

        Show
        James Dyer added a comment - This patch adds the functionality with a new parameter: numberToKeep . The unit test has been enhanced to do 2 backups and then check to see if the first one was automatically deleted (numberToKeep=1).

          People

          • Assignee:
            James Dyer
            Reporter:
            James Dyer
          • Votes:
            2 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development