bug #45619 [redis-messenger] remove undefined array key warnings (PhilETaylor)

nicolas-grekas · nicolas-grekas · commit e091d74fa89a · 2022-03-04T16:46:40.000+01:00
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [redis-messenger] remove undefined array key warnings | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no  | Deprecations? | no  | Tickets | Fix #45270  | License | MIT check that we actually have some information back from redis before assuming its an array with 2 keys to avoid undefined array key warnings as per #45270 ---- so the code in question is this: ```php public function get(): ?array { if ($this->autoSetup) { $this->setup(); } $now = microtime(); $now = substr($now, 11).substr($now, 2, 3); $queuedMessageCount = $this->rawCommand('ZCOUNT', 0, $now); while ($queuedMessageCount--) { if (![$queuedMessage, $expiry] = $this->rawCommand('ZPOPMIN', 1)) { break; } if (\strlen($expiry) === \strlen($now) ? $expiry > $now : \strlen($expiry) < \strlen($now)) { // if a future-placed message is popped because of a race condition with // another running consumer, the message is readded to the queue if (!$this->rawCommand('ZADD', 'NX', $expiry, $queuedMessage)) { throw new TransportException('Could not add a message to the redis stream.'); } break; } $decodedQueuedMessage = json_decode($queuedMessage, true); $this->add(\array_key_exists('body', $decodedQueuedMessage) ? $decodedQueuedMessage['body'] : $queuedMessage, $decodedQueuedMessage['headers'] ?? [], 0); } ``` I think what's happening, as I often can have up to 1000 workers (yup, for real, making 1000 http requests to remote sites - see mySites.guru) I think the logic might be flawed. Because, the following line gets the number of messages using ZCOUNT ```php $queuedMessageCount = $this->rawCommand('ZCOUNT', 0, $now); ``` It will then enter a loop counting down that number of messages checking them ```php while ($queuedMessageCount--) { ``` At the same time, 999 other workers can be doing the same thing it gets to a point where one of the workers doesnt get a message back from `ZPOPMIN` and thus we get undefined array keys... You can see the result of ZPOPMIN is no longer an array once empty using the interactive test on the official redis page. <img width="616" alt="Screen Shot 2022-03-02 at 20 21 00" src="https://user-images.githubusercontent.com/400092/156443762-564bc496-19dd-4e60-9b0d-94589d7dcd4d.png"> Commits ------- 758539a [redis-messenger] remove undefined array key warnings
diff --git a/src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php b/src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php
@@ -348,10 +348,12 @@ public function get(): ?array
         $queuedMessageCount = $this->rawCommand('ZCOUNT', 0, $now);
 
         while ($queuedMessageCount--) {
-            if (![$queuedMessage, $expiry] = $this->rawCommand('ZPOPMIN', 1)) {
+            if (!$message = $this->rawCommand('ZPOPMIN', 1)) {
                 break;
             }
 
+            [$queuedMessage, $expiry] = $message;
+
             if (\strlen($expiry) === \strlen($now) ? $expiry > $now : \strlen($expiry) < \strlen($now)) {
                 // if a future-placed message is popped because of a race condition with
                 // another running consumer, the message is readded to the queue
