Lucene - Core
  1. Lucene - Core
  2. LUCENE-3558

SearcherManager and NRTManager should be in the same package

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      I didnt even know NRTManager was still around, because its in the .index package, whereas SearcherManager is in the .search package.

      Separately, I don't like that this stuff is so 'hard' with core lucene... would it be so bad if this stuff was added to core?

      I suspect a lot of people have issues with this stuff (see http://www.lucidimagination.com/search/document/37964e5f0e5d733b) for example.

      Worst case is just that, combine mistakes with trying to manage this stuff with MMap unmapping and total lack of error detection
      for searching closed readers (LUCENE-3439) and its a mess.

      1. LUCENE-3558.patch
        88 kB
        Simon Willnauer
      2. LUCENE-3558.patch
        71 kB
        Simon Willnauer
      3. LUCENE-3558.patch
        70 kB
        Simon Willnauer
      4. LUCENE-3558.patch
        59 kB
        Simon Willnauer
      5. LUCENE-3558.sh
        0.9 kB
        Simon Willnauer
      6. LUCENE-3558.sh
        0.7 kB
        Simon Willnauer
      7. LUCENE-3558.sh
        0.6 kB
        Simon Willnauer

        Activity

        Hide
        Michael McCandless added a comment -

        I think NRTManager should move to oal.search?

        And I think moving to core is a good idea.

        Show
        Michael McCandless added a comment - I think NRTManager should move to oal.search? And I think moving to core is a good idea.
        Hide
        Simon Willnauer added a comment -

        here is a patch that moves NRTManager & SearcherManager into core. Both are now in o.a.l.search - To apply the patch run the bash file first or the included svn commands

        Show
        Simon Willnauer added a comment - here is a patch that moves NRTManager & SearcherManager into core. Both are now in o.a.l.search - To apply the patch run the bash file first or the included svn commands
        Hide
        Michael McCandless added a comment -

        Thanks Simon! Can we also move SearcherLifetimeManager? And... I think we should move NRTCachingDirectory as well (or we can do this separately...).

        Show
        Michael McCandless added a comment - Thanks Simon! Can we also move SearcherLifetimeManager? And... I think we should move NRTCachingDirectory as well (or we can do this separately...).
        Hide
        Steve Rowe added a comment -

        here is a patch that moves NRTManager & SearcherManager into core. Both are now in o.a.l.search - To apply the patch run the bash file first or the included svn commands

        I'm curious why you used svn copy instead of svn move?

        Show
        Steve Rowe added a comment - here is a patch that moves NRTManager & SearcherManager into core. Both are now in o.a.l.search - To apply the patch run the bash file first or the included svn commands I'm curious why you used svn copy instead of svn move ?
        Hide
        Simon Willnauer added a comment -

        Can we also move SearcherLifetimeManager

        I wasn't sure about this one - SeracherManager and NRTManager went through serious refactorings since they where added. But we can still do that if it is in core... I will mover the NRTCachingDirectory in a different issue.

        I'm curious why you used svn copy instead of svn move?

        I used svn cp since the patch removes the files and otherwise some systems have problems if the files don't exist anymore. I used svn mv to create the patch actually

        Show
        Simon Willnauer added a comment - Can we also move SearcherLifetimeManager I wasn't sure about this one - SeracherManager and NRTManager went through serious refactorings since they where added. But we can still do that if it is in core... I will mover the NRTCachingDirectory in a different issue. I'm curious why you used svn copy instead of svn move? I used svn cp since the patch removes the files and otherwise some systems have problems if the files don't exist anymore. I used svn mv to create the patch actually
        Hide
        Michael McCandless added a comment -

        OK we can leave SearcherLifetimeMGR in contrib for now...

        Show
        Michael McCandless added a comment - OK we can leave SearcherLifetimeMGR in contrib for now...
        Hide
        Simon Willnauer added a comment -

        new patch moving SearcherLifetimeManager into core and adds additional @lucene.experimental to NRTManagerReopenThread & SearcherLifetimeManager

        I think this is ready.

        Show
        Simon Willnauer added a comment - new patch moving SearcherLifetimeManager into core and adds additional @lucene.experimental to NRTManagerReopenThread & SearcherLifetimeManager I think this is ready.
        Hide
        Simon Willnauer added a comment -

        new patch containing CHANGES entries

        Show
        Simon Willnauer added a comment - new patch containing CHANGES entries
        Hide
        Simon Willnauer added a comment -

        next iteration moving NRTCachingDirectory to core too. TestNRTManager depends on it and I think its worth it anyway so lets do it here to while we on it. I will open a follow up to enable this in tests etc.

        Show
        Simon Willnauer added a comment - next iteration moving NRTCachingDirectory to core too. TestNRTManager depends on it and I think its worth it anyway so lets do it here to while we on it. I will open a follow up to enable this in tests etc.
        Hide
        Mark Miller added a comment -

        moving NRTCachingDirectory to core too

        +1 - if it's the normal recommendation for NRT it should be part of core.

        Show
        Mark Miller added a comment - moving NRTCachingDirectory to core too +1 - if it's the normal recommendation for NRT it should be part of core.
        Hide
        Simon Willnauer added a comment -

        committed to trunk and backported to 3.x

        Show
        Simon Willnauer added a comment - committed to trunk and backported to 3.x
        Hide
        Uwe Schindler added a comment -

        Bulk close after release of 3.5

        Show
        Uwe Schindler added a comment - Bulk close after release of 3.5

          People

          • Assignee:
            Simon Willnauer
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development