Solr
  1. Solr
  2. SOLR-5555

CloudSolrServer need not declare to throw MalformedURLException

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.6
    • Fix Version/s: 4.7, Trunk
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      Currently CloudSolrServer declares to throw MalformedURLException for some of its constructors. This does not seem necessary.

      Details based on looking through Solr 4.6 release code:

      CloudSolrServer has the following constructor that declares a checked exception MalformedURLException..

       
       public CloudSolrServer(String zkHost) throws MalformedURLException {
       
            this.zkHost = zkHost;
       
            this.myClient = HttpClientUtil.createClient(null);
       
            this.lbServer = new LBHttpSolrServer(myClient);
       
            this.lbServer.setRequestWriter(new BinaryRequestWriter());
       
            this.lbServer.setParser(new BinaryResponseParser());
       
            this.updatesToLeaders = true;
       
            shutdownLBHttpSolrServer = true;
       
        }
       
      

      The only thing that seemed capable of throwing MalformedURLException seems to be LBHttpSolrServer’s constructor:

        public LBHttpSolrServer(HttpClient httpClient, String... solrServerUrl)
                throws MalformedURLException {
          this(httpClient, new BinaryResponseParser(), solrServerUrl);
        }
      

      which calls ..

        public LBHttpSolrServer(HttpClient httpClient, ResponseParser parser, String... solrServerUrl)
                throws MalformedURLException {
          clientIsInternal = (httpClient == null);
          this.parser = parser;
          if (httpClient == null) {
            ModifiableSolrParams params = new ModifiableSolrParams();
            params.set(HttpClientUtil.PROP_USE_RETRY, false);
            this.httpClient = HttpClientUtil.createClient(params);
          } else {
            this.httpClient = httpClient;
          }
          for (String s : solrServerUrl) {
            ServerWrapper wrapper = new ServerWrapper(makeServer(s)); 
            aliveServers.put(wrapper.getKey(), wrapper);
          }
          updateAliveList();
        }
      

      which calls ..

      protected HttpSolrServer makeServer(String server) throws MalformedURLException {
          HttpSolrServer s = new HttpSolrServer(server, httpClient, parser);
          if (requestWriter != null) {
            s.setRequestWriter(requestWriter);
          }
          if (queryParams != null) {
            s.setQueryParams(queryParams);
          }
          return s;
        }
      

      Note that makeServer(String server) above does not need to throw MalformedURLException.. sine the only thing that seems capable of throwing MalformedURLException is HttpSolrServer’s constructor (which does not):

      public HttpSolrServer(String baseURL, HttpClient client, ResponseParser parser) {
          this.baseUrl = baseURL;
          if (baseUrl.endsWith("/")) {
            baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
          }
          if (baseUrl.indexOf('?') >= 0) {
            throw new RuntimeException(
                "Invalid base url for solrj.  The base URL must not contain parameters: "
                    + baseUrl);
          }
          
          if (client != null) {
            httpClient = client;
            internalClient = false;
          } else {
            internalClient = true;
            ModifiableSolrParams params = new ModifiableSolrParams();
            params.set(HttpClientUtil.PROP_MAX_CONNECTIONS, 128);
            params.set(HttpClientUtil.PROP_MAX_CONNECTIONS_PER_HOST, 32);
            params.set(HttpClientUtil.PROP_FOLLOW_REDIRECTS, followRedirects);
            httpClient =  HttpClientUtil.createClient(params);
          }
          
          this.parser = parser;
        }
      

      I see nothing above that’d throw MalformedURLException. It is throwing a RuntimeException when the baseUrl does not match certain pattern, may be that was intended to be a MalformedURLException.

      It seems like an error or oversight that CloudSolrServer declares to throw MalformedURLException for some of its constructors.

      This could be fixed by making LBHttpSolrServer not declare the MalformedURLException, and thus other callers to it do not need to do so.

      1. SOLR-5555.patch
        3 kB
        Alan Woodward
      2. SOLR-5555.patch
        9 kB
        Alan Woodward

        Activity

        Sushil Bajracharya created issue -
        Alan Woodward made changes -
        Field Original Value New Value
        Assignee Alan Woodward [ romseygeek ]
        Hide
        Alan Woodward added a comment -

        Here's a simple patch.

        Show
        Alan Woodward added a comment - Here's a simple patch.
        Alan Woodward made changes -
        Attachment SOLR-5555.patch [ 12618621 ]
        Hide
        Alan Woodward added a comment -

        Updated patch with a better CHANGES entry and a couple of compilation fixes. Will commit in a bit.

        Show
        Alan Woodward added a comment - Updated patch with a better CHANGES entry and a couple of compilation fixes. Will commit in a bit.
        Alan Woodward made changes -
        Attachment SOLR-5555.patch [ 12618631 ]
        Hide
        ASF subversion and git services added a comment -

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

        SOLR-5555: CloudSolrServer and LBHttpSolrServer shouldn't throw MUE from constructors

        Show
        ASF subversion and git services added a comment - Commit 1550824 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1550824 ] SOLR-5555 : CloudSolrServer and LBHttpSolrServer shouldn't throw MUE from constructors
        Hide
        ASF subversion and git services added a comment -

        Commit 1550826 from Alan Woodward in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1550826 ]

        SOLR-5555: CloudSolrServer and LBHttpSolrServer shouldn't throw MUE from constructors

        Show
        ASF subversion and git services added a comment - Commit 1550826 from Alan Woodward in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1550826 ] SOLR-5555 : CloudSolrServer and LBHttpSolrServer shouldn't throw MUE from constructors
        Hide
        Alan Woodward added a comment -

        Thanks Sushil!

        Show
        Alan Woodward added a comment - Thanks Sushil!
        Alan Woodward made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Mark Miller made changes -
        Fix Version/s 5.0 [ 12321664 ]
        Fix Version/s 4.7 [ 12325573 ]
        David Smiley made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development