aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.47k stars 3.82k forks source link

(dynamodb): pointInTimeRecovery for global tables (replicationRegions) #18582

Open phstc opened 2 years ago

phstc commented 2 years ago

Description

I want to enable pointInTimeRecovery for global tables.

replicationRegions creates tables on the given regions, but the pointInTimeRecovery setting is not inherited from the main table definition.

Use Case

I want point-in-time recovery from any global table region for additional resilience; ability to recover from any region, in case the primary region is failing.

Proposed Solution

I would like to propose that the global tables inherit pointInTimeRecovery from the main table or support to customize table props for global tables, for example, replicationRegionPropOverrides: Partial<TableProps>.

Other information

No response

Acknowledge

skinny85 commented 2 years ago

Hey @phstc,

thanks for opening the issue. I think this might be tricky to do in our implementation of the global Tables backing replicationRegions, as we use a CloudFormation Custom Resource for this purpose. We do an UpdateTable API call (here is the code where that happens), and from what I can see, the ReplicaUpdates property of that call does not allow you to pass pointInTimeRecovery for the replicas.

I see that the new AWS::DynamoDB::GlobalTable resource supports setting pointInTimeRecovery through the Replicas property. We don't support that resource in the Layer 2 Table class in the CDK, but we do have the CfnGlobalTable class that allows you to use it.

Maybe that's a better choice?

Thanks, Adam

github-actions[bot] commented 2 years ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

phstc commented 2 years ago

@skinny85 thank you - that's very helpful.

Since it is an existing table, I'm wondering if changing to CfnGlobalTable would cause CDK/Formation to consider it a new resource instead of an update, and it would try to delete the existing table and create a new one.

We do an UpdateTable API call (here is the code where that happens)

Could that also call updateContinuousBackups for updating the point in time recovery?

In the interim, we are considering a AwsCustomResource:

new AwsCustomResource(this, id, {
  onUpdate: {
    service: "DynamoDB",
    action: "updateContinuousBackups",
    parameters: {
      TableName: this.globalTable.tableName,
      PointInTimeRecoverySpecification: {
        PointInTimeRecoveryEnabled: true,
      },
    },
    region,
    physicalResourceId: PhysicalResourceId.of(id),
  },
  policy: AwsCustomResourcePolicy.fromSdkCalls({
    resources: AwsCustomResourcePolicy.ANY_RESOURCE,
  }),
});
skinny85 commented 2 years ago

Yes, I agree with you on both of those points @phstc. I think the Custom Resource is a good interim solution.

Would you be interested in contributing the change to call updateContinuousBackups() in our Custom Resource? Our "Contributing" guide: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md.

Thanks, Adam

Goncharo commented 1 year ago

Just ran into this as well as per this construct:

    return new dynamodb.Table(stack, "MyTable", {
      partitionKey: {
        name: "attr1_attr2",
        type: dynamodb.AttributeType.STRING,
      },
      sortKey: { name: "year_month", type: dynamodb.AttributeType.STRING },
      replicationRegions: ["us-east-1"],
      replicationTimeout: Duration.hours(3),
      encryption: dynamodb.TableEncryption.AWS_MANAGED,
      pointInTimeRecovery: true,
    });

As noted in the issue, the pointInTimeRecovery setting did not get passed down to the replica tabes, and they were created with PITR disabled.

Any ETA for this fix in CDK?

rix0rrr commented 11 months ago

This issue was for the existing Table construct, which used custom resources to implement table replication. We no longer recommend the use of the Table construct.

Instead, the TableV2 construct has been released in 2.95.1 (#27023) which maps to the AWS::DynamoDB::GlobalTable resource, has better support for replication and does not suffer from the issue described here.


Be aware that there are additional deployment steps involved in a migration from Table to TableV2. You need to do a RETAIN deployment, a delete deployment, then change the code to use TableV2 and then use cdk import. A link to a full guide will be posted once it is available.

Here are some other resources to get you started (using CfnGlobalTable instead of TableV2) if you want to get going on the migration: