Recreate channel after reconnect. Message confirmation feature #418

dbalabka · 2016-12-22T17:25:02Z

Fixed issue with missed channels after reconnect

Long running producer job may lead to following exception:

PHP Fatal error:  Uncaught exception 'PhpAmqpLib\Exception\AMQPProtocolConnectionException' with message 'CHANNEL_ERROR - expected 'channel.open'' in /home/sites/dmitry.balabka/src/opcart_test/vendor/videlalvaro/php-amqplib/PhpAmqpLib/Connection/AbstractConnection.php:714
Stack trace:
#0 [internal function]: PhpAmqpLib\Connection\AbstractConnection->connection_close(Object(PhpAmqpLib\Wire\AMQPReader))
#1 /home/sites/dmitry.balabka/src/opcart_test/vendor/videlalvaro/php-amqplib/PhpAmqpLib/Channel/AbstractChannel.php(166): call_user_func(Array, Object(PhpAmqpLib\Wire\AMQPReader))
#2 /home/sites/dmitry.balabka/src/opcart_test/vendor/videlalvaro/php-amqplib/PhpAmqpLib/Channel/AbstractChannel.php(338): PhpAmqpLib\Channel\AbstractChannel->dispatch('10,50', '\x01\xF8'CHANNEL_ERRO...', NULL)
#3 /home/sites/dmitry.balabka/src/opcart_test/vendor/videlalvaro/php-amqplib/PhpAmqpLib/Connection/AbstractConnection.php(617): PhpAmqpLib\Channel\AbstractChannel->wait()
#4 /home/sites/dmitry.balabka/src/opcart_test/vendor/videlalvaro/php-amqplib in /home/sites/dmitry.balabka/src/opcart_test/vendor/videlalvaro/php-amqplib/PhpAmqpLib/Connection/AbstractConnection.php on line 714

This exception appears right after job finish.
It happen in case when during job is sending messages catch some network issues and tried to reconnect. Use use reconnect like self-healing feature instead of just start job from scratch, because multiple messages may already consumed or/and restarting job may took few hours and it is inappropriate. Current reconnect implementation do not recreate channels. It leads to issue that messages are sending to no where and are not fall into queue. Also you will get above mentioned exception right after job finish process messages, because we can't close channels on destruct that do not exists.

We can simulate this case:

Start job that sends messages and have self-healing feature that use reconnect on some network exception
Open RabbbitMQ admin panel
Goto Connections tab
Open Active connection details
Click Force close connection
Than job will get following exception:

exception 'PhpAmqpLib\Exception\AMQPRuntimeException' with message 'Broken pipe or closed connection' in .../vendor/videlalvaro/php-amqplib/PhpAmqpLib/Wire/IO/StreamIO.php:174

AR: Job will continue to send messages, but messages will not fall into queue and will return exception at the end
ER: Queue size will contine grow

Implemented message confirmantion feature enabling for channels

Your RabbitMQ server may block connections. For example when qeueue is out of memory. If you use default producer settings it will lead that all your messages will be dropped. The main issue is that right after connection was blocked Rabbit isn't able to accept any messages, but publisher is still can publish and catch connection exception only after some time. So some messages will be dropped. Publisher job will sends messages until you will get followinge exception

Error sending data. Socket connection timed out

According to documentation rabbit sends notification to all clients that server supports connection blocking and you must be ready:
https://www.rabbitmq.com/connection-blocked.html

connection.blocked:	true

To catch this situation we must use special method:
https://github.com/php-amqplib/php-amqplib/blob/2bc16b9f0859357a26e6b253c4552b61071dfdd9/PhpAmqpLib/Helper/Protocol/Protocol091.php#L152
according to php-amqplib/php-amqplib and php-amqplib/RabbitMqBundle this method never used. So blocked connection case isn't covered by bundle and library.

Unfortunatly Rabbit by default do not garantee that some of messages will be delivered if you have network problem or Rabbit have blocked connections because of hardware resources issues. The only way how to garantee it is to use transations. But transactions are unnecessarily heavyweight and decrease throughput by a factor of 250. To remedy this, a confirmation mechanism was introduced. Confirmation is more lightweight.
Unfortunalty PHP isn't async and such message confirmation will lead to some overhead. But currently it is a only way how to garantee message delivery. Confirmation garantee that not only connection exists, but also channel and exchange that will consume message.

To reproduce this issue:

Run long runnnig publisher job
Set low memory limit using RabbitMQ console admin tool

PS C:\Program Files (x86)\RabbitMQ Server\rabbitmq_server-3.1.3\sbin> .\rabbitmqctl.bat set_vm_memory_high_watermark 0.2
Setting memory threshold on 'rabbit@DMITRYB-PC' to 0.2 ...
...done.

Restore memory limit
ER: job must catch exception and reconnect w/o lost any messages
Please check messages count in queue

Сonclusions

To properly catch blocking connection reconnect must work properly and you must use message confirmation.

When connection is blocked it may happen that we send some messages twice and duplicates messages may appear in queue, because we can't confirm that last message before blocking exception appears isn't sent and we must send it once again.
This case may affect only small amount of messages, because in cases when Rabbit blocks connection it means that Rabbit not accepts any new connection as well so reconnect method will hung untill Rabbit outages will be solved. So we asume that only one duplicate message will be send to one customer in case when connection has been blocked.
I didn't found how this case may be handled.

…tream # Conflicts: # RabbitMq/BaseAmqp.php # composer.json

oleg-andreyev · 2016-12-22T19:10:23Z

tests are failing

oleg-andreyev · 2016-12-22T19:11:37Z

RabbitMq/BaseAmqp.php

+         * see https://github.com/php-amqplib/php-amqplib/commit/2ccc97ca5b1229f9b12ea47fbab6c16fad26df41
+         * see https://github.com/php-amqplib/php-amqplib/commit/c87469ecbbf38fdc18688d3216c1e253d640ba32
+         */
+        $this->conn->reconnect();


2.6.3 is released

Need time to recheck why it was done

sorry not fixed yet

@oleg-andreyev 2.6.3 and even 2.7.0-rc1 doesn't help :(

oleg-andreyev · 2016-12-22T19:12:37Z

Tests/RabbitMq/ProducerTest.php

+/*
+ * This file is part of the OpCart software.
+ *
+ * (c) 2015, OpticsPlanet, Inc


please use proper header

oleg-andreyev · 2016-12-22T19:12:52Z

Tests/RabbitMq/ProducerTest.php

+ * file that was distributed with this source code.
+ */
+
+namespace OldSound\RabbitMqBundle\Tests\RabbitMq;


old namespace

Good catch...

but seems to be wrong:
https://github.com/php-amqplib/RabbitMqBundle/blob/master/Tests/RabbitMq/ConsumerTest.php#L3

oleg-andreyev · 2016-12-22T19:13:23Z

composer.json

@@ -31,7 +31,8 @@
    },
    "extra": {
        "branch-alias": {
-            "dev-master": "1.10.x-dev"
+            "dev-master": "1.10.x-dev",
+            "dev-reconnect-fix-upstream": "1.13.x-dev"


is it required?

I was done for our needs. Have to be removed.

skafandri · 2016-12-22T19:33:04Z

Can you explain a bit what issue are you fixing? Use case, what exception/behaviour did you encountered, under which settings..
If you received an exception for example, including it here will be very helpful for everyone that woyld just copy/paste it and google it.

dbalabka · 2016-12-22T20:33:42Z

I will describe my complete investigation and describe in which cases it may help.

dbalabka · 2017-01-12T14:22:01Z

@skafandri take a look on description

skafandri · 2017-01-27T14:58:43Z

This is interesting. Can you please verify your notes about php-amqplib 2.6.3 and add a new line at the end of Tests/RabbitMq/ProducerTest.php?

…tream

dbalabka · 2017-05-05T15:02:44Z

@skafandri I have update code with accordance of php-amqplib version 2.6.3. I have added version constraint into compose.json.

It will be good to add some integration tests using real RabbitMQ instance like php-amqplib have to cover reconnect and confirmation features.

Also it will be good to add my exaplanation how to handle server outages and missed messages into documentation of this bundle, because it is not simple things and developers must be aware.

dbalabka · 2017-05-05T15:03:52Z

@oleg-andreyev please take a look on PR

dbalabka · 2017-06-02T10:03:38Z

@skafandri what we have to do to merge this PR?

stloyd · 2017-06-02T10:15:20Z

RabbitMq/BaseAmqp.php

+            $this->ch = null;
+        } catch (\Exception $e) {
+            // ignore exception on Channel object destructor
+            // TODO: this workaround can be removed after php-amqplib will be updated till 2.6.3


You could adjust composer.json to force version of the lib too: ^2.6.3

@stloyd removed

stloyd · 2017-06-02T10:16:06Z

RabbitMq/BaseAmqp.php

+     */
+    public function setWaitConfirmationTimeout($timeout)
+    {
+        if (!is_int($timeout) || $timeout < 0) {


if (!$timeout || !is_int($timeout)) {

@stloyd not sure why this condition need to be changed. Please explain.
I have added few test cases for this this method test. By changing this condition, as you propose, it lead to two test cases fail:

When timeout is zero

Timeout is less then zero

@stloyd ping

stloyd · 2017-06-02T10:17:00Z

composer.json

@@ -15,7 +15,7 @@
        "symfony/config":               "~2.3 || ~3.0",
        "symfony/yaml":                 "~2.3 || ~3.0",
        "symfony/console":              "~2.3 || ~3.0",
-        "php-amqplib/php-amqplib":      "~2.6",
+        "php-amqplib/php-amqplib":      "~2.6 >=2.6.3",


As mentioned, should be ^2.6.3

@stloyd fixed

chernecov · 2017-09-07T14:19:49Z

@torinaki @stloyd
Hello, can you share updates on this pull request?
We really need it to be merged.
Long running jobs with publishing messages fail when one RabbitMQ node restarts. (by puppet for example)

dbalabka · 2017-09-08T08:45:07Z

@stloyd, @skafandri we have started migration from videlalvaro/php-amqplib to version 2.6.3 of php-amqplib/php-amqplib and noticed that reconnect still doesn't work

My initial fix has addresed this issue in previous commit:
https://github.com/ecentria/RabbitMqBundle/blob/36cafbcbee5891df3b9aaef49b805a852fc166f8/RabbitMq/BaseAmqp.php#L89
and assumed that following PR must be merged:
php-amqplib/php-amqplib@2ccc97c
php-amqplib/php-amqplib@c87469e
I have missed that one of this PR was merge into 2.7, but not in 2.6, so composer deps are incorrect, but anyway reconnect doesn't work even with 2.7.0-rc1.

Currenlty we are investigating with @chernecov what must be done to make it work.

dbalabka · 2017-09-08T08:47:21Z

composer.json

@@ -15,7 +15,7 @@
        "symfony/config":               "~2.3 || ~3.0",
        "symfony/yaml":                 "~2.3 || ~3.0",
        "symfony/console":              "~2.3 || ~3.0",
-        "php-amqplib/php-amqplib":      "~2.6",
+        "php-amqplib/php-amqplib":      "^2.6.3",


Reconnect fix doesn't work even in 2.7

Note that originaly I have assumed that following PRs must be merged, but it is still not enought.
php-amqplib/php-amqplib@2ccc97c
php-amqplib/php-amqplib@c87469e

dbalabka · 2017-10-13T16:48:36Z

@stloyd, @skafandri can you please review our conclusions?

github-actions · 2021-03-16T20:26:10Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions · 2021-03-30T20:30:13Z

This PR was closed because it has been stalled for 10 days with no activity.

dbalabka · 2021-04-01T19:20:43Z

There no many changes. I can update the PR and test with latest version.

dbalabka added 8 commits May 23, 2016 15:21

Fixed issue with reconnect and not existing channels

36cafbc

Added additional alise for fix

4d5fbbb

Added additional alise for fix

aac0c2d

Added replace into composer.json

72e93bd

Remove changes from composer json

8c7ecc2

Rollback changes to composer json

b3ad3f9

Merge remote-tracking branch 'upstream/master' into reconnect-fix-ups…

f66fc27

…tream # Conflicts: # RabbitMq/BaseAmqp.php # composer.json

Fixed dev branch name

68d9f1f

oleg-andreyev suggested changes Dec 22, 2016

View reviewed changes

dbalabka added 2 commits December 22, 2016 21:53

Fixed tests. Rollback changes in composer

e73698b

Fixed tests

6842474

dbalabka changed the title ~~Recreate channel after reconnect and message confimration feature~~ Recreate channel after reconnect and message confirmation feature Jan 12, 2017

dbalabka changed the title ~~Recreate channel after reconnect and message confirmation feature~~ Recreate channel after reconnect. Message confirmation feature Jan 12, 2017

dbalabka added 5 commits April 12, 2017 15:46

Added one line

227e305

Merge remote-tracking branch 'upstream/master' into reconnect-fix-ups…

693ba5a

…tream

Merge remote-tracking branch 'upstream/master' into reconnect-fix-ups…

86c3937

…tream

Added minimum required php-amqplib

9cde59c

Updated with accordance of php-amqplib 2.6.3 changes

7b94fd5

stloyd requested changes Jun 2, 2017

View reviewed changes

stloyd added the Enhancement label Jun 2, 2017

dbalabka added 2 commits July 31, 2017 13:41

Fixed composer deps. Added test cases. Removed fix.

1a1b63c

Fixed test for unsupported PHP versions

6a97fde

Fixed test for unsupported PHP versions

70243ca

dbalabka commented Sep 8, 2017

View reviewed changes

Rollback reconnect fix

15af878

chernecov mentioned this pull request Jan 15, 2018

Graceful max execution options is now available for MC #468

Merged

github-actions bot added the no-pr-activity label Mar 16, 2021

github-actions bot closed this Mar 30, 2021

Recreate channel after reconnect. Message confirmation feature #418

Recreate channel after reconnect. Message confirmation feature #418

Uh oh!

Conversation

dbalabka commented Dec 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixed issue with missed channels after reconnect

Implemented message confirmantion feature enabling for channels

Сonclusions

Uh oh!

oleg-andreyev commented Dec 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbalabka Dec 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skafandri commented Dec 22, 2016

Uh oh!

dbalabka commented Dec 22, 2016 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dbalabka commented Jan 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skafandri commented Jan 27, 2017

Uh oh!

dbalabka commented May 5, 2017

Uh oh!

dbalabka commented May 5, 2017

Uh oh!

dbalabka commented Jun 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chernecov commented Sep 7, 2017

Uh oh!

dbalabka commented Sep 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbalabka commented Oct 13, 2017

Uh oh!

github-actions bot commented Mar 16, 2021

Uh oh!

dbalabka commented Dec 22, 2016 •

edited

Loading

dbalabka Dec 22, 2016 •

edited

Loading

dbalabka commented Dec 22, 2016 via email •

edited

Loading

dbalabka commented Jan 12, 2017 •

edited

Loading

dbalabka commented Apr 1, 2021 •

edited

Loading