Uploaded image for project: 'Subversion'
  1. Subversion
  2. SVN-3596

'hotcopy' of fsfs repos may corrupt target rep-cache.db or revprops.db

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: 1.7.0
    • Component/s: libsvn_fs_fs
    • Labels:
      None

      Description

      (description copied from the attached thread)
      
      fs_fs.c:svn_fs_fs__hotcopy() uses this code to copy revprops.db:
      1597   /* Copy the packed revprop db. */
      1598   if (format >= SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT)
      1599     {
      1600       SVN_ERR(svn_io_dir_file_copy(src_subdir, dst_subdir, PATH_REVPROPS_DB,
      1601                                    pool));
      1602     }
      
      This post <http://thread.gmane.org/gmane.comp.db.sqlite.general/48391/focus=48423> 
      on the sqlite-users mailing list implies that an exclusive lock must be
      used for copying an sqlite db, and that plain copying might result in
      a corrupt database.
      
      As far as I can see, 'hotcopy' doesn't take the steps recommended
      in that (sub)thread to avoid corruption.
      
      Could someone more familiar with sqlite comment on this?  Could
      'hotcopy', as now written, potentially corrupt the revprops db
      (of the hotcopy target)?
      

      http://article.gmane.org/gmane.comp.version-control.subversion.devel/118341

        Issue Links

          Activity

          Hide
          philipm Philip Martin added a comment -

          I believe this issue is fixed in 1.7.  It has not be backported to 1.6.
          

          Show
          philipm Philip Martin added a comment - I believe this issue is fixed in 1.7. It has not be backported to 1.6.
          Hide
          danielsh Daniel Shahaf (äñ§€¥£¢) added a comment -

          Philip, can we mark this issue FIXED? (per #desc7)
          
          Has this been backported?
          

          Show
          danielsh Daniel Shahaf (äñ§€¥£¢) added a comment - Philip, can we mark this issue FIXED? (per #desc7) Has this been backported?
          Hide
          danielsh Daniel Shahaf (äñ§€¥£¢) added a comment -

          Could be related to this, and/or to issue #3506 (fixed in 1.6.12) which is about
          reducing the amount of locks.
          
          Can you please try 1.6.12, and post to the mailing list (CC me) if the problem
          persists?  We prefer having tech support discussions on the list rather than in
          the issue tracker.  Thanks.
          

          Show
          danielsh Daniel Shahaf (äñ§€¥£¢) added a comment - Could be related to this, and/or to issue #3506 (fixed in 1.6.12) which is about reducing the amount of locks. Can you please try 1.6.12, and post to the mailing list (CC me) if the problem persists? We prefer having tech support discussions on the list rather than in the issue tracker. Thanks.
          Hide
          subversion-importer Subversion Importer added a comment -

          We have a problem on our Windows server running svnserve... When we execute our 
          daily backup, we sometimes hit a snag when copying rep-cache.db:
          
          svnadmin: Can't open file '<path>\db\rep-cache.db': The process cannot access the 
          file because it is being used by another process.
          
          It looks like this could be related to this issue, since the lack of locks could 
          mean the file is in use at the time of the copying. Is this correct? We are using 
          SVN 1.6.11 at the moment.
          

          Original comment by danov

          Show
          subversion-importer Subversion Importer added a comment - We have a problem on our Windows server running svnserve... When we execute our daily backup, we sometimes hit a snag when copying rep-cache.db: svnadmin: Can't open file '<path>\db\rep-cache.db': The process cannot access the file because it is being used by another process. It looks like this could be related to this issue, since the lack of locks could mean the file is in use at the time of the copying. Is this correct? We are using SVN 1.6.11 at the moment. Original comment by danov
          Hide
          philipm Philip Martin added a comment -

          I spotted another problem with the rep-cache, it's copied after the rev files.
          That makes it possible for there to be a rep in the cache that is not present in
          the database. It's not obvious that such a repository is corrupt, verify will
          give OK, but a subsequent commit that includes the cached rep doesn't fail but
          leads to a repository that is corrupt.
          
          r933938 copies the rep-cache before the rev files to avoid this problem. It also
          introduces an SQLite hotcopy function that takes a read lock to avoid changes
          during the copy.
          

          Show
          philipm Philip Martin added a comment - I spotted another problem with the rep-cache, it's copied after the rev files. That makes it possible for there to be a rep in the cache that is not present in the database. It's not obvious that such a repository is corrupt, verify will give OK, but a subsequent commit that includes the cached rep doesn't fail but leads to a repository that is corrupt. r933938 copies the rep-cache before the rev files to avoid this problem. It also introduces an SQLite hotcopy function that takes a read lock to avoid changes during the copy.
          Hide
          hwright Hyrum Wright added a comment -

          The rep sharing cache is just a cache, and can be truncated or deleted without
          impact on the correctness of the system.
          

          Show
          hwright Hyrum Wright added a comment - The rep sharing cache is just a cache, and can be truncated or deleted without impact on the correctness of the system.
          Hide
          philipm Philip Martin added a comment -

          This problem also affects 1.6 with rep-sharing enabled as that uses SQLite for
          rep-cache.db (rep-sharing is enabled by default but can be disabled in
          db/fsfs.conf).
          

          Show
          philipm Philip Martin added a comment - This problem also affects 1.6 with rep-sharing enabled as that uses SQLite for rep-cache.db (rep-sharing is enabled by default but can be disabled in db/fsfs.conf).
          Hide
          hwright Hyrum Wright added a comment -

          We can set the minimum required SQLite version such that the backup API is
          available.  Is it still experimental in more recent versions of SQLite?
          
          [We should normally be pretty liberal with not using too-new dependency
          versions, but since we've made it possible for users to compile SQLite directly
          into Subversion, I think we can reasonably make an exception in its case.]
          

          Show
          hwright Hyrum Wright added a comment - We can set the minimum required SQLite version such that the backup API is available. Is it still experimental in more recent versions of SQLite? [We should normally be pretty liberal with not using too-new dependency versions, but since we've made it possible for users to compile SQLite directly into Subversion, I think we can reasonably make an exception in its case.]
          Hide
          philipm Philip Martin added a comment -

          Hmm, the backup API is new in 3.6.11 and experimental.  We probably should use
          it if it is available, but we need some other solution when it is not available.
           Perhaps we could use the exclusive repository lock (only if the revprops.db
          file exists and only while copy file).
          

          Show
          philipm Philip Martin added a comment - Hmm, the backup API is new in 3.6.11 and experimental. We probably should use it if it is available, but we need some other solution when it is not available. Perhaps we could use the exclusive repository lock (only if the revprops.db file exists and only while copy file).
          Hide
          philipm Philip Martin added a comment -

          Yes, I think this is a problem. The SQLite documentation provides a sample
          implementation of online backup:
          http://www.sqlite.org/backup.html
          using the C API:
          http://www.sqlite.org/c3ref/backup_finish.html
          We probably need to use something like this in svn_fs_fs__hotcopy.
          

          Show
          philipm Philip Martin added a comment - Yes, I think this is a problem. The SQLite documentation provides a sample implementation of online backup: http://www.sqlite.org/backup.html using the C API: http://www.sqlite.org/c3ref/backup_finish.html We probably need to use something like this in svn_fs_fs__hotcopy.

            People

            • Assignee:
              Unassigned
              Reporter:
              danielsh Daniel Shahaf (äñ§€¥£¢)
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development