Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-2578

ReplicationHandler Backups -- clean up old backups

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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
        jdyer 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
        jdyer 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).
        Hide
        rcmuir Robert Muir added a comment -

        3.4 -> 3.5

        Show
        rcmuir Robert Muir added a comment - 3.4 -> 3.5
        Hide
        hossman 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
        hossman 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
        jdyer James Dyer added a comment -

        Here's a patch for 3.x

        Show
        jdyer James Dyer added a comment - Here's a patch for 3.x
        Hide
        markrmiller@gmail.com Mark Miller added a comment -

        so I plan on committing to trunk

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

        Show
        markrmiller@gmail.com Mark Miller added a comment - so I plan on committing to trunk +1 - only took a quick scan, but looks nice.
        Hide
        hossman 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
        hossman 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
        jdyer James Dyer added a comment -

        Thanks for committing this. I have updated the wiki.

        Show
        jdyer James Dyer added a comment - Thanks for committing this. I have updated the wiki.
        Hide
        thetaphi Uwe Schindler added a comment -

        Bulk close after 3.5 is released

        Show
        thetaphi Uwe Schindler added a comment - Bulk close after 3.5 is released
        Hide
        nhooey 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
        nhooey 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
        nhooey Neil Hooey added a comment -

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

        -   if( i > numberToKeep-1 ) {
        +   if( i++ > numberToKeep-1 ) {
        
        Show
        nhooey Neil Hooey added a comment - Just tested, and it works if you change this line: - if ( i > numberToKeep-1 ) { + if ( i++ > numberToKeep-1 ) {
        Hide
        jdyer 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
        jdyer James Dyer added a comment - Thanks for pointing this out, Neil. I opened SOLR-3168 for this bug and will commit the fix shortly.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development