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(rds): timeout and timeoutAction properties to ServerlessCluster #28534

Merged

Conversation

badmintoncryer
Copy link
Contributor

This pull request introduces two new properties to the ServerlessCluster class in the AWS CDK RDS package: secondsBeforeTimeout and timeoutAction.

The secondsBeforeTimeout property allows users to specify the amount of time that Aurora Serverless v1 will attempt to find a scaling point to perform seamless scaling before enforcing the timeout action. The default value is 300 seconds (5 minutes).

The timeoutAction property allows users to specify the action to take when the timeout is reached. Users can choose between ForceApplyCapacityChange, which will force the capacity to the specified value as soon as possible, even without a scaling point, and RollbackCapacityChange, which will ignore the capacity change if a scaling point is not found. The default behavior is RollbackCapacityChange.

These enhancements provide users with more control over the scaling behavior of their Aurora Serverless clusters.

Closes #27183


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team December 31, 2023 02:02
@github-actions github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK labels Dec 31, 2023
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@badmintoncryer badmintoncryer force-pushed the 27183-supportForScailingConfiguration branch from 5091b1f to e39be9c Compare December 31, 2023 02:05
@aws-cdk-automation aws-cdk-automation dismissed their stale review December 31, 2023 02:05

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@badmintoncryer badmintoncryer force-pushed the 27183-supportForScailingConfiguration branch from 7ff08ca to a3b6616 Compare January 2, 2024 11:31
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

some comments mostly on naming and typos. Everything else looks good @badmintoncryer :)

packages/aws-cdk-lib/aws-rds/lib/serverless-cluster.ts Outdated Show resolved Hide resolved
*
* @default - 300 (5 minutes)
*/
readonly secondsBeforeTimeout? : Duration;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
readonly secondsBeforeTimeout? : Duration;
readonly secondsBeforeTimeout?: Duration;

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this property needs to be changed to just timeout. Since we're providing a Duration here, a user could provide minutes if they wanted to. readonly timeout?: Duration makes the most sense

packages/aws-cdk-lib/aws-rds/lib/serverless-cluster.ts Outdated Show resolved Hide resolved
packages/aws-cdk-lib/aws-rds/lib/serverless-cluster.ts Outdated Show resolved Hide resolved
packages/aws-cdk-lib/aws-rds/README.md Outdated Show resolved Hide resolved
@mergify mergify bot dismissed kaizencc’s stale review January 2, 2024 15:46

Pull request has been modified.

@badmintoncryer badmintoncryer force-pushed the 27183-supportForScailingConfiguration branch from 1f2cd3b to 0dbf982 Compare January 2, 2024 16:11
@badmintoncryer
Copy link
Contributor Author

badmintoncryer commented Jan 2, 2024

@kaizencc Thank you for your review! I have changed the variable name to 'timeout' and accordingly updated the tests and README as well.

@badmintoncryer badmintoncryer changed the title feat(rds): Add secondsBeforeTimeout and timeoutAction properties to ServerlessCluster feat(rds): Add timeout and timeoutAction properties to ServerlessCluster Jan 2, 2024
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

Thanks @badmintoncryer. Looks good, just need to update the integ test to get the build succeeding.

@kaizencc kaizencc changed the title feat(rds): Add timeout and timeoutAction properties to ServerlessCluster feat(rds): timeout and timeoutAction properties to ServerlessCluster Jan 2, 2024
@badmintoncryer badmintoncryer force-pushed the 27183-supportForScailingConfiguration branch from 9034014 to e1234da Compare January 3, 2024 04:35
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 3, 2024
@badmintoncryer
Copy link
Contributor Author

@kaizencc I fixed integ test.

Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

A few review comments that I will try to merge to your branch and hopefully I didnt break the build

packages/aws-cdk-lib/aws-rds/lib/serverless-cluster.ts Outdated Show resolved Hide resolved
packages/aws-cdk-lib/aws-rds/lib/serverless-cluster.ts Outdated Show resolved Hide resolved
packages/aws-cdk-lib/aws-rds/lib/serverless-cluster.ts Outdated Show resolved Hide resolved
Comment on lines 957 to 963
ScalingConfiguration: {
AutoPause: true,
MaxCapacity: 32,
MinCapacity: 8,
SecondsUntilAutoPause: 600,
TimeoutAction: 'ForceApplyCapacityChange',
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Whatever timeout maps to is missing from this assertion

Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

If all goes well, then approved :)

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 3, 2024
Copy link
Contributor

mergify bot commented Jan 3, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 4bc9d4b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 508825b into aws:main Jan 3, 2024
9 checks passed
Copy link
Contributor

mergify bot commented Jan 3, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@badmintoncryer badmintoncryer deleted the 27183-supportForScailingConfiguration branch January 4, 2024 06:07
paulhcsun pushed a commit to paulhcsun/aws-cdk that referenced this pull request Jan 5, 2024
…ter (aws#28534)

This pull request introduces two new properties to the `ServerlessCluster` class in the AWS CDK RDS package: `secondsBeforeTimeout` and `timeoutAction`. 

The `secondsBeforeTimeout` property allows users to specify the amount of time that Aurora Serverless v1 will attempt to find a scaling point to perform seamless scaling before enforcing the timeout action. The default value is 300 seconds (5 minutes).

The `timeoutAction` property allows users to specify the action to take when the timeout is reached. Users can choose between `ForceApplyCapacityChange`, which will force the capacity to the specified value as soon as possible, even without a scaling point, and `RollbackCapacityChange`, which will ignore the capacity change if a scaling point is not found. The default behavior is `RollbackCapacityChange`.

These enhancements provide users with more control over the scaling behavior of their Aurora Serverless clusters.

Closes aws#27183

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws-rds : Allow full support for scaling configuration options
3 participants