-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 initial autoResubscribe support #13145
Add initial autoResubscribe support #13145
Conversation
PR #13145: Size comparison from 246473c to 3a62164 Increases above 0.2%:
Increases (26 builds for cyw30739, efr32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (30 builds for cyw30739, efr32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
3a62164
to
57f7b65
Compare
PR #13145: Size comparison from b8049f0 to 57f7b65 Increases above 0.2%:
Increases (28 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (32 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
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.
Can you please add some tests to controller/tests/data_model/TestRead?
Just add subscription test that has used this new API. Currently we cannot simulate the liveness timeout in the test |
d25fdd7
to
0813408
Compare
My approval can still stand. But I am wondering what subset of cases where subscriptions can be lost would be repaired by attempting resubscribe over the existing session. Is this implicitly relying on session resumption? I would have figured in many cases, lost subscriptions will also lead to lost sessions. |
In data I have previously mined from devices in the field at Nest that had a similar concept of subscriptions, the bulk of subscriptions failures were due to networking issues (momentary Thread and Wifi connectivity disturbances) that would happen sometimes, a couple of times a day. In those cases, both sides of the subscription are still up and running. However, this question you pose brings up a related point I have brought up that is unique to Matter. Because an interaction error of timeout in nature cannot distinguish between the myriad of reasons why (including the worst, which is the other side has rebooted and lost their CASE session, or changed their IP address), I don't see how we can continue to re-use the same session indefinitely. One possible heuristic is to re-subscribe just once, and if that fails, to restart the whole mDNS + CASE session setup process...horrible, but I'm not sure how to make that better. |
190b491
to
8e9eab1
Compare
/rebase |
8e9eab1
to
31aaa71
Compare
PR #13145: Size comparison from 43a1e1a to 31aaa71 Increases above 0.2%:
Increases (29 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (33 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
31aaa71
to
ec0d863
Compare
PR #13145: Size comparison from ddb7ce7 to ec0d863 Increases above 0.2%:
Increases (29 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (33 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
Problem
Need to add auto-resubscribe capability for subscribe interaction
Change overview
Extend ReadClient with re-subscription function and add public function, SendSubscribeRequest with && ReadPrepareParams, and further add mpResubscribeDelegate that can be used for user-defined resubscribe policy.
Add OnDeallocatePaths callback, which could deallocated attribute path list and event path list.
Add ResubscribePlolicyCB and provide Fibonacci backup for default resubscribe policy
Note: At a given time in the system, you can either have a single subscription with re-sub enabled that that has mKeepSubscriptions = false, OR, multiple subs with re-sub enabled with mKeepSubscriptions = true.
Testing
Update unit test and cirque tests.
Manually simulate the error situation and validate the resubscribe logic.