Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elasticsearch pagination does not work on 2.3.3 #25038

Closed
piorus opened this issue Oct 14, 2019 · 29 comments
Closed

Elasticsearch pagination does not work on 2.3.3 #25038

piorus opened this issue Oct 14, 2019 · 29 comments
Labels
Component: Elasticsearch Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Progress: done Severity: S1 Affects critical data or functionality and forces users to employ a workaround.

Comments

@piorus
Copy link

piorus commented Oct 14, 2019

Preconditions (*)

Magento version 2.3.3
Elasticsearch 6.0
Stores->Configuration->Catalog->Catalog->Catalog Search->Search Engine = Elasticsearch 6.0+

Steps to reproduce (*)

NEW steps used for REOPEN on 2.4-develop -> #25038 (comment)

  1. Set Elasticsearch 6.0 as search engine
  2. bin/magento indexer:reindex
  3. Go to category page that have more than one page
  4. Navigate to second page

Expected result (*)

Products from the second page are displayed.

Actual result (*)

No products found.

More details

I've digged up in Magento's ES implementation and found really strange thing. Magento is requesting elasticsearch with from and size parameters that limits search results but additionally there is another limit in \Magento\Eav\Model\Entity\Collection\AbstractCollection::_loadEntities
$this->getSelect()->limitPage($this->getCurPage(), $this->_pageSize);

Result is as follow for second page of category listing:

  1. Elasticsearch returns second page of products limited by from and size and magento is adding product IDs to the collection.
    Example ElasticSearch request:
POST /magento2_product_1/document/_search
{ 
   "from":9,
   "size":9,
   "stored_fields":[ 
      "_id",
      "_score"
   ],
   "sort":[ 
      { 
         "position_category_5":{ 
            "order":"asc"
         }
      }
   ],
   "query":{ 
      "bool":{ 
         "must":[ 
            { 
               "term":{ 
                  "category_ids":"5"
               }
            },
            { 
               "terms":{ 
                  "visibility":[ 
                     "2",
                     "4"
                  ]
               }
            }
         ]
      }
   },
   "aggregations":{ 
      "price_bucket":{ 
         "extended_stats":{ 
            "field":"price_0_1"
         }
      },
      "category_bucket":{ 
         "terms":{ 
            "field":"category_ids",
            "size":500
         }
      },
      "manufacturer_bucket":{ 
         "terms":{ 
            "field":"manufacturer",
            "size":500
         }
      },
      "color_bucket":{ 
         "terms":{ 
            "field":"color",
            "size":500
         }
      }
   }
}
  1. Collection executes _loadEntities and add yet another limit which goes way out of collection
    s range and this leads to 0 results found.
@m2-assistant
Copy link

m2-assistant bot commented Oct 14, 2019

Hi @piotrusin. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@piotrusin do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Oct 14, 2019
@krishprakash krishprakash self-assigned this Oct 14, 2019
@m2-assistant
Copy link

m2-assistant bot commented Oct 14, 2019

Hi @krishprakash. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

    DetailsIf the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

  • 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 4. Verify that the issue is reproducible on 2.3-develop branch

    Details- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 5. Add label Issue: Confirmed once verification is complete.

  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@krishprakash
Copy link

krishprakash commented Oct 14, 2019

Hi @piotrusin. Thank you for your report.
Seems Issue is already reported please review at #24047

@piorus
Copy link
Author

piorus commented Oct 14, 2019

@krishprakash i don't get how those two are related besides that both issues are related to elasticsearch. in this case there is no exception thrown, just empty result set due to duplicated limit/offset

@domeglic
Copy link

There is a patch for this, but it will also cause the pager to stop showing: example Instead of "Items 1-9 of 13" just "13 Items"

@piorus
Copy link
Author

piorus commented Oct 14, 2019

I've handled it by myself by adding before/after _loadEntities plugin on Magento\Catalog\Model\ResourceModel\Product\Collection that sets pageSize to 0 and resets it back after querying if using elasticsearch. This does not break pager.

@simonmaass
Copy link

I have the same issue after upgrading to m2.3.3. - in all category pages or search results i am unable to click on the next page!

@simonmaass
Copy link

btw this is also with elasticsearch 5x... not just 6x like the patch suggests

@sdzhepa
Copy link
Contributor

sdzhepa commented Oct 15, 2019

Hello @piotrusin

Thank you for contribution and collaboration!

The hotfix for this Issue is published and available for download on magento.com portal here https://magento.com/tech-resources/download#download2331

Could you please apply patch and confirm that the issue is fixed?

@simonmaass
Copy link

simonmaass commented Oct 16, 2019

@sdzhepa can confirm that the hotfix works!

Just to add - the error ist not just for eastic 6.x but also 5.x

@domeglic
Copy link

@simonmaass Did the pagination still work after applying the patch? When I apply the patch it breaks it.

Before:
magento pagination counter before
After:
magento pagination counter after

@simonmaass
Copy link

@domeglic you are right! The display of "xx Items" is broken! @sdzhepa

@ArthurSCD
Copy link

ArthurSCD commented Oct 21, 2019

Obviously this has been closed and marked as fixed, but with Elastic Search enabled and Magento 2.3.3, it breaks pagination for me period. Even on non-search pages, (so just category/catalog pagination in general) - I applied the patch, triple checked the edit and regenerated files and all that but the pagination remains broken. If I switch to MYSQL search, pagination goes back to working immediately as expected.

Edit: I have also noticed that category ordering does not work at all with Elastic enabled.

@facetimer
Copy link

facetimer commented Nov 6, 2019

Same as @ArthurSCD. I have applied the patch, the issue still there...

@ArthurSCD have you found a solution?
@piotrusin May you be more explicit by adding before/after _loadEntities?

Please reopen this issue.

@piorus piorus reopened this Nov 6, 2019
@ArthurSCD
Copy link

ArthurSCD commented Nov 6, 2019

Same as @ArthurSCD. I have applied the patch, the issue still there...

@ArthurSCD have you found a solution?
@piotrusin May you be more explicit by adding before/after _loadEntities?

Please reopen this issue.

Unfortunately no. I discovered part of the issue is related to One Column layouts as well. Even in a Vanilla M 2.3.3.

On one live Magento I was forced to change to MYSQL search again for now as with Elastic enabled, the layered navigation added by the theme completely broke. And by broke I mean a mostly white page and some broken text.

Relevant:
#24619

@sedax90
Copy link

sedax90 commented Nov 11, 2019

I can confirm, @piotrusin temporary fix is working fine:

etc/frontend/di.xml

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <type name="Magento\Catalog\Model\ResourceModel\Product\Collection">
        <plugin name="dexanet-catalog-plugin-collection-elastic-bug" type="Dexanet\Catalog\Plugin\Collection"
                sortOrder="1"/>
    </type>
</config>

Plugin\Collection

public function around_loadEntities(\Magento\Catalog\Model\ResourceModel\Product\Collection $subject, callable $proceed, $printQuery = FALSE, $logQuery = FALSE) {
    $pageSize = $subject->getPageSize();
    $subject->setPageSize(0);
    $result = $proceed($printQuery, $logQuery);
    $subject->setPageSize($pageSize);
    return $result;
  }

@densen45
Copy link
Contributor

@piotrusin thank you for sharing your temporary fix. Applying before and after plugins work great. However, when we applied the plugins globally (i.e. placed the di.xml in the etc directory of our module), we discovered that the product tables in the adminhtml were broken (as described here #25555).

For those who want to apply this temporary fix manually, make sure to apply the plugin(s) only in the frontend area (i.e. place the di.xml in the etc/frontend directory of your module) as suggested by @CRYX2 in #25038 (comment)).

@engcom-Charlie
Copy link
Contributor

Hello everyone!
We are closing this issue as @piotrusin temporary fix is working fine:
#25038 (comment)
#25038 (comment).

@engcom-Golf
Copy link
Contributor

engcom-Golf commented Feb 20, 2020

Issue is reproducible if some custom module loads collection before Magento\Catalog\Block\Product\ListProduct block, and sets pageSize() then when Magento\Catalog\Block\Product\ListProduct initialized, they use the previous collection where exists pageSize(), and this is wrong, because on store front we have broken pagination.

  • 1 Create Custom module, wich loads collection before ListProduct
  • 2 In Admin Configure PageLimits and add 200
  • 2 Set pageSize For collection for example 200
  • 3 Go to category page
  • 4 You will see all products on the page

@engcom-Golf engcom-Golf added the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Feb 20, 2020
@ghost ghost removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Feb 20, 2020
@ghost
Copy link

ghost commented Feb 20, 2020

✅ Confirmed by @engcom-Golf
Thank you for verifying the issue! 👍 Your confirmation will help us to acknowledge and process this report.

@engcom-Golf engcom-Golf added the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Feb 26, 2020
@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Feb 26, 2020
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @engcom-Golf
Thank you for verifying the issue. Based on the provided information internal tickets MC-31913 were created

Issue Available: @engcom-Golf, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@ghost ghost unassigned wigman Mar 12, 2020
@urvashikrish urvashikrish self-assigned this Mar 17, 2020
@m2-assistant
Copy link

m2-assistant bot commented Mar 17, 2020

Hi @urvashikrish. Thank you for working on this issue.
Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

  • 1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 2. Verify that the issue is reproducible on 2.4-develop branch

    Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 3. If the issue is not relevant or is not reproducible any more, feel free to close it.


@urvashikrish urvashikrish removed their assignment Mar 17, 2020
@baranada
Copy link

baranada commented Mar 19, 2020

Issue is reproducible if some custom module loads collection before Magento\Catalog\Block\Product\ListProduct block, and sets pageSize() then when Magento\Catalog\Block\Product\ListProduct initialized, they use the previous collection where exists pageSize(), and this is wrong, because on store front we have broken pagination.

* 1  Create Custom module, wich loads collection before ListProduct

* 2 In Admin Configure PageLimits and add 200

* 2  Set pageSize For collection for example 200

* 3 Go to category page

* 4 You will see all products on the page

Maybe I want to ask magento architecture this is a right way to use ListProduct Catalog Block.
@engcom-Golf, Could you share the test code how to initiate to load collection on first step?
Generally, I don't think broken store front pagination does persist on category page.
if it persists on custom module page, maybe it is because the initiation of the collection is not correct.

I have checked that original issue has been fixed on 2.4-develop branch.
I have tested with elasticsearch 6.5.4 version.

@santosh-gaggle
Copy link

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

Hi @santosh-gaggle. Thank you for your request. I'm working on Magento 2.3-develop instance for you

@ihor-sviziev ihor-sviziev added the Severity: S1 Affects critical data or functionality and forces users to employ a workaround. label May 28, 2020
@agustinbalbuena
Copy link

does anybody fix this issue on 2.3?

@sdzhepa sdzhepa added the Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. label Aug 27, 2020
@magento-engcom-team magento-engcom-team added Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch and removed Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch labels Sep 16, 2020
@magento-engcom-team
Copy link
Contributor

Hi @piotrusin.

Thank you for your report and collaboration!

The related internal Jira ticket MC-31913 was closed as non-reproducible in 2.4-develop.
It means that Magento team either unable to reproduce this issue using provided Steps to Reproduce from the Description section on clean Magento instance or the issue has been already fixed in the scope of other tasks.

But if you still run into this problem please update or provide additional information/steps/preconditions in the Description section and reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Elasticsearch Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Progress: done Severity: S1 Affects critical data or functionality and forces users to employ a workaround.
Projects
None yet
Development

No branches or pull requests