aws-greengrass / aws-greengrass-shadow-manager

A GreengrassV2 Component that provides offline device shadow documents and optional synchronization to the IoT device shadow service.
Apache License 2.0
9 stars 5 forks source link

fix: wait for executing sync request to finish before cancelling threads #113

Closed rbattle closed 2 years ago

rbattle commented 2 years ago

Issue #, if available:

Description of changes: Cancelling sync threads can cause database exceptions if an I/O operation is occurring when the thread is cancelled.

A semaphore is used along with the existing flag to force the stopper to wait until the request is finished.

SyncStrategy common logic was refactored into the BaseSyncStrategy class to avoid duplication.

The syncLoop method from the PeriodicSyncStrategy class was pulled up to the BaseSyncStrategy. Periodic still uses the non-blocking poll and RealTime still uses the blocking take to get data off the queue. A permit on the semaphore is acquired before executing the request and there are checks for whether syncing has stopped. A latch is used to wait for threads to exit.

Extra test cases have been added to cover this change, and an integration test that turns syncing on and off repeatedly for 30 seconds has been added.

Why is this change necessary: To avoid H2 database exceptions and other Interrupted exceptions when syncing gets stopped while a request is processing.

How was this change tested: Unit/integration

Any additional information or context required to review the change:

Checklist:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging d93f9ba9b2419c6d8e2bf66f0f78274413b1ab78 into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 3d14cedccc03bb0bf2aca3abc5fc2f10c2bc31c6 into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 589683c440b1d917f59471f0e14cb42f82d74b00 into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

github-actions[bot] commented 2 years ago

Unit Tests Coverage Report

File Coverage Lines Branches
All files 87% 91% 83% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalUpdateSyncRequest 84% 88% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalDeleteSyncRequest 99% 98% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.FullShadowSyncRequest 84% 97% 71% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.CloudDeleteSyncRequest 80% 78% 83% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.BaseSyncRequest 78% 55% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.CloudUpdateSyncRequest 74% 76% 71% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.Operation 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.PubSubRequest 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.PeriodicSyncStrategy 76% 76% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.SyncStrategyFactory 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.BaseSyncStrategy 87% 88% 86% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.RealTimeSyncStrategy 88% 75% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientFactory 87% 100% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestBlockingQueue 96% 99% 93% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.SyncHandler 77% 94% 61% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientWrapper 91% 91% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestMerger 74% 71% 77% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.CloudDataClient 75% 76% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ShadowSyncConfiguration 87% 93% 82% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ThingShadowSyncConfiguration 76% 78% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDatabase 0% 0% 0% :x:
com.aws.greengrass.shadowmanager.ShadowManager 80% 84% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDAOImpl 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.AuthorizationHandlerWrapper 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManager$1 50% 50% 0% :x:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractDeleteThingShadowOperationHandler 67% 67% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractListNamedShadowsForThingOperationHandler 67% 67% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractUpdateThingShadowOperationHandler 67% 67% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractGetThingShadowOperationHandler 67% 67% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.StrategyType 86% 92% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.Strategy 90% 100% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonMerger 94% 100% 89% :white_check_mark:
com.aws.greengrass.shadowmanager.util.SyncNodeMerger 91% 98% 84% :white_check_mark:
com.aws.greengrass.shadowmanager.util.DataOwner 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.ShadowWriteSynchronizeHelper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.Validator 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonUtil 96% 100% 92% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ResponseMessageBuilder 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowDocument 81% 89% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowStateMetadata 93% 98% 89% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ErrorMessage 75% 100% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowRequest 86% 71% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowState 84% 87% 81% :white_check_mark:
com.aws.greengrass.shadowmanager.model.LogEvents 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.BaseRequestHandler 77% 77% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowRequestHandler 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.PubSubClientWrapper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.IpcRateLimiter 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowIPCHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter$1 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowRequestHandler 99% 99% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowRequestHandler 91% 95% 88% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowIPCHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowIPCHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.ListNamedShadowsForThingIPCHandler 100% 100% 100% :white_check_mark:

Minimum allowed coverage is 65%

Generated by :monkey: cobertura-action against 1068a3b2dde6e0d2579aee41529e60e29ee3174e

github-actions[bot] commented 2 years ago

Integration Tests Coverage Report

File Coverage Lines Branches
All files 63% 69% 56% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalUpdateSyncRequest 34% 47% 20% :x:
com.aws.greengrass.shadowmanager.sync.model.LocalDeleteSyncRequest 55% 59% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.FullShadowSyncRequest 39% 43% 36% :x:
com.aws.greengrass.shadowmanager.sync.model.CloudDeleteSyncRequest 53% 55% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.BaseSyncRequest 78% 55% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.CloudUpdateSyncRequest 51% 51% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.Operation 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.PubSubRequest 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.PeriodicSyncStrategy 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.SyncStrategyFactory 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.BaseSyncStrategy 78% 75% 82% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.RealTimeSyncStrategy 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientFactory 85% 100% 69% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestBlockingQueue 59% 69% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.SyncHandler 82% 92% 72% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientWrapper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestMerger 19% 29% 10% :x:
com.aws.greengrass.shadowmanager.sync.CloudDataClient 51% 48% 54% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ShadowSyncConfiguration 70% 79% 61% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ThingShadowSyncConfiguration 76% 78% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDatabase 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManager 87% 87% 86% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDAOImpl 84% 90% 77% :white_check_mark:
com.aws.greengrass.shadowmanager.AuthorizationHandlerWrapper 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManager$1 17% 17% 0% :x:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractDeleteThingShadowOperationHandler 100% 100% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractListNamedShadowsForThingOperationHandler 100% 100% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractUpdateThingShadowOperationHandler 100% 100% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractGetThingShadowOperationHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.StrategyType 55% 69% 40% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.Strategy 90% 100% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonMerger 34% 38% 31% :x:
com.aws.greengrass.shadowmanager.util.SyncNodeMerger 0% 0% 0% :x:
com.aws.greengrass.shadowmanager.util.DataOwner 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.ShadowWriteSynchronizeHelper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.Validator 62% 64% 59% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonUtil 80% 86% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ResponseMessageBuilder 91% 91% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowDocument 81% 89% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowStateMetadata 81% 86% 77% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ErrorMessage 97% 94% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowRequest 86% 71% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowState 94% 98% 91% :white_check_mark:
com.aws.greengrass.shadowmanager.model.LogEvents 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.BaseRequestHandler 77% 77% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowRequestHandler 87% 74% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.PubSubClientWrapper 80% 80% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.IpcRateLimiter 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowIPCHandler 55% 55% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter 94% 100% 88% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter$1 75% 100% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowRequestHandler 86% 72% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowRequestHandler 71% 74% 69% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowIPCHandler 53% 53% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowIPCHandler 95% 95% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.ListNamedShadowsForThingIPCHandler 61% 55% 67% :white_check_mark:

Minimum allowed coverage is 45%

Generated by :monkey: cobertura-action against 1068a3b2dde6e0d2579aee41529e60e29ee3174e

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging e34da22e98cabff6b101e8e13409495381ac77d4 into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 10021afca0f9cc9761c7a326fd3cf1d0c60d4dd7 into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging d8d311e8e248c361a462299833ae658dc9741663 into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 3010e6e94b65956a2778bd934529780fcdeee5fb into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging c3c0a2104daaa14d8af8b402e6fdc354c16740da into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 1e1147d06bb1307f9cf829ffc65add3f4e87fb8d into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 1068a3b2dde6e0d2579aee41529e60e29ee3174e into a6c95d3a667b6955f41dce96fe0e2303df7b6585 - view on LGTM.com

fixed alerts: