Solr
  1. Solr
  2. SOLR-5864

Remove previous SolrCore as parameter on reload

    Details

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

      Description

      Currently the reload method is reload(SolrResourceLoader resourceLoader, SolrCore prev), but all the times it’s called with “prev” being the same as “this”:
      core.reload(resourceLoader, core).
      Frankly, I don’t think it even makes sense to call it in other way (it would be just to create the first reader with a different core than the one its being reloaded?)
      I think we should just remove the SolrCore parameter and let the "reload" method always reload the core where it's being called.

      1. SOLR-5864.patch
        2 kB
        Tomás Fernández Löbbe
      2. SOLR-5864.patch
        4 kB
        Tomás Fernández Löbbe
      3. SOLR-5864.patch
        2 kB
        Tomás Fernández Löbbe
      4. SOLR-5864.patch
        2 kB
        Tomás Fernández Löbbe

        Activity

        Hide
        Mark Miller added a comment -

        +1.

        I think in 4x, we probably want to keep the old signatures and deprecate them.

        Also, we should probably use a better variable name than prev now:

        -    if (!getNewIndexDir().equals(getIndexDir())) {
        +    SolrCore prev;
        +    if (getNewIndexDir().equals(getIndexDir())) {
        +      prev = this;
        +    } else {
               // the directory is changing, don't pass on state
               prev = null;
             }
        
        Show
        Mark Miller added a comment - +1. I think in 4x, we probably want to keep the old signatures and deprecate them. Also, we should probably use a better variable name than prev now: - if (!getNewIndexDir().equals(getIndexDir())) { + SolrCore prev; + if (getNewIndexDir().equals(getIndexDir())) { + prev = this; + } else { // the directory is changing, don't pass on state prev = null; }
        Hide
        Tomás Fernández Löbbe added a comment -

        Really the SolrCore (prev) is not being used by the SolrCore constructor, just the UpdateHandler is needed, and we are already sending it as a parameter. Maybe instead of renaming the variable it can be removed. I'm attaching a patch

        Show
        Tomás Fernández Löbbe added a comment - Really the SolrCore (prev) is not being used by the SolrCore constructor, just the UpdateHandler is needed, and we are already sending it as a parameter. Maybe instead of renaming the variable it can be removed. I'm attaching a patch
        Hide
        Tomás Fernández Löbbe added a comment -

        Any more thoughts?

        Show
        Tomás Fernández Löbbe added a comment - Any more thoughts?
        Hide
        Tomás Fernández Löbbe added a comment -

        Forgot about this issue. Patch is updated to trunk. Changed the name of the variable inside reload, but kept the SolrCore parameter in the SolrCore constructor.
        I'm planning to commit this to trunk and 5x, deprecate in 4.10.x

        Show
        Tomás Fernández Löbbe added a comment - Forgot about this issue. Patch is updated to trunk. Changed the name of the variable inside reload, but kept the SolrCore parameter in the SolrCore constructor. I'm planning to commit this to trunk and 5x, deprecate in 4.10.x
        Hide
        ASF subversion and git services added a comment -

        Commit 1641819 from Tomás Fernández Löbbe in branch 'dev/trunk'
        [ https://svn.apache.org/r1641819 ]

        SOLR-5864: Remove previous SolrCore as parameter on reload

        Show
        ASF subversion and git services added a comment - Commit 1641819 from Tomás Fernández Löbbe in branch 'dev/trunk' [ https://svn.apache.org/r1641819 ] SOLR-5864 : Remove previous SolrCore as parameter on reload
        Hide
        ASF subversion and git services added a comment -

        Commit 1641826 from Tomás Fernández Löbbe in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1641826 ]

        SOLR-5864: Remove previous SolrCore as parameter on reload

        Show
        ASF subversion and git services added a comment - Commit 1641826 from Tomás Fernández Löbbe in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1641826 ] SOLR-5864 : Remove previous SolrCore as parameter on reload
        Hide
        ASF subversion and git services added a comment -

        Commit 1641844 from Tomás Fernández Löbbe in branch 'dev/branches/lucene_solr_4_10'
        [ https://svn.apache.org/r1641844 ]

        SOLR-5864: Deprecated SolrCore.reload(ConfigSet, SolrCore) and added SolrCore.reload(ConfigSet)

        Show
        ASF subversion and git services added a comment - Commit 1641844 from Tomás Fernández Löbbe in branch 'dev/branches/lucene_solr_4_10' [ https://svn.apache.org/r1641844 ] SOLR-5864 : Deprecated SolrCore.reload(ConfigSet, SolrCore) and added SolrCore.reload(ConfigSet)
        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:
            Tomás Fernández Löbbe
            Reporter:
            Tomás Fernández Löbbe
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development