Lucene - Core
  1. Lucene - Core
  2. LUCENE-4986

NRT reader doesn't see changes after successful IW.tryDeleteDocument

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3.1, Trunk
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Reported by Reg on the java-user list, subject "TrackingIndexWriter.tryDeleteDocument(IndexReader, int) vs deleteDocuments(Query)":

      When IW.tryDeleteDocument succeeds, it marks the document as deleted in the pending BitVector in ReadersAndLiveDocs, but then when the NRT reader checks if it's still current by calling IW.nrtIsCurrent, we fail to catch changes to the BitVector, resulting in the NRT reader thinking it's current and not reopening.

      1. LUCENE-4986.patch
        6 kB
        Michael McCandless
      2. LUCENE-4986.patch
        14 kB
        Michael McCandless

        Activity

        Hide
        Michael McCandless added a comment -

        Patch with test case showing the failure, from Reg (I tweaked it a bit to adapt to trunk APIs).

        I think to fix this, the NRT reader should also (only?) track the changeCount that IndexWriter uses.

        Show
        Michael McCandless added a comment - Patch with test case showing the failure, from Reg (I tweaked it a bit to adapt to trunk APIs). I think to fix this, the NRT reader should also (only?) track the changeCount that IndexWriter uses.
        Hide
        Michael McCandless added a comment -

        Patch w/ fix, plus some cleanup in StandardDirectoryReader ... I think it's ready.

        Show
        Michael McCandless added a comment - Patch w/ fix, plus some cleanup in StandardDirectoryReader ... I think it's ready.
        Hide
        Simon Willnauer added a comment -

        mike, patch looks good. Each time I look at it the more it scares me! We need to clean this class up at some point!

        simon

        Show
        Simon Willnauer added a comment - mike, patch looks good. Each time I look at it the more it scares me! We need to clean this class up at some point! simon
        Hide
        Robert Muir added a comment -

        Can we remove synchronized from noDups?

        release() is already synced, and it scares me to have additional synchronization in a method called only by assert: it could hide bugs.

        Show
        Robert Muir added a comment - Can we remove synchronized from noDups? release() is already synced, and it scares me to have additional synchronization in a method called only by assert: it could hide bugs.
        Hide
        Michael McCandless added a comment -

        We need to clean this class up at some point!

        +1 to that!

        Can we remove synchronized from noDups?

        OK I'll remove it.

        Show
        Michael McCandless added a comment - We need to clean this class up at some point! +1 to that! Can we remove synchronized from noDups? OK I'll remove it.
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] mikemccand
        http://svn.apache.org/viewvc?view=revision&revision=1480650

        LUCENE-4986: NRT reopen after tryDeleteDocument was failing to reflect the deletion

        Show
        Commit Tag Bot added a comment - [trunk commit] mikemccand http://svn.apache.org/viewvc?view=revision&revision=1480650 LUCENE-4986 : NRT reopen after tryDeleteDocument was failing to reflect the deletion
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] mikemccand
        http://svn.apache.org/viewvc?view=revision&revision=1480663

        LUCENE-4986: NRT reopen after tryDeleteDocument was failing to reflect the deletion

        Show
        Commit Tag Bot added a comment - [branch_4x commit] mikemccand http://svn.apache.org/viewvc?view=revision&revision=1480663 LUCENE-4986 : NRT reopen after tryDeleteDocument was failing to reflect the deletion
        Hide
        Steve Rowe added a comment -

        If there are no objections, I'd like to backport this to 4.3.1.

        Show
        Steve Rowe added a comment - If there are no objections, I'd like to backport this to 4.3.1.
        Hide
        Shalin Shekhar Mangar added a comment -

        Back ported to 4.3.1 r1483358.

        I had to change the TestTryDelete test to use NRTManager.TrackingIndexWriter because TrackingIndexWriter only became an independent class since LUCENE-4967

        Show
        Shalin Shekhar Mangar added a comment - Back ported to 4.3.1 r1483358. I had to change the TestTryDelete test to use NRTManager.TrackingIndexWriter because TrackingIndexWriter only became an independent class since LUCENE-4967
        Hide
        Shalin Shekhar Mangar added a comment -

        Bulk closing after 4.3.1 release

        Show
        Shalin Shekhar Mangar added a comment - Bulk closing after 4.3.1 release

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development