Solr
  1. Solr
  2. SOLR-4544

Allow HttpShardHandler to be reused better by subclasses of HttpShardHandlerFactory

    Details

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

      Description

      I would like to have a custom ShardHandlerFactory, which has my own special load balancing logic (among other things). The HttpShardHandler is what currently handles the parallelization of shard requests. I would like to reuse it, but it expects package private variables in HttpShardHandlerFactory which it can reach back into for making load balancer requests and a completion service.

      1. SOLR-4544.patch
        9 kB
        Ryan Ernst
      2. SOLR-4544.patch
        9 kB
        Ryan Ernst

        Activity

        Hide
        Ryan Ernst added a comment -

        This patch makes HttpShardHandler use some new methods on HttpShardHandlerFactory to do things like create a completion service and do a distributed request with a software load balancer. No tests yet. Any thoughts on where how to test this?

        Show
        Ryan Ernst added a comment - This patch makes HttpShardHandler use some new methods on HttpShardHandlerFactory to do things like create a completion service and do a distributed request with a software load balancer. No tests yet. Any thoughts on where how to test this?
        Hide
        Ryan Ernst added a comment -

        Fixed a silly bug. The patch now passes all unit tests.

        Show
        Ryan Ernst added a comment - Fixed a silly bug. The patch now passes all unit tests.
        Hide
        Robert Muir added a comment -

        Looks good. One thing that sticks out at me while reviewing (not anything introduced by your patch), is that this factory
        uses a Random initialized from the default ctor (e.g. current time or whatever) to shuffle the shards.

        It seems like this would make it hard for unit tests to have deterministic behavior. I think we should add a new parameter
        that allows you to set the random seed (if not specified, System.currentTimeMillis() or whatever). Solr test framework could
        use this parameter so that you have the same behavior when you run the test with the same -Dtests.seed.

        Show
        Robert Muir added a comment - Looks good. One thing that sticks out at me while reviewing (not anything introduced by your patch), is that this factory uses a Random initialized from the default ctor (e.g. current time or whatever) to shuffle the shards. It seems like this would make it hard for unit tests to have deterministic behavior. I think we should add a new parameter that allows you to set the random seed (if not specified, System.currentTimeMillis() or whatever). Solr test framework could use this parameter so that you have the same behavior when you run the test with the same -Dtests.seed.
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Robert Muir
        http://svn.apache.org/viewvc?view=revision&revision=1457754

        SOLR-4544: Refactor HttpShardHandlerFactory so load-balancing logic can be customized

        Show
        Commit Tag Bot added a comment - [trunk commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1457754 SOLR-4544 : Refactor HttpShardHandlerFactory so load-balancing logic can be customized
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Robert Muir
        http://svn.apache.org/viewvc?view=revision&revision=1457762

        SOLR-4544: Refactor HttpShardHandlerFactory so load-balancing logic can be customized.

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1457762 SOLR-4544 : Refactor HttpShardHandlerFactory so load-balancing logic can be customized.
        Hide
        Robert Muir added a comment -

        Thanks Ryan!

        Show
        Robert Muir added a comment - Thanks Ryan!
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Unassigned
            Reporter:
            Ryan Ernst
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development