Solr
  1. Solr
  2. SOLR-6897

Nuke non-NRT mode from code and configuration

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      We've had nrtMode as a configurable param in solrconfig.xml. This was maybe necessary in the early days when were testing the waters for NRT searchers but it is not necessary anymore. We should just nuke it and have only NRT mode always.

      1. SOLR-6897.patch
        32 kB
        Shalin Shekhar Mangar
      2. SOLR-6897.patch
        36 kB
        Shalin Shekhar Mangar
      3. SOLR-6897.patch
        31 kB
        Shalin Shekhar Mangar

        Activity

        Hide
        Yonik Seeley added a comment -

        IIRC, it was really just for testing.
        +1

        Show
        Yonik Seeley added a comment - IIRC, it was really just for testing. +1
        Hide
        Shalin Shekhar Mangar added a comment -

        Patch to remove nrtMode. This also removes TestNonNRTOpen, TestArbitraryIndexDir and TestReplicationHandler.testNoWriter.

        TestNonNRTOpen is not needed anymore. We may need another test to replace TestArbitraryIndexDir. I am not yet sure about TestReplicationHandler.testNoWriter.

        Show
        Shalin Shekhar Mangar added a comment - Patch to remove nrtMode. This also removes TestNonNRTOpen, TestArbitraryIndexDir and TestReplicationHandler.testNoWriter. TestNonNRTOpen is not needed anymore. We may need another test to replace TestArbitraryIndexDir. I am not yet sure about TestReplicationHandler.testNoWriter.
        Hide
        Hoss Man added a comment -
        ===================================================================
        --- solr/core/src/java/org/apache/solr/core/SolrConfig.java     (revision 1648550)
        +++ solr/core/src/java/org/apache/solr/core/SolrConfig.java     (revision )
        @@ -192,7 +192,6 @@
               defaultIndexConfig = mainIndexConfig = null;
               indexConfigPrefix = "indexConfig";
             }
        -    nrtMode = getBool(indexConfigPrefix+"/nrtMode", true);
             // Parse indexConfig section, using mainIndex as backup in case old config is used
             indexConfig = new SolrIndexConfig(this, "indexConfig", mainIndexConfig);
         
        

        this change should follow the same pattern we use anytime we stop supporting a config option:

        • check config to see if it's specified, and if so log a warning
        • commit & backport
        • update trunk to change warning log to fatal error & note it in X.0 (6.0 in this case) "upgrading" instructions.

        (this is why methods like SolrIndexConfig.assertWarnOrFail exist .. perhaps that method should be promoted to protected in Config.java?)

        Show
        Hoss Man added a comment - =================================================================== --- solr/core/src/java/org/apache/solr/core/SolrConfig.java (revision 1648550) +++ solr/core/src/java/org/apache/solr/core/SolrConfig.java (revision ) @@ -192,7 +192,6 @@ defaultIndexConfig = mainIndexConfig = null ; indexConfigPrefix = "indexConfig" ; } - nrtMode = getBool(indexConfigPrefix+ "/nrtMode" , true ); // Parse indexConfig section, using mainIndex as backup in case old config is used indexConfig = new SolrIndexConfig( this , "indexConfig" , mainIndexConfig); this change should follow the same pattern we use anytime we stop supporting a config option: check config to see if it's specified, and if so log a warning commit & backport update trunk to change warning log to fatal error & note it in X.0 (6.0 in this case) "upgrading" instructions. (this is why methods like SolrIndexConfig.assertWarnOrFail exist .. perhaps that method should be promoted to protected in Config.java?)
        Hide
        Shalin Shekhar Mangar added a comment -

        Thanks Hoss.

        I moved the assertWarnOrFail method to Config and I used it for this change. This patch is for branch_5x. On trunk I will change the assert to throw an exception.

        Show
        Shalin Shekhar Mangar added a comment - Thanks Hoss. I moved the assertWarnOrFail method to Config and I used it for this change. This patch is for branch_5x. On trunk I will change the assert to throw an exception.
        Hide
        Shalin Shekhar Mangar added a comment -

        This patch restores TestArbitraryIndexDir and modified it to use core reload instead of commit which will make it work fine without the need to turn off NRT mode.

        The TestReplicationHandler.testNoWriter is a weird test. An IW is always opened and the assumption in the test is wrong. I think we can safely remove this test.

        Show
        Shalin Shekhar Mangar added a comment - This patch restores TestArbitraryIndexDir and modified it to use core reload instead of commit which will make it work fine without the need to turn off NRT mode. The TestReplicationHandler.testNoWriter is a weird test. An IW is always opened and the assumption in the test is wrong. I think we can safely remove this test.
        Hide
        ASF subversion and git services added a comment -

        Commit 1649810 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1649810 ]

        SOLR-6897: Nuke non-NRT mode from code and configuration

        Show
        ASF subversion and git services added a comment - Commit 1649810 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1649810 ] SOLR-6897 : Nuke non-NRT mode from code and configuration
        Hide
        ASF subversion and git services added a comment -

        Commit 1649812 from shalin@apache.org in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1649812 ]

        SOLR-6897: Nuke non-NRT mode from code and configuration

        Show
        ASF subversion and git services added a comment - Commit 1649812 from shalin@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1649812 ] SOLR-6897 : Nuke non-NRT mode from code and configuration
        Hide
        ASF subversion and git services added a comment -

        Commit 1649814 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1649814 ]

        SOLR-6897: Fix wrong assert

        Show
        ASF subversion and git services added a comment - Commit 1649814 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1649814 ] SOLR-6897 : Fix wrong assert
        Hide
        ASF subversion and git services added a comment -

        Commit 1649815 from shalin@apache.org in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1649815 ]

        SOLR-6897: Fix wrong assert

        Show
        ASF subversion and git services added a comment - Commit 1649815 from shalin@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1649815 ] SOLR-6897 : Fix wrong assert
        Hide
        ASF subversion and git services added a comment -

        Commit 1649816 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1649816 ]

        SOLR-6897: Throw fatal error if nrtMode is present in solrconfig on 6.0. Added note to upgrade section.

        Show
        ASF subversion and git services added a comment - Commit 1649816 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1649816 ] SOLR-6897 : Throw fatal error if nrtMode is present in solrconfig on 6.0. Added note to upgrade section.
        Hide
        Shalin Shekhar Mangar added a comment -

        This is fixed.

        I added a test on trunk in TestBadConfig.java which asserts that an error is thrown if nrtMode is specified in solrconfig.xml. Added upgrade notes to 5.0 and 6.0 sections.

        Show
        Shalin Shekhar Mangar added a comment - This is fixed. I added a test on trunk in TestBadConfig.java which asserts that an error is thrown if nrtMode is specified in solrconfig.xml. Added upgrade notes to 5.0 and 6.0 sections.
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development