Solr
  1. Solr
  2. SOLR-7614

Distributed Pivot Facet not thread safe

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.2
    • Component/s: None
    • Labels:
      None

      Description

      I just happened to notice the following added to FacetComponent:

        /**
         * Incremented counter used to track the values being refined in a given request.
         * This counter is used in conjunction with {@link PivotFacet#REFINE_PARAM} to identify
         * which refinement values are associated with which pivots.
         */
        int pivotRefinementCounter = 0;
      

      That counter is incremented for each refinement request for correlation, and then reset to 0 at the end. This will obviously break for concurrent distributed pivot facet requests.

      pivotRefinementCounter should be moved to per-request storage.

      1. SOLR-7614.patch
        3 kB
        Yonik Seeley

        Activity

        Hide
        Yonik Seeley added a comment - - edited

        This may be the root cause of SOLR-7605

        edit: nope... something else is wrong as well since the patch didn't fix those fails.

        Show
        Yonik Seeley added a comment - - edited This may be the root cause of SOLR-7605 edit: nope... something else is wrong as well since the patch didn't fix those fails.
        Hide
        Yonik Seeley added a comment -

        Here's a patch that moves the counter to FacetInfo.
        Since a new FacetInfo instance is created for each request, there is no longer a need to reset the counter to 0 in finish().

        The concurrency issue was even worse - the counter was set to 0 in finish() even when faceting wasn't enabled.... so any concurrent query requests could mess up pivot facets.

        Show
        Yonik Seeley added a comment - Here's a patch that moves the counter to FacetInfo. Since a new FacetInfo instance is created for each request, there is no longer a need to reset the counter to 0 in finish(). The concurrency issue was even worse - the counter was set to 0 in finish() even when faceting wasn't enabled.... so any concurrent query requests could mess up pivot facets.
        Hide
        ASF subversion and git services added a comment -

        Commit 1683203 from Yonik Seeley in branch 'dev/trunk'
        [ https://svn.apache.org/r1683203 ]

        SOLR-7614: make pivotRefinementCounter specific to each top level request

        Show
        ASF subversion and git services added a comment - Commit 1683203 from Yonik Seeley in branch 'dev/trunk' [ https://svn.apache.org/r1683203 ] SOLR-7614 : make pivotRefinementCounter specific to each top level request
        Hide
        ASF subversion and git services added a comment -

        Commit 1683204 from Yonik Seeley in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1683204 ]

        SOLR-7614: make pivotRefinementCounter specific to each top level request

        Show
        ASF subversion and git services added a comment - Commit 1683204 from Yonik Seeley in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1683204 ] SOLR-7614 : make pivotRefinementCounter specific to each top level request
        Hide
        ASF subversion and git services added a comment -

        Commit 1683205 from Yonik Seeley in branch 'dev/branches/lucene_solr_5_2'
        [ https://svn.apache.org/r1683205 ]

        SOLR-7614: make pivotRefinementCounter specific to each top level request

        Show
        ASF subversion and git services added a comment - Commit 1683205 from Yonik Seeley in branch 'dev/branches/lucene_solr_5_2' [ https://svn.apache.org/r1683205 ] SOLR-7614 : make pivotRefinementCounter specific to each top level request
        Hide
        Hoss Man added a comment -

        gah! ... good catch.

        +1

        Show
        Hoss Man added a comment - gah! ... good catch. +1
        Hide
        Anshum Gupta added a comment -

        Bulk close for 5.2.0.

        Show
        Anshum Gupta added a comment - Bulk close for 5.2.0.

          People

          • Assignee:
            Yonik Seeley
            Reporter:
            Yonik Seeley
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development