Skip to content

Commit

Permalink
feat(redshift-alpha): query execution timeout setting during table cr…
Browse files Browse the repository at this point in the history
…eation (#31818)

### Issue # (if applicable)

Closes #31329.

### Reason for this change

The timeout setting for the custom resource that executes the query is fixed at 1 minute, causing a timeout when attempting to run heavy operations.
Detail example is describe in [the original issue](#31329 (comment)).

### Description of changes

- Add timeout prop to `DatabaseQueryProps` and `TableProps`

### Description of how you validated changes

Added both unit and integ tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
badmintoncryer authored Oct 25, 2024
1 parent 09256db commit 40f07ae
Show file tree
Hide file tree
Showing 30 changed files with 567 additions and 409 deletions.
18 changes: 18 additions & 0 deletions packages/@aws-cdk/aws-redshift-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,24 @@ new Table(this, 'Table', {
});
```

Query execution duration is limited to 1 minute by default. You can change this by setting the `timeout` property.

Valid timeout values are between 1 seconds and 15 minutes.

```ts fixture=cluster
import { Duration } from 'aws-cdk-lib';

new Table(this, 'Table', {
tableColumns: [
{ id: 'col1', name: 'col1', dataType: 'varchar(4)' },
{ id: 'col2', name: 'col2', dataType: 'float' }
],
cluster: cluster,
databaseName: 'databaseName',
timeout: Duration.minutes(15),
});
```

### Granting Privileges

You can give a user privileges to perform certain actions on a table by using the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ export interface DatabaseQueryProps<HandlerProps> extends DatabaseOptions {
* @default cdk.RemovalPolicy.Destroy
*/
readonly removalPolicy?: cdk.RemovalPolicy;

/**
* The handler timeout duration
*
* @default cdk.Duration.minutes(1)
*/
readonly timeout?: cdk.Duration;
}

export class DatabaseQuery<HandlerProps> extends Construct implements iam.IGrantable {
Expand All @@ -29,14 +36,23 @@ export class DatabaseQuery<HandlerProps> extends Construct implements iam.IGrant
constructor(scope: Construct, id: string, props: DatabaseQueryProps<HandlerProps>) {
super(scope, id);

if (props.timeout && !cdk.Token.isUnresolved(props.timeout)) {
if (props.timeout.toMilliseconds() < cdk.Duration.seconds(1).toMilliseconds()) {
throw new Error(`The timeout for the handler must be BETWEEN 1 second and 15 minutes, got ${props.timeout.toMilliseconds()} milliseconds.`);
}
if (props.timeout.toSeconds() > cdk.Duration.minutes(15).toSeconds()) {
throw new Error(`The timeout for the handler must be between 1 second and 15 minutes, got ${props.timeout.toSeconds()} seconds.`);
}
}

const adminUser = this.getAdminUser(props);
const handler = new lambda.SingletonFunction(this, 'Handler', {
code: lambda.Code.fromAsset(path.join(__dirname, 'database-query-provider'), {
exclude: ['*.ts'],
}),
runtime: lambda.determineLatestNodeRuntime(this),
handler: 'index.handler',
timeout: cdk.Duration.minutes(1),
timeout: props.timeout ?? cdk.Duration.minutes(1),
uuid: '3de5bea7-27da-4796-8662-5efb56431b5f',
lambdaPurpose: 'Query Redshift Database',
});
Expand Down
9 changes: 9 additions & 0 deletions packages/@aws-cdk/aws-redshift-alpha/lib/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,15 @@ export interface TableProps extends DatabaseOptions {
* @default - no comment
*/
readonly tableComment?: string;

/**
* Handler timeout duration.
*
* Valid values are between 1 second and 15 minutes.
*
* @default - 1 minute
*/
readonly timeout?: cdk.Duration;
}

/**
Expand Down
32 changes: 32 additions & 0 deletions packages/@aws-cdk/aws-redshift-alpha/test/database-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,38 @@ describe('database query', () => {
});
});

describe('timeout', () => {
it('passes timeout', () => {
new DatabaseQuery(stack, 'Query', {
...minimalProps,
timeout: cdk.Duration.minutes(5),
});

Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', {
Timeout: 300,
Role: { 'Fn::GetAtt': ['QueryRedshiftDatabase3de5bea727da479686625efb56431b5fServiceRole0A90D717', 'Arn'] },
Handler: 'index.handler',
Code: {
S3Bucket: { 'Fn::Sub': 'cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}' },
},
});
});

it('throw error for timeout being too short', () => {
expect(() => new DatabaseQuery(stack, 'Query', {
...minimalProps,
timeout: cdk.Duration.millis(999),
})).toThrow('The timeout for the handler must be BETWEEN 1 second and 15 minutes, got 999 milliseconds.');
});

it('throw error for timeout being too long', () => {
expect(() => new DatabaseQuery(stack, 'Query', {
...minimalProps,
timeout: cdk.Duration.minutes(16),
})).toThrow('The timeout for the handler must be between 1 second and 15 minutes, got 960 seconds.');
});
});

it('passes removal policy through', () => {
new DatabaseQuery(stack, 'Query', {
...minimalProps,
Expand Down

This file was deleted.

Loading

0 comments on commit 40f07ae

Please sign in to comment.