8000 [Messenger] Redis retry doesn't work on 32bit systems (like Raspberry) · Issue #43860 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] Redis retry doesn't work on 32bit systems (like Raspberry) #43860

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.

Sign up for GitHub

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

Jump to bottom
Closed
maartendekeizer opened this issue Oct 31, 2021 · 6 comments
Closed

[Messenger] Redis retry doesn't work on 32bit systems (like Raspberry) #43860

maartendekeizer opened this issue Oct 31, 2021 · 6 comments
Labels
Bug Messenger Status: Needs Review

Comments

@maartendekeizer
Copy link

Symfony version(s) affected

5.3.8

Description

When running Symfony on a 32bits system like a Raspberry PI 4 (ARMv7 = 32 bit) the retry mechanisme with a delay doesn't work. This is caused by reaching the PHP_INT_MAX limit when it does calculate the score for the new 'delayed' message.

vendor\symfony\redis-messenger\Transport\Connection.php:476

                $score = (int) ($this->getCurrentTimeInMilliseconds() + $delayInMs);
var_dump($score);
                $added = $this->connection->zadd($this->queue, ['NX'], $score, $message);

Result:
int(-685048788)

Expected is a very large positive integer, for example:
int(1635697524000)

Due to this bug a retried messages is never taken from the queue.

How to reproduce

Install Redis on a Raspberry 4 Model B
Install a Symony project with Messenger and Redis Messenger Transport.
Configure Redis for messenger.
Create a message handler that alway throws an exception.
Define a retry policy for failed messages with a delay of 60.000 ms.
Queue a message.
Start the message consumer with -vvv option.
The message handling will failed, with a message like messenger.ERROR: Error thrown while handling message App\Message\YourMessage. Sending for retry #1 using 60000 ms delay.
The message will never be retried.

Possible Solution

  • Add this limition to the documentation
  • Make it possible to work with seconds instead of microseconds
  • Substract all scores with 1000000000000 for example

Additional Context

No response
@nicolas-grekas
Copy link
Member

Can't we use a float instead?
@maartendekeizer would you like to give it a try?

@maartendekeizer
Copy link
Author

We currently deployed all the system with a override of this class where we use seconds instead of ms, that is working fine.

https://github.com/phpredis/phpredis#zadd it look like zadd wants a double; so I think float is possible. I will try to make a few real world tests with floats the next weeks.

@nicolas-grekas
Copy link
Member

Any news @maartendekeizer ?

@maartendekeizer
Copy link
Author

@nicolas-grekas waiting on a new raspberry to do some tests with; but getting one is hard these days

maartendekeizer added a commit to maartendekeizer/symfony that referenced this issue Dec 26, 2021
@maartendekeizer
Store millisecond timestamps as float instead of int to prevent int o…
0681a8a 
…verflow issues on 32 bit based systems. Issue symfony#43860
< 8000 div class="TimelineItem-badge">
@maartendekeizer maartendekeizer mentioned this issue Dec 26, 2021
Closed
maartendekeizer added a commit to maartendekeizer/symfony that referenced this issue Dec 27, 2021
@maartendekeizer
Store millisecond timestamps as float instead of int to prevent int o…
8549b3f 
…verflow issues on 32 bit based systems. Issue symfony#43860
maartendekeizer added a commit to maartendekeizer/symfony that referenced this issue Dec 27, 2021
@maartendekeizer
Store millisecond timestamps as float instead of int to prevent int o…
a65fdf0 
…verflow issues on 32 bit based systems. Issue symfony#43860
@nicolas-grekas nicolas-grekas mentioned this issue Dec 27, 2021
Merged
@maartendekeizer
Copy link
Author

@nicolas-grekas tested your branch on a raspberry; works like a charm

@nicolas-grekas
Copy link
Member

thanks for checking

nicolas-grekas added a commit that referenced this issue Dec 28, 2021
@nicolas-grekas
bug #44807 [Messenger] fix Redis support on 32b arch (nicolas-grekas)
9d8638c 
This PR was merged into the 4.4 branch.

Discussion
----------

[Messenger] fix Redis support on 32b arch

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #43860
| License       | MIT
| Doc PR        | -

It took me a while to fix it, but PR is now ready. We have to use `rawCommand()` to work around phpredis casting scores to floats and ignoring any overflow/loss of precision in the process. Passing the score as a string is required here (note that Redis always stores numbers as 64b floats, even when running on x86)

Commits
-------

742279c [Messenger] fix Redis support on 32b arch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Messenger Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nicolas-grekas @maartendekeizer @carsonbot
0