[IM] Update read transaction resource minimas #18884
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
pullapprove
bot
requested review from
andy31415,
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
hawk248,
harsha-rajendran,
isiu-apple,
jelderton and
jepenven-silabs
pullapprove
bot
requested review from
xylophone21,
yufengwangca and
yunhanw-google
|
PR #18884: Size comparison from e023667 to 95e2104 Increases (27 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (3 builds for cc13x2_26x2)
Full report (27 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
bzbarsky-apple
requested changes
May 28, 2022
|
/rebase |
|
PR #18884: Size comparison from b8e9ab1 to 4ed3d16 Increases (30 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (5 builds for cc13x2_26x2)
Full report (30 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
|
Terminology Guaranteed resources:
Behavior For an incoming read request, when we have enough resource for handling it, we always accept it regardless the actual resource usage. Test Case: 1.3, Test Case 7, and assumed by all test cases. The following test case all assumes we cannot handle it either we don't have enough free items in the path pool or we don't have enough free read handlers. Note: Base on the response code (and the branches in the code & conditions of the loops), we don't always discuss both cases. For an oversized incoming requests:
For an incoming non-oversized request:
|
|
PR #18884: Size comparison from 663ecf4 to fbeca4c Increases above 0.2%:
Increases (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (6 builds for cc13x2_26x2, esp32)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
bzbarsky-apple
approved these changes
Jun 16, 2022
andy31415
approved these changes
Jun 16, 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.
Problem
Fixes #18610
Fixes #17418
Change overview
Testing
TestReadHandler_TwoParallelReads, addsTestReadHandler_ParallelReadswith 8 cases (including 4 cases around PASE sessions).There are 10 cases covered by the test.
We will serve the read requests at best efforts, and try not to evict the ongoing read requests (expect the client to retry later since the read transactions are short.)
Oversized read requests will be rejected when we don't have enough resources to serve it.
Case 1: Handling of oversized read requests
But we will still evict the read requests when:
Case 2: One read request in one fabric is oversized, then one incoming read request from another fabric => accept by evicting one (when necessary).
(Repeated with different order to cover https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR818)
Note: the incoming read requests won't evict another read requests in the same fabric:
Case 3: One fabric is oversized, then one incoming read request from the same fabric => reject with Busy (when we don't have enough resources to serve it).
Reject if the fabric is oversized: https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR936
Case 4: One fabric is oversized (with no oversized read requests), and one incoming read request from another fabric => accept by evicting one (when necessary)
(Repeated with different order to cover https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR825)
Case 2 + Case 4: evicting priority:
When handling / evicting read transactions for CASE sessions, the logic depends on whether the fabric table is full:
Case 5: When the device's fabric table is not full, PASE sessions are counted as a valid fabric and can evict existing read transactions.
Covers https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR843
Case 6: The device's fabric table is full, PASE sessions won't be counted as a "valid fabric" and it cannot evict existing read transactions.
Covers https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR916
Case 7: We will accept read transactions on PASE session when the fabric table is full but we have enough resources for it.
Case 8: If the fabric table on the device is full, read transactions on PASE session will always be evicted.
Covers https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR967 and https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR847
Case 9 -- PASE sessions are counted as a "normal fabric" if fabric table is not full, the read transactions from PASE sessions have (almost) the same priority when being evicted by incoming read transactions.
Covers https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR967
Case 10: Just the other side of case 9: read transaction on PASE session will be evicted before evicting any other read transactions, even when the normal fabric is oversized.
Also covers https://github.com/project-chip/connectedhomeip/pull/18884/files?diff=split&w=0#diff-09d4e6f8f568b8ae1f60c465f4f2c89e3fb4e92d92ac2308fae80036b2430a1dR967