Solr
  1. Solr
  2. SOLR-7201

Implement multicore handling on HttpSolrClient

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Now that SOLR-7155 has added a collection parameter to the various SolrClient methods, we can let HttpSolrClient use it to allow easier multicore handling.

      1. SOLR-7201.patch
        4 kB
        Alan Woodward

        Activity

        Hide
        Alan Woodward added a comment -

        Patch.

        Show
        Alan Woodward added a comment - Patch.
        Hide
        Shawn Heisey added a comment -

        With this change, would the URL to create HttpSolrClient objects change so that you would pass in the container app context instead of a core base URL? If so, what happens if the object is created with a core URL? Do we need setDefaultCollection in HttpSolrClient?

        Show
        Shawn Heisey added a comment - With this change, would the URL to create HttpSolrClient objects change so that you would pass in the container app context instead of a core base URL? If so, what happens if the object is created with a core URL? Do we need setDefaultCollection in HttpSolrClient?
        Hide
        Alan Woodward added a comment -

        You can still create an HttpSolrClient pointing at a core, but core-specific queries won't work in that case. I don't think we want to add setDefaultCollection here though. The problem is, there's no way to know from just the passed-in URL string if we're pointing at the container app or at a specific core.

        I'll add some JavaDoc to the class explaining the different use-cases:

        • create an HttpSolrClient pointing to a specific core (can't do core admin requests or requests to another core)
        • create an HttpSolrClient pointing to the container app (can do core admin requests, all core-specific requests should use the core-specific request methods)
        Show
        Alan Woodward added a comment - You can still create an HttpSolrClient pointing at a core, but core-specific queries won't work in that case. I don't think we want to add setDefaultCollection here though. The problem is, there's no way to know from just the passed-in URL string if we're pointing at the container app or at a specific core. I'll add some JavaDoc to the class explaining the different use-cases: create an HttpSolrClient pointing to a specific core (can't do core admin requests or requests to another core) create an HttpSolrClient pointing to the container app (can do core admin requests, all core-specific requests should use the core-specific request methods)
        Hide
        Shawn Heisey added a comment -

        Thanks for the update.

        I've been wondering whether the "old" methods should be deprecated. It's clear that they can still be useful in some circumstances, but I think perhaps we should encourage people to use the new methods with "null" for the collection in those circumstances ... and explain everything as clearly as we can in the javadoc.

        Show
        Shawn Heisey added a comment - Thanks for the update. I've been wondering whether the "old" methods should be deprecated. It's clear that they can still be useful in some circumstances, but I think perhaps we should encourage people to use the new methods with "null" for the collection in those circumstances ... and explain everything as clearly as we can in the javadoc.
        Hide
        Alan Woodward added a comment -

        I've been wondering whether the "old" methods should be deprecated

        I wouldn't have thought so, they still work fine for lots of situations. And forcing people to add a null parameter to their code all over the place is just ugly.

        Show
        Alan Woodward added a comment - I've been wondering whether the "old" methods should be deprecated I wouldn't have thought so, they still work fine for lots of situations. And forcing people to add a null parameter to their code all over the place is just ugly.
        Hide
        Shawn Heisey added a comment -

        And forcing people to add a null parameter to their code all over the place is just ugly.

        I agree with this, but I think the suggested forward migration path should be a situation where only the new methods are used, and every call will include a non-null string for collection. Leaving out setDefaultCollection would encourage this as well, so that sounds like a good plan. I think perhaps we should deprecate setDefaultCollection in CloudSolrClient as well, since it is not threadsafe and new functionality removes the need for it.

        Deprecation gives the developer a instant clue that they are not using the class in the way that the authors intended. We can describe the design intent in the javadoc ... but deprecation gives an IDE user an immediate indicator that they should read that javadoc and change their code.

        I'm excited about this new functionality. I'll be able to change code that currently creates sixty HttpSolrServer objects (56 for talking to individual cores, four for CoreAdminRequest functionality to the servers) so it only creates four HttpSolrClient objects.

        Show
        Shawn Heisey added a comment - And forcing people to add a null parameter to their code all over the place is just ugly. I agree with this, but I think the suggested forward migration path should be a situation where only the new methods are used, and every call will include a non-null string for collection. Leaving out setDefaultCollection would encourage this as well, so that sounds like a good plan. I think perhaps we should deprecate setDefaultCollection in CloudSolrClient as well, since it is not threadsafe and new functionality removes the need for it. Deprecation gives the developer a instant clue that they are not using the class in the way that the authors intended. We can describe the design intent in the javadoc ... but deprecation gives an IDE user an immediate indicator that they should read that javadoc and change their code. I'm excited about this new functionality. I'll be able to change code that currently creates sixty HttpSolrServer objects (56 for talking to individual cores, four for CoreAdminRequest functionality to the servers) so it only creates four HttpSolrClient objects.
        Hide
        Shawn Heisey added a comment -

        Further thoughts: If we pursue deprecation, then the trunk code should probably throw an IllegalArgumentException when collection is null.

        Show
        Shawn Heisey added a comment - Further thoughts: If we pursue deprecation, then the trunk code should probably throw an IllegalArgumentException when collection is null.
        Hide
        ASF subversion and git services added a comment -

        Commit 1665199 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1665199 ]

        SOLR-7201: HttpSolrClient can handle collection parameters

        Show
        ASF subversion and git services added a comment - Commit 1665199 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1665199 ] SOLR-7201 : HttpSolrClient can handle collection parameters
        Hide
        Alan Woodward added a comment -

        We can open another issue to talk about deprecations if needed.

        Show
        Alan Woodward added a comment - We can open another issue to talk about deprecations if needed.
        Hide
        ASF subversion and git services added a comment -

        Commit 1665203 from Alan Woodward in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1665203 ]

        SOLR-7201: HttpSolrClient can handle collection parameters

        Show
        ASF subversion and git services added a comment - Commit 1665203 from Alan Woodward in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1665203 ] SOLR-7201 : HttpSolrClient can handle collection parameters

          People

          • Assignee:
            Alan Woodward
            Reporter:
            Alan Woodward
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development