Solr
  1. Solr
  2. SOLR-1068

Use fsync on replicated index and configuration files

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4
    • Component/s: replication (java)
    • Labels:
      None

      Description

      Related discussion on solr-dev at http://markmail.org/thread/f2axi3ffk3ueudpm

      1. SOLR-1068.patch
        9 kB
        Shalin Shekhar Mangar
      2. SOLR-1068.patch
        10 kB
        Noble Paul
      3. SOLR-1068.patch
        6 kB
        Noble Paul
      4. SOLR-1068.patch
        6 kB
        Shalin Shekhar Mangar
      5. SOLR-1068.patch
        4 kB
        Shalin Shekhar Mangar

        Activity

        Hide
        Shalin Shekhar Mangar added a comment -
        1. Adds a method sync(File file) to FileUtils with code copied over from Lucene's Directory.sync()
        2. SnapPuller#copyIndexFiles calls sync after moving index files from tmp location to the index directory
        3. SnapPuller#copyTmpConfFiles2Conf calls sync after moving conf files from tmp location to the conf directory
        Show
        Shalin Shekhar Mangar added a comment - Adds a method sync(File file) to FileUtils with code copied over from Lucene's Directory.sync() SnapPuller#copyIndexFiles calls sync after moving index files from tmp location to the index directory SnapPuller#copyTmpConfFiles2Conf calls sync after moving conf files from tmp location to the conf directory
        Hide
        Shalin Shekhar Mangar added a comment -

        Committed revision 757209.

        Show
        Shalin Shekhar Mangar added a comment - Committed revision 757209.
        Hide
        Yonik Seeley added a comment -

        Actually, the sync should be before the files are moved.... otherwise if there is a crash before the sync is called or completed, you end up in the situation of having neither the old or new files (for certain filesystems).

        From the email thread:

        It seems like we would want to throw a file to the sync executor as soon as we've downloaded it. Then wait until all files are sync'd before moving everything out of the temp directory.

        Show
        Yonik Seeley added a comment - Actually, the sync should be before the files are moved.... otherwise if there is a crash before the sync is called or completed, you end up in the situation of having neither the old or new files (for certain filesystems). From the email thread: It seems like we would want to throw a file to the sync executor as soon as we've downloaded it. Then wait until all files are sync'd before moving everything out of the temp directory.
        Hide
        Shalin Shekhar Mangar added a comment -

        Sorry, I should have paid more attention. I'll take another stab at it.

        Show
        Shalin Shekhar Mangar added a comment - Sorry, I should have paid more attention. I'll take another stab at it.
        Hide
        Shalin Shekhar Mangar added a comment -

        Another attempt.

        1. Uses an single thread executor for syncing the files in the background
        2. Files are passed on to the executor immediately after they are downloaded and before they are moved
        3. The executor is created at the start of a pull and shutdown immediately after.
        Show
        Shalin Shekhar Mangar added a comment - Another attempt. Uses an single thread executor for syncing the files in the background Files are passed on to the executor immediately after they are downloaded and before they are moved The executor is created at the start of a pull and shutdown immediately after.
        Hide
        Noble Paul added a comment -

        do we really need a FsyncService class ? can't we use an ExecutorService directly? the ExecutorService #awaitTermination() wouldwait for all the tasks to complete and return .

        Show
        Noble Paul added a comment - do we really need a FsyncService class ? can't we use an ExecutorService directly? the ExecutorService #awaitTermination() wouldwait for all the tasks to complete and return .
        Hide
        Noble Paul added a comment -

        not tested. but isn't this enough ?

        Show
        Noble Paul added a comment - not tested. but isn't this enough ?
        Hide
        Noble Paul added a comment -

        last patch was incorrect

        Show
        Noble Paul added a comment - last patch was incorrect
        Hide
        Shalin Shekhar Mangar added a comment -

        Updating to trunk. I'll commit this shortly.

        Show
        Shalin Shekhar Mangar added a comment - Updating to trunk. I'll commit this shortly.
        Hide
        Shalin Shekhar Mangar added a comment -

        Committed revision 761577 and 761578.

        Thanks Yonik and Noble!

        Show
        Shalin Shekhar Mangar added a comment - Committed revision 761577 and 761578. Thanks Yonik and Noble!
        Hide
        Grant Ingersoll added a comment -

        Bulk close Solr 1.4 issues

        Show
        Grant Ingersoll added a comment - Bulk close Solr 1.4 issues

          People

          • Assignee:
            Shalin Shekhar Mangar
            Reporter:
            Shalin Shekhar Mangar
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development