Solr
  1. Solr
  2. SOLR-6796

distrib.singlePass does not return correct set of fields for multi-fl-parameter requests

    Details

      Description

      If I pass distrib.singlePass in a request that also has two fl-parameters, in some cases, I will not get the expected set of fields back for the returned documents

      1. fix.patch
        3 kB
        Per Steffensen
      2. fix.patch
        2 kB
        Per Steffensen
      3. fix.patch
        2 kB
        Per Steffensen
      4. fix.patch
        3 kB
        Per Steffensen
      5. SOLR-6796.patch
        5 kB
        Shalin Shekhar Mangar
      6. test_that_reveals_the_problem.patch
        0.9 kB
        Per Steffensen

        Issue Links

          Activity

          Hide
          Per Steffensen added a comment -

          Attached patch (branch_5x) that change an existing test to reveal the problem. Apply the patch and run TestDistributedSearch
          Probably not just want to commit the changed test after a fix of the problem, but maybe we want to add the test somewhere else. In my company we have an agreement that whenever we fix a bug, we also create a test that reveals it

          Show
          Per Steffensen added a comment - Attached patch (branch_5x) that change an existing test to reveal the problem. Apply the patch and run TestDistributedSearch Probably not just want to commit the changed test after a fix of the problem, but maybe we want to add the test somewhere else. In my company we have an agreement that whenever we fix a bug, we also create a test that reveals it
          Hide
          Per Steffensen added a comment -

          Attached an ugly fix to the problem. I believe it will not ruin other functionality - but I havnt run the entire test-suite with this patch

          Show
          Per Steffensen added a comment - Attached an ugly fix to the problem. I believe it will not ruin other functionality - but I havnt run the entire test-suite with this patch
          Hide
          Per Steffensen added a comment -

          Attached a less ugly fix to the problem. Again, havnt run the entire test-suite with this fix.

          Show
          Per Steffensen added a comment - Attached a less ugly fix to the problem. Again, havnt run the entire test-suite with this fix.
          Hide
          Per Steffensen added a comment -

          Thanks for assigning yourself to this one Shalin Shekhar Mangar. Maybe you also want to take a look at SOLR-6795? SOLR-6795 is ready for commit. Lets make this SOLR-6796 about making distrib.singlePass work in every case. Believe the ugly patch will work in every case, but the "less ugly" patch will not. It will not work for e.g. "val:sum(3,4)". I will provide a "less ugly" patch very soon that will work in every case.

          Show
          Per Steffensen added a comment - Thanks for assigning yourself to this one Shalin Shekhar Mangar . Maybe you also want to take a look at SOLR-6795 ? SOLR-6795 is ready for commit. Lets make this SOLR-6796 about making distrib.singlePass work in every case. Believe the ugly patch will work in every case, but the "less ugly" patch will not. It will not work for e.g. "val:sum(3,4)". I will provide a "less ugly" patch very soon that will work in every case.
          Hide
          Per Steffensen added a comment - - edited

          I believe this fix will make distrib.singlePass=true work for any query (together with SOLR-6795). That is, you can turn on distrib.singlePass whenever you like, and have a response similar to a non-distributed request to a single core/shard containing the same data.
          We have a version of Solr based on 4.4.0, with a lot of our own changes, and with SOLR-5768, SOLR-1880 and parts of SOLR-5399 merged. With this fix, then entire 4.4.0 test-suite is green, when we make all queries issued across the test-suite run with distrib.singlePass=true.
          At least it fixes the particular problem that this SOLR-6796 is about.

          Show
          Per Steffensen added a comment - - edited I believe this fix will make distrib.singlePass=true work for any query (together with SOLR-6795 ). That is, you can turn on distrib.singlePass whenever you like, and have a response similar to a non-distributed request to a single core/shard containing the same data. We have a version of Solr based on 4.4.0, with a lot of our own changes, and with SOLR-5768 , SOLR-1880 and parts of SOLR-5399 merged. With this fix, then entire 4.4.0 test-suite is green, when we make all queries issued across the test-suite run with distrib.singlePass=true . At least it fixes the particular problem that this SOLR-6796 is about.
          Hide
          Shalin Shekhar Mangar added a comment -

          Thanks Per. I've assigned the other issue to myself too. I'm travelling today and tomorrow but I will review and commit the fixes by the weekend.

          Show
          Shalin Shekhar Mangar added a comment - Thanks Per. I've assigned the other issue to myself too. I'm travelling today and tomorrow but I will review and commit the fixes by the weekend.
          Hide
          Yonik Seeley added a comment -

          Both the original code and the patches seem more complex than needed....
          why is it necessary to rebuild the fl parameter rather than just append another (it's multi-valued)?

          Show
          Yonik Seeley added a comment - Both the original code and the patches seem more complex than needed.... why is it necessary to rebuild the fl parameter rather than just append another (it's multi-valued)?
          Hide
          Per Steffensen added a comment -

          Both the original code and the patches seem more complex than needed....

          I agree. I didnt want to spend much time making it less complex. Just wanted it to work. When I have brought cleaned up code in the past, I hasnt been appreciated much. Some times it has been rejected with an argument about creating another issue for cleanup.

          Show
          Per Steffensen added a comment - Both the original code and the patches seem more complex than needed.... I agree. I didnt want to spend much time making it less complex. Just wanted it to work. When I have brought cleaned up code in the past, I hasnt been appreciated much. Some times it has been rejected with an argument about creating another issue for cleanup.
          Hide
          Per Steffensen added a comment - - edited

          But ok, here is a fix that cleans up a little bit. Essentially using the FL's of the outer request. But we have to add id-field and score if not already in there
          Besides the patch you should be able to delete "import java.util.regex.Pattern" as well

          Also passes 4.4.0+ test-suite running with distrib.singlePass all over

          Show
          Per Steffensen added a comment - - edited But ok, here is a fix that cleans up a little bit. Essentially using the FL's of the outer request. But we have to add id-field and score if not already in there Besides the patch you should be able to delete "import java.util.regex.Pattern" as well Also passes 4.4.0+ test-suite running with distrib.singlePass all over
          Hide
          Per Steffensen added a comment - - edited

          Maybe I can convince you to take a look at SOLR-6812 also, Shalin Shekhar Mangar?
          And maybe SOLR-6813? I know I am pushing it now

          Show
          Per Steffensen added a comment - - edited Maybe I can convince you to take a look at SOLR-6812 also, Shalin Shekhar Mangar ? And maybe SOLR-6813 ? I know I am pushing it now
          Hide
          Shalin Shekhar Mangar added a comment -

          Patch which combines the test and fix given by Per. I moved the test to DistributedQueryComponentOptimizationTest.

          I'll commit once the test suite passes.

          Show
          Shalin Shekhar Mangar added a comment - Patch which combines the test and fix given by Per. I moved the test to DistributedQueryComponentOptimizationTest. I'll commit once the test suite passes.
          Hide
          Shalin Shekhar Mangar added a comment -

          I'll take a look at SOLR-6812 and SOLR-6813 too but I know very little about ExpandComponent so if things get too hairy, I'll probably defer to someone else.

          Show
          Shalin Shekhar Mangar added a comment - I'll take a look at SOLR-6812 and SOLR-6813 too but I know very little about ExpandComponent so if things get too hairy, I'll probably defer to someone else.
          Hide
          ASF subversion and git services added a comment -

          Commit 1642873 from shalin@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1642873 ]

          SOLR-6796: distrib.singlePass does not return correct set of fields for multi-fl-parameter requests

          Show
          ASF subversion and git services added a comment - Commit 1642873 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1642873 ] SOLR-6796 : distrib.singlePass does not return correct set of fields for multi-fl-parameter requests
          Hide
          ASF subversion and git services added a comment -

          Commit 1642874 from shalin@apache.org in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1642874 ]

          SOLR-6796: distrib.singlePass does not return correct set of fields for multi-fl-parameter requests

          Show
          ASF subversion and git services added a comment - Commit 1642874 from shalin@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1642874 ] SOLR-6796 : distrib.singlePass does not return correct set of fields for multi-fl-parameter requests
          Hide
          ASF subversion and git services added a comment -

          Commit 1642876 from shalin@apache.org in branch 'dev/branches/lucene_solr_4_10'
          [ https://svn.apache.org/r1642876 ]

          SOLR-6796: distrib.singlePass does not return correct set of fields for multi-fl-parameter requests

          Show
          ASF subversion and git services added a comment - Commit 1642876 from shalin@apache.org in branch 'dev/branches/lucene_solr_4_10' [ https://svn.apache.org/r1642876 ] SOLR-6796 : distrib.singlePass does not return correct set of fields for multi-fl-parameter requests
          Hide
          Shalin Shekhar Mangar added a comment -

          Thanks Per!

          Show
          Shalin Shekhar Mangar added a comment - Thanks Per!
          Hide
          Anshum Gupta added a comment -

          Bulk close after 5.0 release.

          Show
          Anshum Gupta added a comment - Bulk close after 5.0 release.

            People

            • Assignee:
              Shalin Shekhar Mangar
              Reporter:
              Per Steffensen
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development