Solr
  1. Solr
  2. SOLR-3079

Backport of Solr-1431 (CommComponent abstracted)

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.5
    • Fix Version/s: 3.6
    • Component/s: search
    • Labels:
      None

      Description

      Initial attempt at backporting the work done for Solr-1431 into the 3.x series

      1. 0001-Initial-backport-of-solr-cloud-ShardHandler.patch
        29 kB
        Greg Bowyer
      2. SOLR-3079.patch
        28 kB
        Erick Erickson
      3. SOLR-3079.patch
        28 kB
        Greg Bowyer
      4. SOLR-3079.patch
        28 kB
        Greg Bowyer

        Activity

        Hide
        Greg Bowyer added a comment -

        Hi all, due to a need of mine to be able to configure the maximum number of connections per host I have backported the changes that were made as part of SOLR-1431 into the 3.x series (basing my work on the 3.5 branch)

        The backported version naturally does not leverage ZooKeeper, and also has the load balancing http client removed, however it appears to pass testing.

        Patch attached

        Show
        Greg Bowyer added a comment - Hi all, due to a need of mine to be able to configure the maximum number of connections per host I have backported the changes that were made as part of SOLR-1431 into the 3.x series (basing my work on the 3.5 branch) The backported version naturally does not leverage ZooKeeper, and also has the load balancing http client removed, however it appears to pass testing. Patch attached
        Hide
        Greg Bowyer added a comment -

        Initial backport

        Show
        Greg Bowyer added a comment - Initial backport
        Hide
        Noble Paul added a comment -

        Looks good. Do you expect another pr-4.x release?

        Show
        Noble Paul added a comment - Looks good. Do you expect another pr-4.x release?
        Hide
        Greg Bowyer added a comment -

        I dont know if there will be another pre 4 release

        Show
        Greg Bowyer added a comment - I dont know if there will be another pre 4 release
        Hide
        Greg Bowyer added a comment -

        looks like there is talk of a 3.6 release, how would we go about making this part of that version

        Show
        Greg Bowyer added a comment - looks like there is talk of a 3.6 release, how would we go about making this part of that version
        Hide
        Erick Erickson added a comment -

        The patch isn't in SVN format, looks like you made it with Git? The git repo is a "shadow" repository, not used for released code as far as I know.

        Through the magic of IntelliJ, I managed to apply the patch and I'm uploading that version. Can you take a look and see if it made it through the transformations OK?

        And any Git people out there; is there magic to make Git produce a SVN-compatibile patch? Seems like a good addition to the "How to contribute" page, lots of people seem to be using Git...

        Beyond that, I'll run the tests with it and report back if there's a problem. I'd really like someone who knows what this is all about to take a look before committing....

        Meanwhile, keep prompting <G>....

        Show
        Erick Erickson added a comment - The patch isn't in SVN format, looks like you made it with Git? The git repo is a "shadow" repository, not used for released code as far as I know. Through the magic of IntelliJ, I managed to apply the patch and I'm uploading that version. Can you take a look and see if it made it through the transformations OK? And any Git people out there; is there magic to make Git produce a SVN-compatibile patch? Seems like a good addition to the "How to contribute" page, lots of people seem to be using Git... Beyond that, I'll run the tests with it and report back if there's a problem. I'd really like someone who knows what this is all about to take a look before committing.... Meanwhile, keep prompting <G>....
        Hide
        Robert Muir added a comment -

        And any Git people out there; is there magic to make Git produce a SVN-compatibile patch? Seems like a good addition to the "How to contribute" page, lots of people seem to be using Git...

        I just use patch -p1 when I want to apply git patches... (eclipse has a checkbox or some other gui-toggle for -p if you prefer guis)

        Show
        Robert Muir added a comment - And any Git people out there; is there magic to make Git produce a SVN-compatibile patch? Seems like a good addition to the "How to contribute" page, lots of people seem to be using Git... I just use patch -p1 when I want to apply git patches... (eclipse has a checkbox or some other gui-toggle for -p if you prefer guis)
        Hide
        Greg Bowyer added a comment -

        My mistake, I tend to work from the git mirrors as it makes it easier than trying to keep a bunch of subversion working directories kicking about.

        Show
        Greg Bowyer added a comment - My mistake, I tend to work from the git mirrors as it makes it easier than trying to keep a bunch of subversion working directories kicking about.
        Hide
        Erick Erickson added a comment -

        Greg:
        No problem, it looks like Git patches can be applied pretty easily, I just didn't know the magic.

        All:
        Is there any reason NOT to apply this patch? I'd like to resolve this one sometime soon. All test pass, but I'm not familiar enough with the this area of the code to review it competently....

        Show
        Erick Erickson added a comment - Greg: No problem, it looks like Git patches can be applied pretty easily, I just didn't know the magic. All: Is there any reason NOT to apply this patch? I'd like to resolve this one sometime soon. All test pass, but I'm not familiar enough with the this area of the code to review it competently....
        Hide
        Greg Bowyer added a comment -

        I have noticed an issue whereby I exposed the thread pool sizing as well which I feel needs more testing on my side and is not something done in the lucene 4.0 version on
        SOLR-1431. I will cut a new patch with that removed, in order to not cause confusion

        Show
        Greg Bowyer added a comment - I have noticed an issue whereby I exposed the thread pool sizing as well which I feel needs more testing on my side and is not something done in the lucene 4.0 version on SOLR-1431 . I will cut a new patch with that removed, in order to not cause confusion
        Hide
        Greg Bowyer added a comment -

        Sorry, I meant I exposed the max connections per host, I think that should be exposed, but its better if its done in later patches so that it can be applied to both lucene 4.0 and lucene 3.x.

        Does anyone have any take on this, before I remove that piece of code ?

        Show
        Greg Bowyer added a comment - Sorry, I meant I exposed the max connections per host, I think that should be exposed, but its better if its done in later patches so that it can be applied to both lucene 4.0 and lucene 3.x. Does anyone have any take on this, before I remove that piece of code ?
        Hide
        Greg Bowyer added a comment -

        Updated version removing the parameter maxConnectionsPerHost

        Show
        Greg Bowyer added a comment - Updated version removing the parameter maxConnectionsPerHost
        Hide
        Greg Bowyer added a comment -

        Altered patch that makes SearchHandler PluginInfoInitialized so that it can actually do the right thing when it is configured in solrconfig.xml

        Show
        Greg Bowyer added a comment - Altered patch that makes SearchHandler PluginInfoInitialized so that it can actually do the right thing when it is configured in solrconfig.xml
        Hide
        Erick Erickson added a comment -

        If nobody objects, I'll commit this over the next couple of days. All tests pass. I'll look this over, but I'm not all that familiar with the code, so any additional reviews would be very welcome.

        Show
        Erick Erickson added a comment - If nobody objects, I'll commit this over the next couple of days. All tests pass. I'll look this over, but I'm not all that familiar with the code, so any additional reviews would be very welcome.
        Hide
        Erick Erickson added a comment -

        r: 1298032

        Show
        Erick Erickson added a comment - r: 1298032
        Hide
        Erick Erickson added a comment -

        P.S. Thanks Greg!

        Show
        Erick Erickson added a comment - P.S. Thanks Greg!

          People

          • Assignee:
            Erick Erickson
            Reporter:
            Greg Bowyer
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development