Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use same AMQP 0.9.1 header for correlation ID #8680

Merged
merged 3 commits into from
Jun 26, 2023
Merged

Use same AMQP 0.9.1 header for correlation ID #8680

merged 3 commits into from
Jun 26, 2023

Conversation

ansd
Copy link
Member

@ansd ansd commented Jun 26, 2023

For correlation IDs greater than 255 bytes, AMQP 1.0 to AMQP 0.9.1 conversion puts the correlation ID into an AMQP 0.9.1 header called x-correlation-id, see

{Headers, CorrId091} = message_id(CorrId, <<"x-correlation-id-type">>, Headers1),

Let's use the same key for the MQTT 5.0 to AMQP 0.9.1 conversion.

For correlation IDs greater than 255 bytes, AMQP 1.0 to AMQP 0.9.1
conversion puts the correlation ID into an AMQP 0.9.1 header called
x-correlation-id (see
https://github.com/rabbitmq/rabbitmq-server/blob/a17b28ec618cdf79a29fa520748dbe340561e197/deps/rabbit/src/rabbit_msg_record.erl#L380
)
Let's use the same key for the MQTT 5.0 to AMQP 0.9.1 conversion.
@michaelklishin
Copy link
Member

It's a good question what release this should go into. 3.13.0?

@ansd
Copy link
Member Author

ansd commented Jun 26, 2023

Yes, this should go into 3.13.0.

@ansd ansd added this to the 3.13.0 milestone Jun 26, 2023
ansd added 2 commits June 26, 2023 07:37
Attempt to fix the following flake:
```
=== Ended at 2023-06-26 07:13:34
=== Location: [{shared_SUITE,events,570},
              {test_server,ts_tc,1782},
              {test_server,run_test_case_eval1,1291},
              {test_server,run_test_case_eval,1223}]
=== === Reason: no match of right hand side value []
  in function  shared_SUITE:events/1 (shared_SUITE.erl, line 570)
  in call from test_server:ts_tc/3 (test_server.erl, line 1782)
  in call from test_server:run_test_case_eval1/6 (test_server.erl, line 1291)
  in call from test_server:run_test_case_eval/9 (test_server.erl, line 1223)
```

of test
```
-group [web_mqtt,v3,cluster_size_1] -case events
```

The logs showed that deletion of the exclusive queue took place in the
same millisecond as the server received the DISCONNECT:
```
2023-06-26 07:13:32.838282+00:00 [debug] <0.2494.0> Received a CONNECT, client ID: events, username: undefined, clean start: true, protocol version: 3, keepalive: 60, property names: []
2023-06-26 07:13:32.838436+00:00 [debug] <0.2494.0> MQTT connection 127.0.0.1:38808 -> 127.0.0.1:21007 picked vhost using plugin_configuration_or_default_vhost
2023-06-26 07:13:32.838523+00:00 [debug] <0.2494.0> User 'guest' authenticated successfully by backend rabbit_auth_backend_internal
2023-06-26 07:13:32.838672+00:00 [info] <0.2494.0> Accepted Web MQTT connection 127.0.0.1:38808 -> 127.0.0.1:21007 for client ID events
2023-06-26 07:13:33.147196+00:00 [debug] <0.2494.0> Received a SUBSCRIBE with subscription(s) [{mqtt_subscription,<<"my/topic">>,
2023-06-26 07:13:33.147196+00:00 [debug] <0.2494.0>                                             {mqtt_subscription_opts,0,false,
2023-06-26 07:13:33.147196+00:00 [debug] <0.2494.0>                                              false,0,undefined}}]
2023-06-26 07:13:33.457541+00:00 [debug] <0.2494.0> Received an UNSUBSCRIBE for topic filter(s) [<<"my/topic">>]
2023-06-26 07:13:33.762171+00:00 [debug] <0.2494.0> Received a DISCONNECT with reason code 0 and properties #{}
2023-06-26 07:13:33.762350+00:00 [info] <0.2494.0> Web MQTT closing connection 127.0.0.1:38808 -> 127.0.0.1:21007
2023-06-26 07:13:33.762780+00:00 [debug] <0.2504.0> Deleting exclusive queue 'mqtt-subscription-eventsqos0' in vhost '/' because its declaring connection <0.2494.0> was closed
```
However, there could be some delay between disconnecting on the client
WebMQTT side and the processor processing the DISCONNECT packet.
Test
```
//deps/rabbitmq_mqtt:shared_SUITE-mixed --test_env FOCUS="-group [web_mqtt,v3,cluster_size_1] -case block_only_publisher"
```
was flaky:
```
=== Ended at 2023-06-26 07:09:57
=== Location: [{shared_SUITE,block_only_publisher,1323},
              {test_server,ts_tc,1782},
              {test_server,run_test_case_eval1,1291},
              {test_server,run_test_case_eval,1223}]
=== === Reason: no match of right hand side value {error,ack_timeout}
  in function  shared_SUITE:block_only_publisher/1 (shared_SUITE.erl, line 1323)
  in call from test_server:ts_tc/3 (test_server.erl, line 1782)
  in call from test_server:run_test_case_eval1/6 (test_server.erl, line 1291)
  in call from test_server:run_test_case_eval/9 (test_server.erl, line 1223)
```
It seems that the ack_timeout of 1 second was too low for a
subscription.
@ansd ansd merged commit b3795f5 into main Jun 26, 2023
@ansd ansd deleted the x-correlation-id branch June 26, 2023 11:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants