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

Use the timeout when querying Elasticsearch #15592

Merged

Conversation

mpchadwick
Copy link
Contributor

Description

Sets a timeout when querying Elasticsearch. Without a timeout the following can happen...

  • For whatever reason Elasticsearch becomes REALLY slow to respond to a bunch of queries (on one site I've seen it taking like 900 seconds in New Relic)
  • Lots of PHP processes wait A LONG TIME for these responses to come back from Elasticsearch.
  • Build up of PHP processes cause entire site to go down due to high load average, php-fpm max children, etc, etc

If the timeout is exceeded the user will get a 503, but at least it won't bring the site down. Anyway, if it's waiting more than the timeout (default 15 seconds) it's a pretty awful user experience already at that point...

Fixed Issues (if relevant)

N/A

Manual testing scenarios

Here's how I tested...

  1. Create a dummy php server that sleeps for 60 seconds and returns nothing[1]
  2. Run that on port 9201 [2]
  3. Update the Magento Elasticseach settings to point to php instance running on port 9201
  4. Visit any category page

[1]

Assuming Magento is querying index magento2-3-develop_product_1

$ tree
.
└── magento2-3-develop_product_1
    └── document
        └── _search
            └── index.php

3 directories, 1 file
$ cat magento2-3-develop_product_1/document/_search/index.php
<?php

sleep(60);

[2]

$ php -S localhost:9201

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento magento deleted a comment from magento-engcom-team Jun 15, 2018
@mpchadwick
Copy link
Contributor Author

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @mpchadwick. Thank you for your request.\nUnfortunately, I can only deploy instances for 2.2-develop and 2.3-develop base branches

@mpchadwick
Copy link
Contributor Author

@magento-engcom-team give me new test instance

@magento-engcom-team
Copy link
Contributor

Hi @mpchadwick. Thank you for your request.\nUnfortunately, I can only deploy instances for 2.2-develop and 2.3-develop base branches

@magento-engcom-team
Copy link
Contributor

Hi @mpchadwick. Thank you for your contribution.
We will aim to release these changes as part of 2.3.0.
Please check the release notes for final confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants