[IM] Update read transaction resource minimas

src/app/InteractionModelEngine.h

@@ -87,10 +87,11 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
      * Spec 8.5.1 A publisher SHALL always ensure that every fabric the node is commissioned into can create at least three
      * subscriptions to the publisher and that each subscription SHALL support at least 3 attribute/event paths.
      */
-    static constexpr size_t kMinSupportedSubscriptionsPerFabric           = 2;
-    static constexpr size_t kMinSupportedPathsPerSubscription             = 2;
-    static constexpr size_t kReservedPathsPerReadRequest                  = 9;
-    static constexpr size_t kReservedReadHandlersPerFabricForReadRequests = 1;
+    static constexpr size_t kMinSupportedSubscriptionsPerFabric = 2;
+    static constexpr size_t kMinSupportedPathsPerSubscription   = 2;
+    static constexpr size_t kMinSupportedPathsPerReadRequest    = 9;
+    static constexpr size_t kMinSupportedReadRequestsPerFabric  = 1;
+    static constexpr size_t kReadHandlerPoolSize                = CHIP_IM_MAX_NUM_SUBSCRIPTIONS + CHIP_IM_MAX_NUM_READS;
 
     // TODO: Per spec, the above numbers should be 3, 3, 9, 1, however, we use a lower limit to reduce the memory usage and should
     // fix it when we have reduced the memory footprint of ReadHandlers.
@@ -242,29 +243,25 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
      */
     bool HasConflictWriteRequests(const WriteHandler * apWriteHandler, const ConcreteAttributePath & aPath);
 
-    /**
-     * We only allow one active read transaction per fabric, and the number of paths used is limited by
-     * kReservedPathsPerReadRequest. This function will check if the given ReadHandler will exceed the limitations for the accessing
-     * fabric.
-     *
-     * If CHIP_NO_ERROR is returned, it's OK to proceed with the read.
-     *
-     * Otherwise the CHIP_ERROR encodes an interaction model status that needs
-     * to turn into a Status Response to the client.
-     *
-     * TODO: (#17418) We are now reserving resources for read requests, could be changed to similar algorithm for read resources
-     * minimas.
-     */
-    CHIP_ERROR CanEstablishReadTransaction(const ReadHandler * apReadHandler);
-
     /**
      * Select the oldest (and the one that exceeds the per subscription resource minimum if there are any) read handler on the
      * fabric with the given fabric index. Evict it when the fabric uses more resources than the per fabric quota or aForceEvict is
      * true.
      *
      * @retval Whether we have evicted a subscription.
      */
-    bool TrimFabric(FabricIndex aFabricIndex, bool aForceEvict);
+    bool TrimFabricForSubscriptions(FabricIndex aFabricIndex, bool aForceEvict);
+
+    /**
+     * Select a read handler and abort the read transaction if the fabric is using more resources (number of paths or number of read
+     * handlers) then we guaranteed.
+     *
+     * - The youngest oversized read handlers will be chosen first.
+     * - If there are no oversized read handlers, the youngest read handlers will be chosen.
+     *
+     * @retval Whether we have evicted a read transaction.
+     */
+    bool TrimFabricForRead(FabricIndex aFabricIndex);
 
     uint16_t GetMinSubscriptionsPerFabric() const;
 
@@ -275,48 +272,30 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     auto & GetReadHandlerPool() { return mReadHandlers; }
 
     //
-    // Override the maximal capacity of the underlying read handler pool to mimic
-    // out of memory scenarios in unit-tests.
-    //
-    // This function did not considered the resources reserved for read handlers,
-    // SetHandlerCapacityForSubscriptions if there are subscriptions in the tests.
+    // Override the maximal capacity of the fabric table only for interaction model engine
     //
     // If -1 is passed in, no override is instituted and default behavior resumes.
     //
-    void SetHandlerCapacity(int32_t sz) { mReadHandlerCapacityOverride = sz; }
-
-    //
-    // Override the maximal capacity of the underlying attribute path pool and event path pool to mimic
-    // out of paths exhausted scenarios in unit-tests.
-    //
-    // This function did not considered the resources reserved for read handlers,
-    // SetPathPoolCapacityForSubscriptions if there are subscriptions in the tests.
-    //
-    // If -1 is passed in, no override is instituted and default behavior resumes.
-    //
-    void SetPathPoolCapacity(int32_t sz) { mPathPoolCapacityOverride = sz; }
+    void SetConfigMaxFabrics(int32_t sz) { mMaxNumFabricsOverride = sz; }
 
     //
     // Override the maximal capacity of the underlying read handler pool to mimic
-    // out of memory scenarios in unit-tests.
+    // out of memory scenarios in unit-tests. You need to SetConfigMaxFabrics to make GetGuaranteedReadRequestsPerFabric
+    // working correctly.
     //
     // If -1 is passed in, no override is instituted and default behavior resumes.
     //
-    void SetHandlerCapacityForSubscriptions(int32_t sz)
-    {
-        SetHandlerCapacity(sz == -1 ? -1 : sz + static_cast<int32_t>(kReservedHandlersForReads));
-    }
+    void SetHandlerCapacityForReads(int32_t sz) { mReadHandlerCapacityForReadsOverride = sz; }
+    void SetHandlerCapacityForSubscriptions(int32_t sz) { mReadHandlerCapacityForSubscriptionsOverride = sz; }
 
     //
     // Override the maximal capacity of the underlying attribute path pool and event path pool to mimic
     // out of paths exhausted scenarios in unit-tests.
     //
     // If -1 is passed in, no override is instituted and default behavior resumes.
     //
-    void SetPathPoolCapacityForSubscriptions(int32_t sz)
-    {
-        SetPathPoolCapacity(sz == -1 ? -1 : sz + static_cast<int32_t>(kReservedPathsForReads));
-    }
+    void SetPathPoolCapacityForReads(int32_t sz) { mPathPoolCapacityForReadsOverride = sz; }
+    void SetPathPoolCapacityForSubscriptions(int32_t sz) { mPathPoolCapacityForSubscriptionsOverride = sz; }
 
     //
     // We won't limit the handler used per fabric on platforms that are using heap for memory pools, so we introduces a flag to
@@ -416,26 +395,61 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     CHIP_ERROR ShutdownExistingSubscriptionsIfNeeded(Messaging::ExchangeContext * apExchangeContext,
                                                      System::PacketBufferHandle && aPayload);
 
-    inline size_t GetPathPoolCapacity() const
+    inline size_t GetPathPoolCapacityForReads() const
     {
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
-        return (mPathPoolCapacityOverride == -1) ? CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS
-                                                 : static_cast<size_t>(mPathPoolCapacityOverride);
+        return (mPathPoolCapacityForReadsOverride == -1) ? CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS
+                                                         : static_cast<size_t>(mPathPoolCapacityForReadsOverride);
 #else
-        return CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS;
+        return CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS;
 #endif
     }
 
-    inline size_t GetReadHandlerPoolCapacity() const
+    inline size_t GetReadHandlerPoolCapacityForReads() const
     {
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
-        return (mReadHandlerCapacityOverride == -1) ? CHIP_IM_MAX_NUM_READ_HANDLER
-                                                    : static_cast<size_t>(mReadHandlerCapacityOverride);
+        return (mReadHandlerCapacityForReadsOverride == -1) ? CHIP_IM_MAX_NUM_READS
+                                                            : static_cast<size_t>(mReadHandlerCapacityForReadsOverride);
 #else
-        return CHIP_IM_MAX_NUM_READ_HANDLER;
+        return CHIP_IM_MAX_NUM_READS;
 #endif
     }
 
+    inline size_t GetPathPoolCapacityForSubscriptions() const
+    {
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+        return (mPathPoolCapacityForSubscriptionsOverride == -1) ? CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS
+                                                                 : static_cast<size_t>(mPathPoolCapacityForSubscriptionsOverride);
+#else
+        return CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS;
+#endif
+    }
+
+    inline size_t GetReadHandlerPoolCapacityForSubscriptions() const
+    {
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+        return (mReadHandlerCapacityForSubscriptionsOverride == -1)
+            ? CHIP_IM_MAX_NUM_SUBSCRIPTIONS
+            : static_cast<size_t>(mReadHandlerCapacityForSubscriptionsOverride);
+#else
+        return CHIP_IM_MAX_NUM_SUBSCRIPTIONS;
+#endif
+    }
+
+    inline uint8_t GetConfigMaxFabrics() const
+    {
+#if CONFIG_IM_BUILD_FOR_UNIT_TEST
+        return (mMaxNumFabricsOverride == -1) ? CHIP_CONFIG_MAX_FABRICS : static_cast<uint8_t>(mMaxNumFabricsOverride);
+#else
+        return CHIP_CONFIG_MAX_FABRICS;
+#endif
+    }
+
+    inline size_t GetGuaranteedReadRequestsPerFabric() const
+    {
+        return GetReadHandlerPoolCapacityForReads() / GetConfigMaxFabrics();
+    }
+
     /**
      * Verify and ensure (by killing oldest read handlers that make the resources used by the current fabric exceed the fabric
      * quota)
@@ -451,6 +465,29 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     bool EnsureResourceForSubscription(FabricIndex aFabricIndex, size_t aRequestedAttributePathCount,
                                        size_t aRequestedEventPathCount);
 
+    /**
+     * Verify and ensure (by killing oldest read handlers that make the resources used by the current fabric exceed the fabric
+     * quota) the resources for handling a new read transaction with the given resource requirments.
+     * - PASE sessions will be counted in a virtual fabric (i.e. kInvalidFabricIndex will be consided as a "valid" fabric in this
+     * function)
+     * - If the existing resources can serve this read transaction, this function will return Status::Success.
+     * - or if the resources used by read transactions in the fabric index meets the per fabric resource limit (i.e. 9 paths & 1
+     * read) after accepting this read request, this function will always return Status::Success by evicting existing read
+     * transactions from other fabrics which are using more than the guaranteed minimum number of read.
+     * - or if the resources used by read transactions in the fabric index will exceed the per fabric resource limit (i.e. 9 paths &
+     * 1 read) after accepting this read request, this function will return a failure status without evicting any existing
+     * transaction.
+     * - However, read transactions on PASE sessions won't evict any existing read transactions when we have already commissioned
+     * CHIP_CONFIG_MAX_FABRICS fabrics on the device.
+     *
+     * @retval Status::Success: The read transaction can be accepted.
+     * @retval Status::Busy: The remaining resource is insufficient to handle this read request, and the accessing fabric for this
+     * read request will use more resources than we guaranteed, the client is expected to retry later.
+     * @retval Status::PathsExhausted: The attribute / event path pool is exhausted, and the read request is requesting more
+     * resources than we guaranteed.
+     */
+    Status EnsureResourceForRead(FabricIndex aFabricIndex, size_t aRequestedAttributePathCount, size_t aRequestedEventPathCount);
+
     template <typename T, size_t N>
     void ReleasePool(ObjectList<T> *& aObjectList, ObjectPool<ObjectList<T>, N> & aObjectPool);
     template <typename T, size_t N>
@@ -465,31 +502,46 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER];
     reporting::Engine mReportingEngine;
 
-    static constexpr size_t kReservedHandlersForReads = kReservedReadHandlersPerFabricForReadRequests * (CHIP_CONFIG_MAX_FABRICS);
-    static constexpr size_t kReservedPathsForReads    = kReservedPathsPerReadRequest * kReservedHandlersForReads;
+    static constexpr size_t kReservedHandlersForReads = kMinSupportedReadRequestsPerFabric * (CHIP_CONFIG_MAX_FABRICS);
+    static constexpr size_t kReservedPathsForReads    = kMinSupportedPathsPerReadRequest * kReservedHandlersForReads;
 
 #if !CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
-    static_assert(CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS >= CHIP_CONFIG_MAX_FABRICS *
-                          (kMinSupportedPathsPerSubscription * kMinSupportedSubscriptionsPerFabric + kReservedPathsPerReadRequest),
-                  "CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS is too small to match the requirements of spec 8.5.1");
-    static_assert(CHIP_IM_MAX_NUM_READ_HANDLER >= CHIP_CONFIG_MAX_FABRICS *
-                          (kMinSupportedSubscriptionsPerFabric + kReservedReadHandlersPerFabricForReadRequests),
-                  "CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS is too small to match the requirements of spec 8.5.1");
+    static_assert(CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS >=
+                      CHIP_CONFIG_MAX_FABRICS * (kMinSupportedPathsPerSubscription * kMinSupportedSubscriptionsPerFabric),
+                  "CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS is too small to match the requirements of spec 8.5.1");
+    static_assert(CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS >=
+                      CHIP_CONFIG_MAX_FABRICS * (kMinSupportedReadRequestsPerFabric * kMinSupportedPathsPerReadRequest),
+                  "CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS is too small to match the requirements of spec 8.5.1");
+    static_assert(CHIP_IM_MAX_NUM_SUBSCRIPTIONS >= CHIP_CONFIG_MAX_FABRICS * kMinSupportedSubscriptionsPerFabric,
+                  "CHIP_IM_MAX_NUM_SUBSCRIPTIONS is too small to match the requirements of spec 8.5.1");
+    static_assert(CHIP_IM_MAX_NUM_READS >= CHIP_CONFIG_MAX_FABRICS * kMinSupportedReadRequestsPerFabric,
+                  "CHIP_IM_MAX_NUM_READS is too small to match the requirements of spec 8.5.1");
 #endif
 
-    ObjectPool<ObjectList<AttributePathParams>, CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS> mAttributePathPool;
-    ObjectPool<ObjectList<EventPathParams>, CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS> mEventPathPool;
-    ObjectPool<ObjectList<DataVersionFilter>, CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS> mDataVersionFilterPool;
+    ObjectPool<ObjectList<AttributePathParams>,
+               CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS + CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS>
+        mAttributePathPool;
+    ObjectPool<ObjectList<EventPathParams>,
+               CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS + CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS>
+        mEventPathPool;
+    ObjectPool<ObjectList<DataVersionFilter>,
+               CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_READS + CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS_FOR_SUBSCRIPTIONS>
+        mDataVersionFilterPool;
 
-    ObjectPool<ReadHandler, CHIP_IM_MAX_NUM_READ_HANDLER> mReadHandlers;
+    ObjectPool<ReadHandler, CHIP_IM_MAX_NUM_READS + CHIP_IM_MAX_NUM_SUBSCRIPTIONS> mReadHandlers;
 
     ReadClient * mpActiveReadClientList = nullptr;
 
     ReadHandler::ApplicationCallback * mpReadHandlerApplicationCallback = nullptr;
 
 #if CONFIG_IM_BUILD_FOR_UNIT_TEST
-    int mReadHandlerCapacityOverride = -1;
-    int mPathPoolCapacityOverride    = -1;
+    int mReadHandlerCapacityForSubscriptionsOverride = -1;
+    int mPathPoolCapacityForSubscriptionsOverride    = -1;
+
+    int mReadHandlerCapacityForReadsOverride = -1;
+    int mPathPoolCapacityForReadsOverride    = -1;
+
+    int mMaxNumFabricsOverride = -1;
 
     // We won't limit the handler used per fabric on platforms that are using heap for memory pools, so we introduces a flag to
     // enforce such check based on the configured size. This flag is used for unit tests only, there is another compare time flag

src/app/ReadHandler.cpp

@@ -41,10 +41,10 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon
                          InteractionType aInteractionType) :
     mManagementCallback(apCallback)
 {
-    mpExchangeCtx                = apExchangeContext;
-    mInteractionType             = aInteractionType;
-    mLastWrittenEventsBytes      = 0;
-    mSubscriptionStartGeneration = InteractionModelEngine::GetInstance()->GetReportingEngine().GetDirtySetGeneration();
+    mpExchangeCtx               = apExchangeContext;
+    mInteractionType            = aInteractionType;
+    mLastWrittenEventsBytes     = 0;
+    mTransactionStartGeneration = InteractionModelEngine::GetInstance()->GetReportingEngine().GetDirtySetGeneration();
     mFlags.ClearAll();
     mFlags.Set(ReadHandlerFlags::PrimingReports, true);
     if (apExchangeContext != nullptr)
@@ -385,10 +385,6 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa
     }
     ReturnErrorOnFailure(err);
 
-    // Ensure the read transaction doesn't exceed the resources dedicated to
-    // read transactions.
-    ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->CanEstablishReadTransaction(this));
-
     bool isFabricFiltered;
     ReturnErrorOnFailure(readRequestParser.GetIsFabricFiltered(&isFabricFiltered));
     mFlags.Set(ReadHandlerFlags::FabricFiltered, isFabricFiltered);

src/app/ReadHandler.h

@@ -315,7 +315,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
     Transport::SecureSession * GetSession() const;
     SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); }
 
-    auto GetSubscriptionStartGeneration() const { return mSubscriptionStartGeneration; }
+    auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; }
 
     void UnblockUrgentEventDelivery()
     {
@@ -431,7 +431,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
 
     // When we don't have enough resources for a new subscription, the oldest subscription might be evicted by interaction model
     // engine, the "oldest" subscription is the subscription with the smallest generation.
-    uint64_t mSubscriptionStartGeneration = 0;
+    uint64_t mTransactionStartGeneration = 0;
 
     SubscriptionId mSubscriptionId    = 0;
     uint16_t mMinIntervalFloorSeconds = 0;