-
Notifications
You must be signed in to change notification settings - Fork 739
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.
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
Add mqtt reconnect counter #2319
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sages if last reconnect was too recent (i.e. device is rapidly disconnecting and reconnecting)
vaavva
requested review from
CIPop,
danewalton,
ericwolz,
ewertons,
jspaith and
preethi826
as code owners
June 15, 2022 22:01
ericwolz
reviewed
Jun 15, 2022
ericwolz
reviewed
Jun 15, 2022
ericwolz
reviewed
Jun 16, 2022
ericwolz
reviewed
Jun 16, 2022
Ok, this is a better solution. Thanks! |
ericwolz
approved these changes
Jun 16, 2022
jspaith
reviewed
Jun 17, 2022
* Added mqtt disconnect trace logs * Update iothub_client/src/iothubtransport_mqtt_common.c Co-authored-by: Dane Walton <[email protected]> Co-authored-by: Dane Walton <[email protected]>
This is my test. With current setting in the branch, we retry once, and after 3mins we give up and disconnect.
Sending message 1 to IoTHub
Sending message 2 to IoTHub
Sending message 3 to IoTHub
Sending message 4 to IoTHub
Sending message 5 to IoTHub
-> 14:09:36 CONNECT | VER: 4 | KEEPALIVE: 240 | FLAGS: 192 | USERNAME: ericwol-hub.azure-devices.net/snoopy2/?api-version=2020-09-30&DeviceClientType=iothubclient%2f1.9.0%20(native%3b%20WindowsProduct%3a0x00000004%206.2%3b%20x64%3b%20%7bF9FA04EF-2602-43AB-8505-A1EDE028ADD8%7d) | PWD: XXXX | CLEAN: 0
<- 14:09:36 CONNACK | SESSION_PRESENT: true | RETURN_CODE: 0x0
The device client is connected to iothub. result=IOTHUB_CLIENT_CONNECTION_AUTHENTICATED, reason=IOTHUB_CLIENT_CONNECTION_OK
-> 14:09:36 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 2 | PAYLOAD_LEN: 12
-> 14:09:36 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 3 | PAYLOAD_LEN: 12
-> 14:09:36 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 4 | PAYLOAD_LEN: 12
-> 14:09:36 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 5 | PAYLOAD_LEN: 12
-> 14:09:36 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 6 | PAYLOAD_LEN: 12
-> 14:10:37 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 2 | PAYLOAD_LEN: 12
-> 14:10:37 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 3 | PAYLOAD_LEN: 12
-> 14:10:37 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 4 | PAYLOAD_LEN: 12
-> 14:10:37 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 5 | PAYLOAD_LEN: 12
-> 14:10:37 PUBLISH | IS_DUP: false | RETAIN: 0 | QOS: DELIVER_AT_LEAST_ONCE | TOPIC_NAME: devices/snoopy2/messages/events/property_key=property_value | PACKET_ID: 6 | PAYLOAD_LEN: 12
Confirmation callback received for message 1 with result IOTHUB_CLIENT_CONFIRMATION_MESSAGE_TIMEOUT
-> 14:11:38 DISCONNECT
From: John Spaith ***@***.***>
Sent: Friday, June 17, 2022 2:04 PM
To: Azure/azure-iot-sdk-c ***@***.***>
Cc: Eric Wolz ***@***.***>; Review requested ***@***.***>
Subject: Re: [Azure/azure-iot-sdk-c] Add mqtt reconnect counter (PR #2319)
@jspaith commented on this pull request.
________________________________
In iothub_client/src/iothubtransport_mqtt_common.c<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fazure-iot-sdk-c%2Fpull%2F2319%23discussion_r900500446&data=05%7C01%7CEric.Wolz%40microsoft.com%7Ca5f0024329a44c1c30ea08da50a4f389%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637910966639312876%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=u5KXk0Zr94FBJmkgygmLcvAQHe%2BGKP2zvDAi85pgiaE%3D&reserved=0>:
@@ -47,7 +47,7 @@
#define BUILD_CONFIG_USERNAME 24
#define SAS_TOKEN_DEFAULT_LEN 10
#define RESEND_TIMEOUT_VALUE_MIN 1*60
-#define MAX_SEND_RECOUNT_LIMIT 2
+#define MSG_TIMEOUT_VALUE_MIN 2*60
Yup, you're totally right I'd forgot that. The code here with the 2 minutes is fine indeed.
Which is another good reason I guess to get rid of this retryCount since it wasn't mapping up to how one would naively think it'd work.
-
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fazure-iot-sdk-c%2Fpull%2F2319%23discussion_r900500446&data=05%7C01%7CEric.Wolz%40microsoft.com%7Ca5f0024329a44c1c30ea08da50a4f389%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637910966639312876%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=u5KXk0Zr94FBJmkgygmLcvAQHe%2BGKP2zvDAi85pgiaE%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAI5PLYOJ56GRDVWCPX7YVHLVPTR5LANCNFSM5Y4Y32BQ&data=05%7C01%7CEric.Wolz%40microsoft.com%7Ca5f0024329a44c1c30ea08da50a4f389%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637910966639312876%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iYveu1k%2Fn7x%2BH3hWTnWO%2B5S3UetoMQI6cQMeJQ9j9TU%3D&reserved=0>.
You are receiving this because your review was requested.Message ID: ***@***.******@***.***>>
|
…Azure/azure-iot-sdk-c into vaavva/add-mqtt-reconnect-timer
…Azure/azure-iot-sdk-c into vaavva/add-mqtt-reconnect-timer
ewertons
requested changes
Jun 20, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests must be updated to reflect the changes as well.
iothub_client/tests/iothubtransport_mqtt_common_ut/iothubtransport_mqtt_common_ut.c
Show resolved
Hide resolved
…Azure/azure-iot-sdk-c into vaavva/add-mqtt-reconnect-timer
…less than a minute
…Azure/azure-iot-sdk-c into vaavva/add-mqtt-reconnect-timer
…retry the message each time
ericwolz
approved these changes
Jun 24, 2022
…Azure/azure-iot-sdk-c into vaavva/add-mqtt-reconnect-timer
ewertons
approved these changes
Jun 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
devdoc
folder and added or modified requirements.main
branch.main
branch prior to submission and re-merged as needed after I took any feedback.Reference/Link to the issue solved with this PR (if any)
Description of the problem
When we added a retry of messaged after reconnection to ensure mqtt messages are sent in order, this caused us to use up the retries very quickly. This PR adds a separate counter for retrying messages in the case of a reconnect so that if there are a few disconnect/reconnects in a row it doesn't automatically expire the message.
Description of the solution