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

feat: cda work queue #132

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: cda work queue #132

wants to merge 3 commits into from

Conversation

jbutler
Copy link
Contributor

@jbutler jbutler commented Sep 21, 2022

Issue #, if available:

Description of changes:
[WIP] Introduces work queue as infrastructure. This can be injected to other classes and used to queue async jobs

Why is this change necessary:
This simplifies queuing of work that must happen in another thread context. It also enables scheduling jobs in the future.

How was this change tested:

Any additional information or context required to review the change:

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

@@ -156,6 +157,8 @@ void GIVEN_brokerWithValidCredentials_WHEN_GetClientDeviceAuthToken_THEN_returns
}
}

// TODO: Re-enable once infrastructure supports limiting queue size
@Disabled
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to sort out how this should work moving forward

@jbutler jbutler force-pushed the work-queue branch 2 times, most recently from ea84697 to 3e7c8b3 Compare September 22, 2022 02:34
@github-actions
Copy link

github-actions bot commented Sep 22, 2022

Unit Tests Coverage Report

File Coverage Lines Branches
All files 69% 77% 61%
com.aws.greengrass.clientdevices.auth.infra.NetworkState 36% 36% 0%
com.aws.greengrass.clientdevices.auth.infra.NetworkState$ConnectionState 100% 100% 0%
com.aws.greengrass.clientdevices.auth.infra.CDAExecutor 82% 88% 75%
com.aws.greengrass.clientdevices.auth.infra.NetworkState$1 20% 20% 0%
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils 78% 82% 74%
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils$Operation 100% 100% 0%
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils$Resource 100% 100% 0%
com.aws.greengrass.clientdevices.auth.CertificateManager 76% 90% 63%
com.aws.greengrass.clientdevices.auth.ClientDevicesAuthService 85% 94% 75%
com.aws.greengrass.clientdevices.auth.DeviceAuthClient 70% 76% 64%
com.aws.greengrass.clientdevices.auth.iot.registry.ThingRegistry 100% 100% 100%
com.aws.greengrass.clientdevices.auth.iot.registry.RegistryConfig 0% 0% 0%
com.aws.greengrass.clientdevices.auth.iot.registry.CertificateRegistry$1 75% 100% 50%
com.aws.greengrass.clientdevices.auth.iot.registry.CertificateRegistry 82% 89% 75%
com.aws.greengrass.clientdevices.auth.iot.registry.ThingRegistry$1 75% 100% 50%
com.aws.greengrass.clientdevices.auth.certificate.ClientCertificateGenerator 96% 92% 100%
com.aws.greengrass.clientdevices.auth.certificate.CertificateHelper 77% 99% 55%
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore 74% 84% 64%
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor 77% 88% 67%
com.aws.greengrass.clientdevices.auth.certificate.ServerCertificateGenerator 95% 90% 100%
com.aws.greengrass.clientdevices.auth.certificate.CertificateGenerator 70% 90% 50%
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore$CAType 100% 100% 0%
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor$CertRotationDecider 90% 100% 80%
com.aws.greengrass.clientdevices.auth.certificate.CertificatesConfig 100% 100% 100%
com.aws.greengrass.ipc.IPCUtils 58% 67% 50%
com.aws.greengrass.ipc.VerifyClientDeviceIdentityOperationHandler 64% 79% 50%
com.aws.greengrass.ipc.GetClientDeviceAuthTokenOperationHandler 87% 98% 75%
com.aws.greengrass.ipc.AuthorizeClientDeviceActionOperationHandler 80% 93% 67%
com.aws.greengrass.ipc.SubscribeToCertificateUpdatesOperationHandler 77% 87% 67%
com.aws.greengrass.clientdevices.auth.session.SessionConfig 92% 100% 83%
com.aws.greengrass.clientdevices.auth.session.SessionManager$1 100% 100% 100%
com.aws.greengrass.clientdevices.auth.session.MqttSessionFactory 100% 100% 100%
com.aws.greengrass.clientdevices.auth.session.SessionCreator 100% 100% 100%
com.aws.greengrass.clientdevices.auth.session.SessionManager 88% 100% 75%
com.aws.greengrass.clientdevices.auth.session.SessionImpl 94% 89% 100%
com.aws.greengrass.clientdevices.auth.session.SessionCreator$SessionFactorySingleton 100% 100% 0%
com.aws.greengrass.clientdevices.auth.session.MqttSessionFactory$MqttCredential 100% 100% 0%
com.aws.greengrass.clientdevices.auth.certificate.handlers.CACertificateChainChangedHandler 100% 100% 0%
com.aws.greengrass.clientdevices.auth.certificate.handlers.CAConfigurationChangedHandler 100% 100% 100%
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureManagedCertificateAuthority 77% 77% 0%
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureCustomCertificateAuthority 75% 75% 0%
com.aws.greengrass.clientdevices.auth.certificate.usecases.RegisterCertificateAuthorityUseCase 59% 68% 50%
com.aws.greengrass.clientdevices.auth.configuration.AuthorizationPolicyStatement$Effect 100% 100% 0%
com.aws.greengrass.clientdevices.auth.configuration.InfrastructureConfiguration 100% 100% 0%
com.aws.greengrass.clientdevices.auth.configuration.GroupManager 89% 94% 83%
com.aws.greengrass.clientdevices.auth.configuration.ConfigurationFormatVersion 100% 100% 0%
com.aws.greengrass.clientdevices.auth.configuration.CAConfiguration 97% 100% 94%
com.aws.greengrass.clientdevices.auth.configuration.RuntimeConfiguration 100% 100% 0%
com.aws.greengrass.clientdevices.auth.configuration.CDAConfiguration 95% 100% 90%
com.aws.greengrass.clientdevices.auth.configuration.GroupDefinition 75% 100% 50%
com.aws.greengrass.clientdevices.auth.configuration.ExpressionVisitor 84% 94% 75%
com.aws.greengrass.clientdevices.auth.configuration.GroupConfiguration 90% 95% 86%
com.aws.greengrass.clientdevices.auth.api.ClientDevicesAuthServiceApi 82% 65% 100%
com.aws.greengrass.clientdevices.auth.api.Result 54% 83% 25%
com.aws.greengrass.clientdevices.auth.api.DomainEvents 100% 100% 100%
com.aws.greengrass.clientdevices.auth.api.UseCases 69% 88% 50%
com.aws.greengrass.clientdevices.auth.api.DomainEvent 0% 0% 0%
com.aws.greengrass.clientdevices.auth.api.GetCertificateRequestOptions$CertificateType 100% 100% 0%
com.aws.greengrass.clientdevices.auth.api.Result$Status 100% 100% 0%
com.aws.greengrass.clientdevices.auth.connectivity.usecases.GetConnectivityInformationUseCase 100% 100% 0%
com.aws.greengrass.clientdevices.auth.connectivity.usecases.RecordConnectivityChangesUseCase 100% 100% 100%
com.aws.greengrass.clientdevices.auth.session.attribute.StringLiteralAttribute 100% 100% 0%
com.aws.greengrass.clientdevices.auth.session.attribute.WildcardSuffixAttribute 88% 100% 75%
com.aws.greengrass.clientdevices.auth.util.ResizableLinkedBlockingQueue 50% 50% 0%
com.aws.greengrass.clientdevices.auth.util.ParseIPAddress 90% 95% 84%
com.aws.greengrass.clientdevices.auth.certificate.events.CACertificateChainChanged 100% 100% 0%
com.aws.greengrass.clientdevices.auth.iot.IotAuthClient$Default 83% 86% 80%
com.aws.greengrass.clientdevices.auth.iot.Thing 100% 100% 100%
com.aws.greengrass.clientdevices.auth.iot.Certificate 100% 100% 0%
com.aws.greengrass.clientdevices.auth.iot.Component 67% 67% 0%
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor 61% 77% 46%
com.aws.greengrass.clientdevices.auth.connectivity.RecordConnectivityChangesResponse 100% 100% 100%
com.aws.greengrass.clientdevices.auth.connectivity.HostAddress 67% 67% 0%
com.aws.greengrass.clientdevices.auth.connectivity.RecordConnectivityChangesRequest 100% 100% 0%
com.aws.greengrass.clientdevices.auth.connectivity.ConnectivityInformation 100% 100% 100%
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionConstants 100% 100% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.TokenMgrError 22% 32% 12%
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTokenManager 61% 65% 58%
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTStart 33% 33% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTAnd 67% 67% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.Token 58% 58% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionDefaultVisitor 0% 0% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTOr 67% 67% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleCharStream 28% 31% 25%
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTreeConstants 0% 0% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.JJTRuleExpressionState 67% 65% 70%
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTThing 67% 67% 0%
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpression 63% 63% 62%
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleNode 27% 35% 19%

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against d264ae1

@jbutler jbutler changed the title feat: cda work queue (DNM) feat: cda work queue Sep 22, 2022
@jbutler jbutler marked this pull request as ready for review September 22, 2022 02:59
public final class InfrastructureConfiguration {
public static final int DEFAULT_WORK_QUEUE_DEPTH = 100;
public static final int DEFAULT_THREAD_POOL_SIZE = 1;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this configuration - might make sense to re-organize a bit. But for now, we'll keep it how it is.

Note: I didn't want to change the existing SessionConfig, so this performance key is split between the two config classes. I don't love it, but I also wanted to keep this PR as contained as possible.

public void shutdown() {
executor.shutdown();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll like add methods for scheduling tasks in the future. We'll need to replace the queue with a delay queue for that. That can come later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant