Skip to content

added retry_on_timeout and socket_keepalive to redis connection#5952

Merged
auvipy merged 1 commit into
celery:masterfrom
Seleznev-nvkz:master
Feb 7, 2020
Merged

added retry_on_timeout and socket_keepalive to redis connection#5952
auvipy merged 1 commit into
celery:masterfrom
Seleznev-nvkz:master

Conversation

@Seleznev-nvkz

@Seleznev-nvkz Seleznev-nvkz commented Feb 4, 2020

Copy link
Copy Markdown
Contributor

resolve #5936
Added two parameters to provide to Redis connection. Also, added description to docs.

thedrow
thedrow approved these changes Feb 6, 2020
View reviewed changes

@thedrow thedrow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@auvipy
auvipy merged commit d4df182 into celery:master Feb 7, 2020
@auvipy auvipy added this to the 4.4.x milestone Feb 7, 2020
@frost-nzcr4 frost-nzcr4 mentioned this pull request Feb 11, 2020
Merged
@jaap3

jaap3 commented Mar 8, 2020

Copy link
Copy Markdown

I've updated a project to the most recent Celery version (4.4.1) and suddenly get these errors: TypeError redis.connection in make_connection error__init__() got an unexpected keyword argument 'socket_keepalive'. Could this be related to this PR?

As far as I can tell the version of the redis client I'm using (3.4.1) should accept this keyword argument.

When I upgraded I updated the following packages:

-  billiard==3.6.2.0
+  billiard==3.6.3.0
-  celery==4.4.0
+  celery==4.4.1
-  kombu==4.6.7
+  kombu==4.6.8

Not sure what's going on. I'm not seeing anyone else with this issue in all the relevant issue trackers.

@jaap3

jaap3 commented Mar 8, 2020

Copy link
Copy Markdown

Looking a bit further this seems very similar to #2903

It appears that not all Redis connection types support the socket_keepalive keyword argument.

e.g. UnixDomainSocketConnection's __init__ does not accept this kwarg: https://github.com/andymccurdy/redis-py/blob/3.4.1/redis/connection.py#L868

@jaap3 jaap3 mentioned this pull request Mar 9, 2020
Closed
18 tasks
john-bodley
john-bodley reviewed Mar 11, 2020
View reviewed changes
Comment thread docs/userguide/configuration.rst
To retry reading/writing operations on TimeoutError to the Redis server,
used by the redis result backend.

.. setting:: socket_keepalive

@john-bodley john-bodley Mar 11, 2020 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Seleznev-nvkz per line 188 in celery/backends/redis.py shouldn't this be redis_socket_keepalive rather than socket_keepalive?

Also should there be . versionadded:: 4.4.1 for both of these added settings?

@Seleznev-nvkz Seleznev-nvkz Mar 12, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great catch! Thank you.

@Seleznev-nvkz Seleznev-nvkz Mar 12, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done #6000

@Seleznev-nvkz

Seleznev-nvkz commented Mar 12, 2020

Copy link
Copy Markdown
Contributor Author

Looking a bit further this seems very similar to #2903

It appears that not all Redis connection types support the socket_keepalive keyword argument.

e.g. UnixDomainSocketConnection's __init__ does not accept this kwarg: https://github.com/andymccurdy/redis-py/blob/3.4.1/redis/connection.py#L868

Thanks for your feedback! Sorry for long response - will answer in the issue.

auvipy added a commit that referenced this pull request Jul 8, 2020
@thedrow @verhovsky
@auvipy @jainal09 @gsfish @dipAch @spengjie @chris-griffin @mhewedy @phated @tamsanh @eriktews @anilaratna2 @wyguy444 @galCohen88 @alfred-sa @paramkpr @sveniu @safwanrahman Aissaoui Anouar @qdzzyb2014 @Alireza2n @marcosmoyano @shenek @AndrewPix @fladi @dejlek @yannickschuchmann @matteius @tirkarthi @bernd-wechner @soldag @uddmorningsun @marfgold1 @Wooden-Robot @imomaliev @AlexRiina @xncbf @nixphix @martey @Seleznev-nvkz @Prodge @adyouri @Ixiodor @abhishekakamai @allanlei @M1ha-Shvn @hicay @michamos @chrisburr @tildedave @the-owl Michael Gaddis @epwalsh @TalRoni @lpsinger @neonbunny Abhishek @theirix @yukihira1992 @jpays-cnp @gward Alexa Griffith @heedong-jung @heedongsamsung @shreyanshk @georgedorn @Alphadelta14 @azimjohn @ysde @amirzia @nadflinn @nezhar @specialorange @aeijdenberg rafaelreuber @coderanger @clems4ever Mathieu Chataigner @eugeneyalansky @leonardsocialcodeinc @xinydev @watercrossing @hanchau shaoziwei @mchataigner @Anakael @ckdanny @Tirzono @dtilloy-criteo @anthonynsimon @lironhl @Darkheir @Kirade @singlaive Murphy Meng @whtsky @kwist-sgr @ldragan @nijel @radimsuckr @art-vasilyev @kakakikikeke-fork @baixuexue123 baixue @prashnts @AbdealiLoKo