Issue-2011 - Set configuration for readtimeout and set timeout on ES …

[alin1918]

…client #2011

[romainruaud]

Sounds great !

[rbayet]

Change param name to 'read_timeout' here if we change it in the system.xml ?

[rbayet]

Hello @alin1918,

Thanks for the PR !
What do you think of the proposed change to use 'read_timeout' as far as possible from the name of the system.xml variable up to the point it is passed as a connection param ?
Also, if we have a new system.xml variable, please add a default value in config.xml.

Feel free also to squash your commits and use the prefix "Fixes #2011 " in your commit message.

Regards,

[alin1918]

Hello @alin1918,

Thanks for the PR !
What do you think of the proposed change to use 'read_timeout' as far as possible from the name of the system.xml variable up to the point it is passed as a connection param ?
Also, if we have a new system.xml variable, please add a default value in config.xml.

Feel free also to squash your commits and use the prefix "Fixes #2011 " in your commit message.

Regards,

Hi, the reason why I let it as "timeout" instead of "read_timeout", is because is already defined in etc/config.xml as "timeout" (but not used), the fact that the "timeout" name is being use in the library that creates the curl object, and also that the curl option is called being defined is CURLOPT_TIMEOUT. In etc/adminhtml/system.xml the label states that is "Read Timeout".
I have no problem changing to read_timeout, just wanted to point out my reasoning in case you missed any of those.

Just let me know and I can change it. Thanks.

[rbayet]

Hello @alin1918,

Thanks for your explanations. Indeed you used the correct naming conventions.
Could you simply squash your two commits with a commit message along the way of "Fixes #2011 ES client support for connection and read timeout" ?

Regards,
Richard

[alinalexandru]

@rbayet
There is one thing that might be done here.
To spit that timeout into 2 fields (frontend and admin). During reindex you might need to have a higher value for read timeout. (because it takes longer to process), and when it's used in frontend to mark the server as down much faster.

For example for frontend use: 2-3 seconds should be ok.
For admin usage (reindex): 30-60 seconds.

We are using this patch in production, already

[rbayet]

@alin1918,

I was thinking about that test case of reindexing on a huge catalog and also for some particular operations.
If that cannot be nicely done, maybe we'll go with the connection_timeout (can't seem to find where the support for it disappeared) for now and rethink about the read_timeout

Regards,

[alin1918]

@rbayet I will do some tests to see how will this work with a big reindexing and see if there is an easy fix for that.