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

aws-rds: deprecate writer and readers props for DatabaseCluster #26726

Open
plumdog opened this issue Aug 11, 2023 · 13 comments
Open

aws-rds: deprecate writer and readers props for DatabaseCluster #26726

plumdog opened this issue Aug 11, 2023 · 13 comments
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database documentation This is a problem with documentation. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p3

Comments

@plumdog
Copy link
Contributor

plumdog commented Aug 11, 2023

Describe the issue

The props instances and instanceProps for rds.DatabaseCluster are deprecated in favour of writer and readers.

What is the behaviour of these when the Aurora cluster fails over? If I create a reader, can it become promoted to a writer, or will it always be a reader?

Relevant Aurora docs: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/Concepts.AuroraHighAvailability.html#Concepts.AuroraHighAvailability.Instances

When a problem affects the primary instance, one of these reader instances takes over as the primary instance. This mechanism is known as failover.

Further, assuming they do all just become instances in the cluster, any of which might get promoted to be the writer, what happens on future CDK deployments? I can imagine a situation where I think I'm changing something about the readers, but am actually changing something about all-but-one of the readers and the writer, because:

  • the instance CDK thinks of as the writer is now a reader
  • the instance that is actually the writer is one of the instances the CDK thinks of as readers

I don't see any discussion around this in #25437, the PR that introduced this.

I might just be missing something, but the structure of this seems at odds with how an Aurora cluster actually behaves.

Links

CDK docs:

Aurora failover docs: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/Concepts.AuroraHighAvailability.html#Concepts.AuroraHighAvailability.Instances

@plumdog plumdog added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Aug 11, 2023
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Aug 11, 2023
@pahud pahud self-assigned this Aug 14, 2023
@pahud
Copy link
Contributor

pahud commented Aug 14, 2023

Thanks for the callout. When you delete a writer from the console, the reader will immediately promoted as the new writer and if you go to cloudformation to "detect drfit", the stack will be in DRIFTED status because the writer has been deleted.

Unfortunately we didn't address the recommended practice how to update the existing cdk stack in this scenario given the writer is deleted and reader promoted as writer. I will explore this a little bit further and see if I can find anything.

@pahud pahud added p2 effort/medium Medium work item – several days of effort investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Aug 14, 2023
@s0enke
Copy link

s0enke commented Oct 1, 2023

The naming introduced into CDK with writer and readers is misleading and confusing, since it's only valid at db cluster creation. Which instance is a reader or writer is not a property of the instance assigned by the createDbIstance API, but it depends on the order which db instance is created first: The first db instance created becomes the writer. Other instances created after the first instance become readers. And, in case of a fail-over (initiated by a cloud event, or a manually triggered fail-over by a user), a reader becomes the new writer. CDK cannot and should not reflect this in their APIs, since it's a dynamic property.

@plumdog plumdog changed the title aws-rds: document behaviour of DatabaseCluster writer and readers under failover aws-rds: deprecate writer and readers props for DatabaseCluster Oct 1, 2023
@plumdog
Copy link
Contributor Author

plumdog commented Oct 1, 2023

I've updated the title of this issue because I don't think it's possible to resolve this confusion adequately with documentation as the behaviour implied by CDK's writer and readers props is contrary to how Aurora behaves.

@s0enke
Copy link

s0enke commented Oct 2, 2023

I just noticed that even the initial writer/reader assignment might no be valid. With a fresh cluster (one writer, one reader), Aurora just assigned the supposedly writer role ("assigned" by the CDK) the reader role, and vice versa.

@adrianhincuvisma
Copy link

adrianhincuvisma commented Nov 23, 2023

Hello, I have an issue, I'm trying to migrate from instanceProps to writer and readers and CloudFormation wants to delete my old isntances and replace them with new ones, which is not optimal, how did you guys handle this? Don't tell me you created new ones and then removed the old ones

LE: found a solutions here #25900

@tim-finnigan tim-finnigan added the feature-request A feature should be added or improved. label Mar 14, 2024
@jsamuel1
Copy link

jsamuel1 commented Apr 7, 2024

I'm hitting this issue.
Created a cluster with writer property, and added DBInstances.
After writer is deleted in cluster (outside of CDK), fails to update.

@jsamuel1
Copy link

jsamuel1 commented Apr 7, 2024

Ideal scenario -- Allow for initial creation of multiple provisioned DBInstances in the cluster, spread across AZ's, but don't fail on upgrades if they are different later.

btw. It would be good if we could specify a naming convention for the dbInstances too - eg. include the AZ letter identifier in the name.

@mjgp2
Copy link

mjgp2 commented Apr 9, 2024

This new API is very counterintuitive and I would support rolling it back. The readers and writers concept is orthogonal to how readers and writers are chosen - they are certainly not static assignments.

Further, I have just come to look at specifying the new CA certificates, but it's not available in the instanceProps so I cannot update the CA certs without trying to use a workaround.

@pahud pahud added p3 and removed p2 labels Jun 11, 2024
@mjgp2
Copy link

mjgp2 commented Jun 15, 2024

@pahud - can you reassure the community here that the deprecated instances and instanceProps for the DatabaseClusterProps will not be removed until there is a viable solution in place? Ideally dropping the deprecated flag on them for now? Right now for aurora provisioned with read replicas it works as expected, with there being no difference in the CDK between writers and readers as RDS treats them all as a pool of fungible instances. It would be a disaster for us if the APIs were removed with no viable upgrade path. Thanks!

@pahud
Copy link
Contributor

pahud commented Aug 7, 2024

Hi @mjgp2

With #30277, on the cluster initial provisioning, the writer would always be created before the reader so it would reflect the initial state. But due to the nature of failover, the writer could be gone with reader promoted as the new writer and that would be a drift. I have no solution off the top of my head when the drift happens what is the best way to continue update this stack but I think this is something we should figure out the solution in CFN before we know how to do that in CDK.

I am leaving this thread open. Any feedbacks are welcome here.

@pahud pahud removed their assignment Aug 7, 2024
@pahud pahud removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Aug 7, 2024
@s0enke
Copy link

s0enke commented Aug 8, 2024

@pahud the solution is already in CFN: CFN does simply not assume any writers or readers. There are only plain DBInstances. That should also be the solution for CDK: Writer/reader is a runtime property of instances which can change at any time, which may not be managed by IaC tools. The only thing CDK might set for sure is an initial writer/reader setup.

@zrfr
Copy link
Member

zrfr commented Aug 23, 2024

RDS CFN engineer here. The feedback in this issue is correct. The current design of DatabaseCluster doesn't model Aurora clusters correctly and makes it unwieldy to properly implement a multi-AZ Aurora cluster.

As others have mentioned, Aurora cluster instances are supposed to be fungible (although this is a simplification). Any instance can be the writer, and which one it is can change at any time when a failover occurs. There is no direct control over which instance becomes the writer, but you can influence the choice using promotion tiers. Failover is not considered as drift and does not block CFN stack updates.

Instead of having 1 writer + n readers, it would be more correct to model cluster instances in terms of promotion tiers.

@AyoubOukh
Copy link

Any updates here please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database documentation This is a problem with documentation. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p3
Projects
None yet
Development

No branches or pull requests

9 participants