Fix subscribed_to maybe empty

[uuip]

Description

According to these pull requests: #7040 and #7220, I improved the behavior when subscribed_to has been empty, no additional requests.

[codecov]

Codecov Report

Merging #7232 (af47c7f) into master (95015a1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #7232   +/-   ##
=======================================
  Coverage   89.32%   89.32%           
=======================================
  Files         138      138           
  Lines       16775    16776    +1     
  Branches     2450     2451    +1     
=======================================
+ Hits        14984    14985    +1     
- Misses       1559     1560    +1     
+ Partials      232      231    -1     

Flag	Coverage Δ
unittests	89.31% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
celery/backends/redis.py	92.26% <100.00%> (+0.04%)	⬆️
celery/bin/base.py	44.08% <0.00%> (-0.30%)	⬇️
celery/canvas.py	94.95% <0.00%> (ø)
celery/app/log.py	96.92% <0.00%> (ø)
celery/__init__.py	84.48% <0.00%> (ø)
celery/app/base.py	96.45% <0.00%> (ø)
celery/app/task.py	94.70% <0.00%> (ø)
celery/bin/beat.py	42.30% <0.00%> (ø)
celery/bin/call.py	88.88% <0.00%> (ø)
celery/app/trace.py	98.60% <0.00%> (ø)
... and 25 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95015a1...af47c7f. Read the comment docs.

[auvipy]

could you please add additional unit tests for the changes proposed?

[lgtm-com]

This pull request introduces 1 alert when merging 6c977aa into 95015a1 - view on LGTM.com

new alerts:

• 1 for Module is imported with 'import' and 'import from'

[thedrow]

Overall this looks to be the correct fix. I do have a question and we do need a unit test to merge this.

[lgtm-com]

This pull request introduces 1 alert when merging 1140174 into 95015a1 - view on LGTM.com

new alerts:

• 1 for Module is imported with 'import' and 'import from'

[uuip]

@thedrow test_asynchronous.py reduced the coverage, but this pr didn't modify that file.

[auvipy]

@thedrow test_asynchronous.py reduced the coverage, but this pr didn't modify that file.

no worries the PR coverage looks good.

[uuip]

There is another problem than I haven't submitted.

    def _reconnect_pubsub(self):
        self._pubsub = None

    def _reconnect_pubsub(self):
        # release old connection before assign to new connection
        self._pubsub.reset()

I'm not sure yet.

[auvipy]

There is another problem than I haven't submitted.

    def _reconnect_pubsub(self):
        self._pubsub = None

    def _reconnect_pubsub(self):
        # release old connection before assign to new connection
        self._pubsub.reset()

I'm not sure yet.

can you please elaborate more?

[uuip]

Until 6 hours ago, I completed a test which running 12 hours.
Logically, the app is very simple, repeating tasks every 10 seconds.
At the first 5 minute，there were less than 50 connections，at the ending of test, there were actually more than 1200 connections, and their status were not time_wait, but ESTAB, I suspect that those connections were not closed normally by celery or redis-py.

There is another problem than I haven't submitted.

    def _reconnect_pubsub(self):
        self._pubsub = None

    def _reconnect_pubsub(self):
        # release old connection before assign to new connection
        self._pubsub.reset()

I'm not sure yet.

[auvipy]

@pawl can you share your thought on last comment of @uuip please?

[uuip]

Shall we focus this pr Fix subscribed_to maybe empty? As for the number of connections, I will analyze it again, an issue or pr in future. Thanks for your kind help.

[auvipy]

Shall we focus this pr Fix subscribed_to maybe empty? As for the number of connections, I will analyze it again, an issue or pr in future. Thanks for your kind help.

you can also search for related issues & create a new discussion first to have more consensus

[uuip]

@thedrow here is a review assigned to you, if you have time.

[auvipy]

you will need to adjust tests with the codes as well

[auvipy]

Also please do more thorough check of codes and tests before another round of review

[uuip]

@auvipy I have removed redundant code, please recheck.

[auvipy]

ok, i will re check tomorrow. in the mean time can you please recheck that you are done with the issue you are trying to resolve?

[uuip]

When using redis as backend, gevent or eventlet as concurrency, celery app hold too many redis connections. There are some discussion on #6985.
redis-py always use independent connection for every command, one backend instance is enough.
I fix this scene via a workaround but missing spaces around operator -- this cause previous checking error. This is a workaround, not common solution. After removed it, now all checks have passed.

[thedrow]

Just a minor fix and we're there.

[auvipy]

thanks both