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

IBX-3863: Parametrized Solr HTTP Client timeout and max retries #241

Merged
merged 5 commits into from Oct 5, 2022

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Sep 19, 2022

Question Answer
JIRA issue IBX-3863
Type improvement
Target Ibexa version v3.3+
BC breaks no

When switching to Symfony HTTP Client to handle Solr queries in #235. I didn't introduce any possibility to configure Client's timeout nor number of max retries. There are some edge cases which show that this parameter might work-around performance issues.

Documentation

Solr Ibexa Solr Bundle uses Symfony HTTP Client to fetch and update Solr index. It's possible to configure timeout and number of max retries for that client using Solr Bundle's Semantic Configuration:

v3.3:

ez_search_engine_solr:
    http_client:
        timeout: 30
        max_retries: 5

v4.x:

ibexa_solr:
    http_client:
        timeout: 30
        max_retries: 5

QA

You can test timeout change by mocking Solr service

$ nc -vv -l 8983

and then trying to perform e.g. reindexing

php ./bin/console ibexa:reindex --processes 1 -n

See that modifying both timeout and max_retries affect the wait time and the amount of tries before failure.

Open questions

Given this rather should be avoided, preferring proper performance fixes, should we:

  • Document those parameters?
  • Provide Semantic configuration instead of parameters (only if we decide to document)

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly.
  • Asked for a review

@alongosz alongosz requested a review from a team September 19, 2022 13:48
Copy link
Contributor

@Steveb-p Steveb-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semantic bundle configuration is almost always preferred and recommended by Symfony.

@alongosz alongosz force-pushed the ibx-3863-solr-http-client-timeout branch from 5c77089 to dda3dda Compare September 20, 2022 15:46
@alongosz alongosz added the Doc needed The changes require some documentation label Sep 20, 2022
Copy link
Contributor

@Steveb-p Steveb-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you can additionally do is to pass additional options to ibexa.solr.http_client factory.

What I mean specifically, we can pass extra.curl.CURLOPT_CONNECTTIMEOUT (extra contains client-specific configurations) so that if we are having trouble establishing a connection, then we're dropping the attempt earlier (like after 2 seconds maybe?).

This will help us differentiate between service being overloaded / responding longer vs. service being completely unavailable.

See https://symfony.com/doc/current/http_client.html#configuring-curlhttpclient-options

@alongosz
Copy link
Member Author

What you can additionally do is to pass additional options to ibexa.solr.http_client factory.

What I mean specifically, we can pass extra.curl.CURLOPT_CONNECTTIMEOUT (extra contains client-specific configurations) so that if we are having trouble establishing a connection, then we're dropping the attempt earlier (like after 2 seconds maybe?).

This will help us differentiate between service being overloaded / responding longer vs. service being completely unavailable.

See https://symfony.com/doc/current/http_client.html#configuring-curlhttpclient-options

@Steveb-p I got the error when trying to set that option

Cannot set "CURLOPT_CONNECTTIMEOUT" with "extra.curl"

Moreover I think it will be the opposite of what we're trying to achieve. We need to extend connecting time in case of slow solr instance, but we want to avoid retries when instance is not running at all.

@sonarcloud
Copy link

sonarcloud bot commented Sep 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA approved on IbexaDXP 3.3 commerce.

@alongosz alongosz merged commit 5c6878b into 3.3 Oct 5, 2022
@alongosz alongosz deleted the ibx-3863-solr-http-client-timeout branch October 5, 2022 09:29
@DominikaK DominikaK removed the Doc needed The changes require some documentation label Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants