Skip to content
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

subscriptionsPerFabric in basic cluster has incorrect values #19535

Closed
mrjerryjohns opened this issue Jun 13, 2022 · 4 comments · Fixed by #20207
Closed

subscriptionsPerFabric in basic cluster has incorrect values #19535

mrjerryjohns opened this issue Jun 13, 2022 · 4 comments · Fixed by #20207
Assignees
Labels
cert blocker Interaction Model Work p1 priority 1 work spec Mismatch between spec and implementation V1.0

Comments

@mrjerryjohns
Copy link
Contributor

Problem

Reading out subscriptionsPerFabric gives a really large number (which might be accurate on heap-based platforms anyways), but looked suspicious.

Looks like the logic is off in InteractionModelEngine::GetMinSubscriptionsPerFabric():

    size_t perFabricSubscriptionCapacity = static_cast<size_t>(readHandlerPoolCapacity - kReservedPathsForReads) / fabricCount;

That should be kReservedHandlersForRead, not paths.

Also, GetReadHandlerPoolCapacity is not quite handling heap-based platforms today, resulting in an advertised number lower than the capability of the platform.

@mrjerryjohns
Copy link
Contributor Author

Last of all, the computation of the value is dividing the total available resources by the number of currently commissioned fabrics. This is not compliant with what the spec is actually saying here (which is requiring the minimum number of subs supported per fabric assuming at least 5 fabrics could be commissioned on the device).

@mrjerryjohns mrjerryjohns added the spec Mismatch between spec and implementation label Jun 21, 2022
@bzbarsky-apple
Copy link
Contributor

So we need two functions: the bare minimum we guarantee per fabric (which depends on max fabrics), and the value the resource-allocation logic needs, which is depends on our actual number of fabrics.

@erjiaqing
Copy link
Contributor

Note: Since #18884 touched the macros for resource minimals, the original issue is fixed already.

@mrjerryjohns mrjerryjohns added the p1 priority 1 work label Jun 23, 2022
@cjandhyala
Copy link
Contributor

fixed in the latest SDK(c60233d) , verified on RPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cert blocker Interaction Model Work p1 priority 1 work spec Mismatch between spec and implementation V1.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants