Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-4986

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3.1, 6.0
    • 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
        14 kB
        Michael McCandless
      2. LUCENE-4986.patch
        6 kB
        Michael McCandless

        Activity

        Hide
        mikemccand 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
        mikemccand 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
        mikemccand Michael McCandless added a comment -

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

        Show
        mikemccand Michael McCandless added a comment - Patch w/ fix, plus some cleanup in StandardDirectoryReader ... I think it's ready.
        Hide
        simonw 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
        simonw 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
        rcmuir 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
        rcmuir 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
        mikemccand 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
        mikemccand 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 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 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 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 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 Steve Rowe added a comment -

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

        Show
        steve_rowe Steve Rowe added a comment - If there are no objections, I'd like to backport this to 4.3.1.
        Hide
        shalinmangar 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
        shalinmangar 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
        shalinmangar Shalin Shekhar Mangar added a comment -

        Bulk closing after 4.3.1 release

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

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development