Skip to content

Commit

Permalink
[Darwin] MTRDevice should throttle writes to the attribute storage (#…
Browse files Browse the repository at this point in the history
…33535)

* [Darwin] MTRDevice should throttle writes to the attribute storage

* Tune unit test timing to avoid race condition that fails the test when the machine is slow

* Additional tuning of unit test timing to avoid timing conditions that fails the test when the machine is slow

* Tune base delay to 3 seconds, to account for slow testing machines

* Fix unit test

* Unit test timing fix and remove debug logging

* Fix TSAN issue with test values in unit test
  • Loading branch information
jtung-apple authored and pull[bot] committed Jun 25, 2024
1 parent 9df8f5f commit 1125342
Show file tree
Hide file tree
Showing 16 changed files with 929 additions and 52 deletions.
313 changes: 286 additions & 27 deletions src/darwin/Framework/CHIP/MTRDevice.mm

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ @implementation MTRDeviceController {

// _serverEndpoints is only touched on the Matter queue.
NSMutableArray<MTRServerEndpoint *> * _serverEndpoints;

MTRDeviceStorageBehaviorConfiguration * _storageBehaviorConfiguration;
}

- (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters error:(NSError * __autoreleasing *)error
Expand All @@ -153,6 +155,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
uniqueIdentifier:(NSUUID *)uniqueIdentifier
concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration
{
if (self = [super init]) {
// Make sure our storage is all set up to work as early as possible,
Expand Down Expand Up @@ -274,6 +277,8 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
_concurrentSubscriptionPool = [[MTRAsyncWorkQueue alloc] initWithContext:self width:concurrentSubscriptionPoolSize];

_storedFabricIndex = chip::kUndefinedFabricIndex;

_storageBehaviorConfiguration = storageBehaviorConfiguration;
}
return self;
}
Expand Down Expand Up @@ -989,6 +994,8 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
}
}

[deviceToReturn setStorageBehaviorConfiguration:_storageBehaviorConfiguration];

return deviceToReturn;
}

Expand Down
5 changes: 4 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
id<MTROTAProviderDelegate> _Nullable otaProviderDelegate;
dispatch_queue_t _Nullable otaProviderDelegateQueue;
NSUInteger concurrentSubscriptionPoolSize = 0;
MTRDeviceStorageBehaviorConfiguration * storageBehaviorConfiguration = nil;
if ([startupParams isKindOfClass:[MTRDeviceControllerParameters class]]) {
MTRDeviceControllerParameters * params = startupParams;
storageDelegate = params.storageDelegate;
Expand All @@ -481,6 +482,7 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
otaProviderDelegate = params.otaProviderDelegate;
otaProviderDelegateQueue = params.otaProviderDelegateQueue;
concurrentSubscriptionPoolSize = params.concurrentSubscriptionEstablishmentsAllowedOnThread;
storageBehaviorConfiguration = params.storageBehaviorConfiguration;
} else if ([startupParams isKindOfClass:[MTRDeviceControllerStartupParams class]]) {
MTRDeviceControllerStartupParams * params = startupParams;
storageDelegate = nil;
Expand Down Expand Up @@ -542,7 +544,8 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
otaProviderDelegate:otaProviderDelegate
otaProviderDelegateQueue:otaProviderDelegateQueue
uniqueIdentifier:uniqueIdentifier
concurrentSubscriptionPoolSize:concurrentSubscriptionPoolSize];
concurrentSubscriptionPoolSize:concurrentSubscriptionPoolSize
storageBehaviorConfiguration:storageBehaviorConfiguration];
if (controller == nil) {
if (error != nil) {
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
Expand Down
8 changes: 8 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import <Matter/MTRDefines.h>

#import <Matter/MTRDeviceControllerStorageDelegate.h>
#import <Matter/MTRDeviceStorageBehaviorConfiguration.h>
#import <Matter/MTROTAProviderDelegate.h>

NS_ASSUME_NONNULL_BEGIN
Expand Down Expand Up @@ -85,6 +86,13 @@ MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))
*/
@property (nonatomic, assign) NSUInteger concurrentSubscriptionEstablishmentsAllowedOnThread MTR_NEWLY_AVAILABLE;

/**
* Sets the storage behavior configuration - see MTRDeviceStorageBehaviorConfiguration.h for details
*
* If this value is nil, a default storage behavior configuration will be used.
*/
@property (nonatomic, copy, nullable) MTRDeviceStorageBehaviorConfiguration * storageBehaviorConfiguration;

@end

MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))
Expand Down
4 changes: 3 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#import "MTRBaseDevice.h"
#import "MTRDeviceController.h"
#import "MTRDeviceControllerDataStore.h"
#import "MTRDeviceStorageBehaviorConfiguration.h"

#import <Matter/MTRDefines.h>
#import <Matter/MTRDeviceControllerStartupParams.h>
Expand Down Expand Up @@ -113,7 +114,8 @@ NS_ASSUME_NONNULL_BEGIN
otaProviderDelegate:(id<MTROTAProviderDelegate> _Nullable)otaProviderDelegate
otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
uniqueIdentifier:(NSUUID *)uniqueIdentifier
concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize;
concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;

/**
* Check whether this controller is running on the given fabric, as represented
Expand Down
98 changes: 98 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceStorageBehaviorConfiguration.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import <Foundation/Foundation.h>
#import <Matter/MTRDefines.h>

NS_ASSUME_NONNULL_BEGIN

/**
* Class that configures how MTRDevice objects persist its attributes to storage, so as to not
* overwhelm the underlying storage system.
*/
MTR_NEWLY_AVAILABLE
@interface MTRDeviceStorageBehaviorConfiguration : NSObject <NSCopying>

/**
* Create configuration with a default set of values. See description below for details.
*/
+ (instancetype)configurationWithDefaultStorageBehavior;

/**
* Create configuration that disables storage behavior optimizations.
*/
+ (instancetype)configurationWithStorageBehaviorOptimizationDisabled;

/**
* Create configuration with specified values. See description below for details, and the list of
* properties below for valid ranges of these values.
*/
+ (instancetype)configurationWithReportToPersistenceDelayTime:(NSTimeInterval)reportToPersistenceDelayTime
reportToPersistenceDelayTimeMax:(NSTimeInterval)reportToPersistenceDelayTimeMax
recentReportTimesMaxCount:(NSUInteger)recentReportTimesMaxCount
timeBetweenReportsTooShortThreshold:(NSTimeInterval)timeBetweenReportsTooShortThreshold
timeBetweenReportsTooShortMinThreshold:(NSTimeInterval)timeBetweenReportsTooShortMinThreshold
reportToPersistenceDelayMaxMultiplier:(double)reportToPersistenceDelayMaxMultiplier
deviceReportingExcessivelyIntervalThreshold:(NSTimeInterval)deviceReportingExcessivelyIntervalThreshold;

/**
* Storage behavior with values in the allowed range:
*
* Each time a report comes in, MTRDevice will wait reportToPersistDelayTime before persisting the
* changes to storage. If another report comes in during this internal, MTRDevice will wait another
* reportToPersistDelayTime interval, until reportToPersistDelayTimeMax is reached, at which
* point all the changes so far will be written to storage.
*
* MTRDevice will also track recentReportTimesMaxCount number of report times. If the running
* average time between reports dips below timeBetweenReportsTooShortThreshold, a portion of the
* reportToPersistenceDelayMaxMultiplier will be applied to both the reportToPersistenceDelayTime
* and reportToPersistenceDelayTimeMax. The multiplier will reach the max when the average time
* between reports reach timeBetweenReportsTooShortMinThreshold.
*
* When the running average time between reports dips below timeBetweenReportsTooShortMinThreshold
* for the first time, the time will be noted. If the device remains in this state for longer than
* deviceReportingExcessivelyIntervalThreshold, persistence will stop until the average time between
* reports go back above timeBetweenReportsTooShortMinThreshold.
*/

/**
* If disableStorageBehaviorOptimization is set to YES, then all the waiting mechanism as described above
* is disabled.
*/
@property (nonatomic, assign) BOOL disableStorageBehaviorOptimization;

/**
* If any of these properties are set to be out of the documented limits, these default values will
* be used to replace all of them:
*
* reportToPersistenceDelayTimeDefault (15)
* reportToPersistenceDelayTimeMaxDefault (20 * kReportToPersistenceDelayTimeDefault)
* recentReportTimesMaxCountDefault (12)
* timeBetweenReportsTooShortThresholdDefault (15)
* timeBetweenReportsTooShortMinThresholdDefault (5)
* reportToPersistenceDelayMaxMultiplierDefault (10)
* deviceReportingExcessivelyIntervalThresholdDefault (5 * 60)
*/
@property (nonatomic, assign) NSTimeInterval reportToPersistenceDelayTime; /* must be > 0 */
@property (nonatomic, assign) NSTimeInterval reportToPersistenceDelayTimeMax; /* must be larger than reportToPersistenceDelayTime */
@property (nonatomic, assign) NSUInteger recentReportTimesMaxCount; /* must be >= 2 */
@property (nonatomic, assign) NSTimeInterval timeBetweenReportsTooShortThreshold; /* must be > 0 */
@property (nonatomic, assign) NSTimeInterval timeBetweenReportsTooShortMinThreshold; /* must be > 0 and smaller than timeBetweenReportsTooShortThreshold */
@property (nonatomic, assign) double reportToPersistenceDelayMaxMultiplier; /* must be > 1 */
@property (nonatomic, assign) NSTimeInterval deviceReportingExcessivelyIntervalThreshold; /* must be > 0 */
@end

NS_ASSUME_NONNULL_END
105 changes: 105 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceStorageBehaviorConfiguration.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import "MTRDeviceStorageBehaviorConfiguration.h"

#import "MTRLogging_Internal.h"

#define kReportToPersistenceDelayTimeDefault (15)
#define kReportToPersistenceDelayTimeMaxDefault (20 * kReportToPersistenceDelayTimeDefault)
#define kRecentReportTimesMaxCountDefault (12)
#define kTimeBetweenReportsTooShortThresholdDefault (15)
#define kTimeBetweenReportsTooShortMinThresholdDefault (5)
#define kReportToPersistenceDelayMaxMultiplierDefault (10)
#define kDeviceReportingExcessivelyIntervalThresholdDefault (5 * 60)

@implementation MTRDeviceStorageBehaviorConfiguration

+ (instancetype)configurationWithReportToPersistenceDelayTime:(NSTimeInterval)reportToPersistenceDelayTime
reportToPersistenceDelayTimeMax:(NSTimeInterval)reportToPersistenceDelayTimeMax
recentReportTimesMaxCount:(NSUInteger)recentReportTimesMaxCount
timeBetweenReportsTooShortThreshold:(NSTimeInterval)timeBetweenReportsTooShortThreshold
timeBetweenReportsTooShortMinThreshold:(NSTimeInterval)timeBetweenReportsTooShortMinThreshold
reportToPersistenceDelayMaxMultiplier:(double)reportToPersistenceDelayMaxMultiplier
deviceReportingExcessivelyIntervalThreshold:(NSTimeInterval)deviceReportingExcessivelyIntervalThreshold
{
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
newConfiguration.reportToPersistenceDelayTime = reportToPersistenceDelayTime;
newConfiguration.reportToPersistenceDelayTimeMax = reportToPersistenceDelayTimeMax;
newConfiguration.recentReportTimesMaxCount = recentReportTimesMaxCount;
newConfiguration.timeBetweenReportsTooShortThreshold = timeBetweenReportsTooShortThreshold;
newConfiguration.timeBetweenReportsTooShortMinThreshold = timeBetweenReportsTooShortMinThreshold;
newConfiguration.reportToPersistenceDelayMaxMultiplier = reportToPersistenceDelayMaxMultiplier;
newConfiguration.deviceReportingExcessivelyIntervalThreshold = deviceReportingExcessivelyIntervalThreshold;

return newConfiguration;
}

+ (instancetype)configurationWithDefaultStorageBehavior
{
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
[newConfiguration checkValuesAndResetToDefaultIfNecessary];
return newConfiguration;
}

+ (instancetype)configurationWithStorageBehaviorOptimizationDisabled
{
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
newConfiguration.disableStorageBehaviorOptimization = YES;
return newConfiguration;
}

- (NSString *)description
{
return [NSString stringWithFormat:@"<MTRDeviceStorageBehaviorConfiguration(%p): disabled %s reportToPersistenceDelayTime %lf reportToPersistenceDelayTimeMax %lf recentReportTimesMaxCount %lu timeBetweenReportsTooShortThreshold %lf timeBetweenReportsTooShortMinThreshold %lf reportToPersistenceDelayMaxMultiplier %lf deviceReportingExcessivelyIntervalThreshold %lf", self, _disableStorageBehaviorOptimization ? "YES" : "NO", _reportToPersistenceDelayTime, _reportToPersistenceDelayTimeMax, static_cast<unsigned long>(_recentReportTimesMaxCount), _timeBetweenReportsTooShortThreshold, _timeBetweenReportsTooShortMinThreshold, _reportToPersistenceDelayMaxMultiplier, _deviceReportingExcessivelyIntervalThreshold];
}

- (void)checkValuesAndResetToDefaultIfNecessary
{
if (_disableStorageBehaviorOptimization) {
return;
}

// Sanity check all the values, and if any is out of range, reset to default values
if ((_reportToPersistenceDelayTime <= 0) || (_reportToPersistenceDelayTimeMax <= 0) || (_reportToPersistenceDelayTimeMax < _reportToPersistenceDelayTime) || (_recentReportTimesMaxCount < 2) || (_timeBetweenReportsTooShortThreshold <= 0) || (_timeBetweenReportsTooShortMinThreshold <= 0) || (_timeBetweenReportsTooShortMinThreshold > _timeBetweenReportsTooShortThreshold) || (_reportToPersistenceDelayMaxMultiplier <= 1) || (_deviceReportingExcessivelyIntervalThreshold <= 0)) {
MTR_LOG_ERROR("%@ storage behavior: MTRDeviceStorageBehaviorConfiguration values out of bounds - resetting to default", self);

_reportToPersistenceDelayTime = kReportToPersistenceDelayTimeDefault;
_reportToPersistenceDelayTimeMax = kReportToPersistenceDelayTimeMaxDefault;
_recentReportTimesMaxCount = kRecentReportTimesMaxCountDefault;
_timeBetweenReportsTooShortThreshold = kTimeBetweenReportsTooShortThresholdDefault;
_timeBetweenReportsTooShortMinThreshold = kTimeBetweenReportsTooShortMinThresholdDefault;
_reportToPersistenceDelayMaxMultiplier = kReportToPersistenceDelayMaxMultiplierDefault;
_deviceReportingExcessivelyIntervalThreshold = kDeviceReportingExcessivelyIntervalThresholdDefault;
}
}

- (id)copyWithZone:(NSZone *)zone
{
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
newConfiguration.disableStorageBehaviorOptimization = _disableStorageBehaviorOptimization;
newConfiguration.reportToPersistenceDelayTime = _reportToPersistenceDelayTime;
newConfiguration.reportToPersistenceDelayTimeMax = _reportToPersistenceDelayTimeMax;
newConfiguration.recentReportTimesMaxCount = _recentReportTimesMaxCount;
newConfiguration.timeBetweenReportsTooShortThreshold = _timeBetweenReportsTooShortThreshold;
newConfiguration.timeBetweenReportsTooShortMinThreshold = _timeBetweenReportsTooShortMinThreshold;
newConfiguration.reportToPersistenceDelayMaxMultiplier = _reportToPersistenceDelayMaxMultiplier;
newConfiguration.deviceReportingExcessivelyIntervalThreshold = _deviceReportingExcessivelyIntervalThreshold;

return newConfiguration;
}

@end
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import "MTRDeviceStorageBehaviorConfiguration.h"

@interface MTRDeviceStorageBehaviorConfiguration ()
- (void)checkValuesAndResetToDefaultIfNecessary;
@end
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#import "MTRAsyncWorkQueue.h"
#import "MTRDefines_Internal.h"
#import "MTRDeviceStorageBehaviorConfiguration_Internal.h"

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -113,6 +114,8 @@ MTR_TESTABLE
- (NSUInteger)unitTestAttributeCount;
#endif

- (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;

@end

#pragma mark - Utility for clamping numbers
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/Matter.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#import <Matter/MTRDeviceControllerParameters.h>
#import <Matter/MTRDeviceControllerStartupParams.h>
#import <Matter/MTRDeviceControllerStorageDelegate.h>
#import <Matter/MTRDeviceStorageBehaviorConfiguration.h>
#import <Matter/MTRDeviceTypeRevision.h>
#import <Matter/MTRDiagnosticLogsType.h>
#import <Matter/MTRError.h>
Expand Down
Loading

0 comments on commit 1125342

Please sign in to comment.