Lucene - Core
  1. Lucene - Core
  2. LUCENE-485

IndexWriter.mergeSegments should not hold the commit lock while cleaning up.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0
    • Component/s: core/index
    • Labels:
      None
    • Environment:

      checked in revision 367361

      Description

      Same happens in IndexWriter.addIndexes(IndexReader[] readers).

      The commit lock should be obtained whenever the Index structure/version is read or written. It should be kept for as short a period as possible.

      The write lock is needed to make sure only one IndexWriter or IndexReader instance can update the index (multiple IndexReaders can of course use the index for searching).

      The list of files that can be deleted is stored in the file "deletable". It is only read or written by the IndexWriter instance that holds the write lock, so there's no need to have the commit lock to to update it.

      On my production system deleting the obsolete segment files after a mergeSegments() happens can occasionally take several seconds and the commit lock blocks the searcher machines from updating their IndexReader instance.
      Even on a standalone machine, the time to update the segments file is about 3ms, the time to delete the obsolete segments about 30ms.

      1. LUCENE-485.patch
        2 kB
        Luc Vanlerberghe

        Activity

        Hide
        Luc Vanlerberghe added a comment -

        This patch moves the calls to deleteFiles and deleteSegments outside of the blocks protected by a COMMIT lock.
        Since only one process can have the WRITE lock this extra locking is not necessary for those calls and the COMMIT lock is available for other processes faster.

        Show
        Luc Vanlerberghe added a comment - This patch moves the calls to deleteFiles and deleteSegments outside of the blocks protected by a COMMIT lock. Since only one process can have the WRITE lock this extra locking is not necessary for those calls and the COMMIT lock is available for other processes faster.
        Hide
        Doug Cutting added a comment -

        +1 This looks reasonable to me. What is important is that IndexReader holds the COMMIT lock while all files are opened. But the writer does not need to be held while they're removed, only while the set of files is changing, to keep readers from reading the set, then trying to open files after they've been deleted. This is still blocked after your patch, since the set cannot change while files are being opened.

        Show
        Doug Cutting added a comment - +1 This looks reasonable to me. What is important is that IndexReader holds the COMMIT lock while all files are opened. But the writer does not need to be held while they're removed, only while the set of files is changing, to keep readers from reading the set, then trying to open files after they've been deleted. This is still blocked after your patch, since the set cannot change while files are being opened.
        Hide
        Luc Vanlerberghe added a comment -

        Creating a TestCase that would show this is a valid patch is pretty difficult, but I'm 100% sure it is valid and I applied it to a production system a couple of months ago without any problem.
        Even if the lucene index is on a local harddisk, writing the updated segments file takes less than 10ms, while deleting the obsolete segments takes anywhere between 30 and 170ms... (on a Windows system that is...)

        Show
        Luc Vanlerberghe added a comment - Creating a TestCase that would show this is a valid patch is pretty difficult, but I'm 100% sure it is valid and I applied it to a production system a couple of months ago without any problem. Even if the lucene index is on a local harddisk, writing the updated segments file takes less than 10ms, while deleting the obsolete segments takes anywhere between 30 and 170ms... (on a Windows system that is...)
        Hide
        Doug Cutting added a comment -

        I committed this. Thanks, Luc!

        Show
        Doug Cutting added a comment - I committed this. Thanks, Luc!

          People

          • Assignee:
            Unassigned
            Reporter:
            Luc Vanlerberghe
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development