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

rds: Bug in writer/reader definition of clusters #30260

Closed
Assignees
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@lucabrunox
Copy link

lucabrunox commented May 17, 2024

Describe the bug

When creating a fresh cluster with readers and writer, one of the reader instances is effectively created before the writer and becomes the actual writer. So the opposite of what's written in the CDK code happens. This is a bug and it's reproducible.

Expected Behavior

The expected behavior is that the writer specified in CDK is indeed the writer when the cluster is created.

Current Behavior

The current behavior is that the writer becomes the reader, and viceversa, on a fresh cluster creation, which is the opposite of what's specified in CDK.

The CDK puts the writer instance first in CloudFormation, but that's not enough to guarantee that the first instance being created in the cluster because CloudFormation creates resources in parallel.

Reproduction Steps

Code:

import { App, RemovalPolicy, Stack, StackProps } from 'aws-cdk-lib';
import { InstanceClass, InstanceSize, InstanceType, SubnetType, Vpc } from 'aws-cdk-lib/aws-ec2';
import {
  AuroraMysqlEngineVersion,
  ClusterInstance,
  Credentials,
  DatabaseCluster,
  DatabaseClusterEngine,
} from 'aws-cdk-lib/aws-rds';

export class RDSReproduceStack extends Stack {
  constructor(parent: App, id: string, props: StackProps, vpc: Vpc) {
    super(parent, id, props);

    const cluster = new DatabaseCluster(this, 'testcluster', {
      clusterIdentifier: 'testcluster',
      engine: DatabaseClusterEngine.auroraMysql({ version: AuroraMysqlEngineVersion.VER_3_04_1 }),
      credentials: Credentials.fromGeneratedSecret('myuser', {}),
      writer: ClusterInstance.provisioned('writer', {
        instanceIdentifier: 'writer',
        instanceType: InstanceType.of(InstanceClass.BURSTABLE4_GRAVITON, InstanceSize.MEDIUM),
        publiclyAccessible: false,
      }),
      serverlessV2MinCapacity: 0.5,
      serverlessV2MaxCapacity: 1.0,
      readers: [
        ClusterInstance.serverlessV2('reader', {
          scaleWithWriter: true,
          publiclyAccessible: false,
        }),
      ],
      vpc: vpc,
      vpcSubnets: {
        subnetType: SubnetType.PRIVATE_WITH_EGRESS,
      },
      removalPolicy: RemovalPolicy.DESTROY,
    });
  }
}

Result, see in the image that the writer is the reader and viceversa, on a freshly created cluster:

snap 240508161922 wxXVOsXL

Possible Solution

The possible solution is to add a dependency on the writer instance to all the reader instances here: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-rds/lib/cluster.ts , to guarantee that the writer instance is the first instance being created in the cluster and thus become the writer.

Additional Information/Context

No response

CDK CLI Version

2.130.0

Framework Version

No response

Node.js Version

18

OS

Ubuntu

Language

TypeScript

Language Version

No response

Other information

No response

@lucabrunox lucabrunox added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 17, 2024
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label May 17, 2024
@pahud
Copy link
Contributor

pahud commented May 20, 2024

internal tracking: D132052101

@pahud
Copy link
Contributor

pahud commented May 20, 2024

Hi

According to this:

// need to create the writer first since writer is determined by what instance is first
const writer = props.writer!.bind(this, this, {
monitoringInterval: props.monitoringInterval,
monitoringRole: monitoringRole,
removalPolicy: props.removalPolicy ?? RemovalPolicy.SNAPSHOT,
subnetGroup: this.subnetGroup,
promotionTier: 0, // override the promotion tier so that writers are always 0
});
instanceIdentifiers.push(writer.instanceIdentifier);

And this

instanceIdentifiers.push(clusterInstance.instanceIdentifier);

The writer should be at the first position of the instanceIdentifiers and executes the bind() that provisions the instance. But actually there's no dependencies to ensure the first reader would create after the writer.

I think we still need to ensure the dependencies for them.

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels May 20, 2024
@godwingrs22 godwingrs22 self-assigned this May 20, 2024
@mergify mergify bot closed this as completed in #30277 May 22, 2024
mergify bot pushed a commit that referenced this issue May 22, 2024
This PR ensures the dependency on the readers always to be created after the writer. This might not be the best solution as all the readers will not start provisioning until the writer is completed. Another solution is to build a custom resource to check if the writer has started provisioning, if yes, return success and let all the dependent readers start provisioning. But that would require a new custom resource.

- [x] unit tests
- [x] update integ tests
  - fixed the integ error `"Cannot find version 8.0.mysql_aurora.3.01.0 for aurora-mysql` for `integ.cluster-instance-id`

### Issue # (if applicable)

Closes #30260

### Reason for this change



### Description of changes



### Description of how you validated changes



### 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*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

atanaspam pushed a commit to atanaspam/aws-cdk that referenced this issue Jun 3, 2024
This PR ensures the dependency on the readers always to be created after the writer. This might not be the best solution as all the readers will not start provisioning until the writer is completed. Another solution is to build a custom resource to check if the writer has started provisioning, if yes, return success and let all the dependent readers start provisioning. But that would require a new custom resource.

- [x] unit tests
- [x] update integ tests
  - fixed the integ error `"Cannot find version 8.0.mysql_aurora.3.01.0 for aurora-mysql` for `integ.cluster-instance-id`

### Issue # (if applicable)

Closes aws#30260

### Reason for this change



### Description of changes



### Description of how you validated changes



### 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*
vdahlberg pushed a commit to vdahlberg/aws-cdk that referenced this issue Jun 10, 2024
This PR ensures the dependency on the readers always to be created after the writer. This might not be the best solution as all the readers will not start provisioning until the writer is completed. Another solution is to build a custom resource to check if the writer has started provisioning, if yes, return success and let all the dependent readers start provisioning. But that would require a new custom resource.

- [x] unit tests
- [x] update integ tests
  - fixed the integ error `"Cannot find version 8.0.mysql_aurora.3.01.0 for aurora-mysql` for `integ.cluster-instance-id`

### Issue # (if applicable)

Closes aws#30260

### Reason for this change



### Description of changes



### Description of how you validated changes



### 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*
@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.