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

Dynamodb: add GSI to existing Dynamodb table via CDK deploy #12343

Closed
2 tasks
mattiLeBlanc opened this issue Jan 5, 2021 · 18 comments
Closed
2 tasks

Dynamodb: add GSI to existing Dynamodb table via CDK deploy #12343

mattiLeBlanc opened this issue Jan 5, 2021 · 18 comments
Assignees
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@mattiLeBlanc
Copy link

A way to use Table.addGlobalSecondaryIndex for existing tables so we can Add an inde to a existing table using the CDK.

Use Case

I create my DynamoDB tables via the CDK.

Now I have to add an index, so I want to ideally update the existing table and not recreate the table (no delete policy is enabled).
So, in my CDK script I check (using the SDK) if the table already exists. If it doesn't exist I use :

const table = new Table(this, tableName, options);
// and 
table.addGlobalSecondaryIndex(options);

to create table and GSI.

However, if the table does already exist (in staging or prod), I am using

const table = Table.fromTableName(this, tableName, tableName);

to get a table reference ( so I can use that to get the datasource for Appsync resolvers and such).

The problem I have is that fromTableName returns an interface of ITable and not Table. ITable doesn't have the addGlobalSecondaryIndex function, only Table does.
So there is no way for me (as far as I know) to add an index to an existing table, either then removing the table first (bad) or adding it manually.

I have add a hacky workaround in my CDK deploy script using the SDK where I use the updateTable command from @aws-sdk/client-dynamodb however that is not ideal.

Proposed Solution

If the Table.fromTableName function would return a Table interface, that could allow us to call addGlobalSecondaryIndex or removeGlobalSecondaryIndex and update our tables via the CDK.

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@mattiLeBlanc mattiLeBlanc added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 5, 2021
@mattiLeBlanc mattiLeBlanc changed the title Dynamodb: ad GSI to existing Dynamodb table Dynamodb: add GSI to existing Dynamodb table via CDK deploy Jan 5, 2021
@skinny85
Copy link
Contributor

skinny85 commented Jan 5, 2021

That's not something we can accommodate @mattiLeBlanc . We can't modify resources in the CDK that are not managed in the CDK.

If you want to do different things in test / prod, you need to vary it in code:

const app = new cdk.App();

// test
new MyStack(app, 'TestStack');

// prod
new MyStack(app, 'ProdStack', {
  table: 'my-table-arn',
});

interface MyStackProps extends cdk.StackProps {
  readonly table?: string;
}

class MyStack extends cdk.Stack {
  readonly table: dynamodb.ITable;

  constructor(scope: Construct, id: string, props?: MyStackProps) {
    super(scope, id, props);

    if (props.table) {
      this.table = dynamodb.Table.fromTableArn(this, 'Table', props.table);
    } else {
      this.table = new dynamodb.Table(this, 'Table', {
        // do whatever you need here...
      });
    }
  }
}

Thanks,
Adam

@skinny85 skinny85 self-assigned this Jan 5, 2021
@skinny85 skinny85 added @aws-cdk/aws-dynamodb Related to Amazon DynamoDB guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 5, 2021
@mattiLeBlanc
Copy link
Author

mattiLeBlanc commented Jan 5, 2021

@skinny85
Hi Adam,

I am using a stack to create my table, it is just when I want to update a deployed table by adding an index, I want to do it through the CDK and not manually for each env and developer that might have the stack deployed.

fromTableArn and fromTableName both return an ITable interface which doesn't allow me to add a GSI (it doesn't implement addGlobalSecondaryIndex().
Does this mean that adding a new GSI or changing an existing can only be done via the SDK and not the CDK?

@skinny85
Copy link
Contributor

skinny85 commented Jan 5, 2021

Does this mean that adding a new GSI or changing an existing can only be done via the SDK and not the CDK?

Yes. If you want a GSI added to a Table using the CDK, that Table also needs to be created by the CDK.

@skinny85
Copy link
Contributor

skinny85 commented Jan 5, 2021

Generally, those things are done in CDK by code changes. You change the code to add the GSI to your CDK-managed Table, and then everyone who updates their code will get the GSI the next time they deploy their Stacks.

@mattiLeBlanc
Copy link
Author

@skinny85 I think we are misunderstanding each other. My table is created by CDK. The problem is that in the docs or typescript definition I do not see a way to add a GSI to an existing table.
the addGlobalSecondaryIndex is only available on the table object returned by the const table = new Table(this, tableName, options); constructor, which returns an interface of Table.

When I already have a table with production data and I we decide to add an index later, I can't remove the table and recreate because I don't want to lose data.

Using the fromTableArn or fromTableName is not returning a Table object but and interface of ITable which doesn't seem to have a function to add or update an index.

Do you understand my issue now?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jan 6, 2021
@skinny85
Copy link
Contributor

skinny85 commented Jan 6, 2021

No, sorry 😕.

If you have a Table managed by the CDK, then you have a Table, not an ITable already available, so what is the problem of changing the code to add the GSI to it?

@skinny85 skinny85 added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jan 6, 2021
@mattiLeBlanc
Copy link
Author

@skinny85
Okay I will try it again, giving more context.

So before the CDK starts generating Cloudformation stuff I run a SDK listTables to get all tablenames and I extract which tables already exist for my account.
That gives me table names (but pretty sure I can get the arn or contruct the arn).

I have a DynamoTableConstruct which creates tables out of a graphqQl schema (like amplify does).
So when I add @model to my schema, that table get's created during the CDK deploy.

However, if I add an index to my schema (to an existing Type which has the @model annotation), and deploy again, the table already exists and I would get a CDK error saying table X can't be created, right?

So I use the list of tables I fetched before to decide if I use:
new Table(this, tableName, options)
or
Table.fromTableName(this, tableName, tableName);.

Just to be clear, it is my understanding that I can't use new Table(this, tableName, options) to update an existing table. So hence I go for the fromTableName or I could use fromTableArn.

However:

image

both functions return an ITable which doesn't support the function addGlobalSecondaryIndex().

new Table() returns a type of Table:
image

those 2 fromTable or fromArn return ITable don't have it:
image

So my issue is a Typescript related issue perhaps.

So in your original example I have no clue how to to add a GSI with the current Typescript:

class MyStack extends cdk.Stack {
  readonly table: dynamodb.ITable;

  constructor(scope: Construct, id: string, props?: MyStackProps) {
    super(scope, id, props);

    if (props.table) {
      this.table = dynamodb.Table.fromTableArn(this, 'Table', props.table);
    } else {
      this.table = new dynamodb.Table(this, 'Table', {
        // do whatever you need here...
      });
    }
   if (this.table) {
     // THIS WILL ONLY WORK WHEN WE CREATE NEW TABLE 
     this.table.addGlobalSecondaryIndex(options);
   }
  }
}

I can't make my issue anymore clear then this. Maybe I am looking at it from the wrong angle and I am not seeing the obvious so I hope at least now you understand what issue I am running into from my perspective.

Thanks for your time and patience.

@skinny85
Copy link
Contributor

skinny85 commented Jan 6, 2021

However, if I add an index to my schema (to an existing Type which has the @model annotation), and deploy again, the table already exists and I would get a CDK error saying table X can't be created, right?

No. The existing Table would be updated with the GSI.

@mattiLeBlanc
Copy link
Author

mattiLeBlanc commented Jan 6, 2021

But I am getting a Table already exist error when I try to update my table which already exists:

App-matti-ClientApiStack: creating CloudFormation changeset...
10:47:42 AM | CREATE_FAILED        | AWS::DynamoDB::Table                | tablesearnrmattiapplication81F3B8EA
app_matti-application already exists
[██████████▉···············································] (7/37)

10:47:42 AM | CREATE_FAILED        | AWS::DynamoDB::Table                | tables/app_matti-application
app_matti-application already exists
10:47:42 AM | CREATE_FAILED        | AWS::DynamoDB::Table                | tables/app_matti-individual

using new Table(this, tableName, options); on an existing table with an updated options object with the new index.
This doesn't work.
And using fromTableArn doesn't work either because it doesn't expose the addGlobalSecondaryIndex in on the ITable interface.

I am don't know how to further explain it, I have exhausted my story telling. :(

edit

I will try to force the type of when I fetch a table via Arn or Name to the interface of Table and see if that works.

update 2
okay just tried fetching the table with fromTableArn and forced interface as Table so I can't fool the TS compiler the function
addGlobalSecondaryIndex exists, but it doesn't:

ByCognitoId doesn't exist on table app_matti-individual. Adding index
(node:50637) UnhandledPromiseRejectionWarning: TypeError: tableResource.addGlobalSecondaryIndex is not a function
    at new DynamoTableConstruct (/Users/matti/www/app/app-client-api/lib/constructs/dynamodb.construct.ts:69:27)
    at AppClientApiStack.init (/Users/matti/www/app/app-client-api/lib/app-client-api-stack.ts:36:27)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

I don't know how to resolve this.

@skinny85
Copy link
Contributor

skinny85 commented Jan 7, 2021

Here's how the CDK works.

Point in time #1

This is the code:

const app = new App();
const stack = new Stack(app, 'Stack');
const table = new dynamodb.Table(stack, 'Table', {
            partitionKey: {
                name: 'Id',
                type: dynamodb.AttributeType.STRING,
            },
            removalPolicy: RemovalPolicy.DESTROY,
        });

You run cdk deploy (or do it through a Pipeline, etc.). The Table is created.

Point in time #2

You want to add a GSI. You change the code to be:

const app = new App();
const stack = new Stack(app, 'Stack');
const table = new dynamodb.Table(stack, 'Table', {
            partitionKey: {
                name: 'Id',
                type: dynamodb.AttributeType.STRING,
            },
            removalPolicy: RemovalPolicy.DESTROY,
        });
table.addGlobalSecondaryIndex({
            partitionKey: {
                name: 'Id',
                type: dynamodb.AttributeType.STRING,
            },
            indexName: 'some-index',
        });

You run cdk deploy again. The existing Table is updated with the GSI.


I also don't know how to explain this better 😕.

@mattiLeBlanc
Copy link
Author

@skinny85 Yeah I think I see why we dont understand each other.
I mentioned in my first post:
so I want to ideally update the existing table and not recreate the table (no delete policy is enabled).
I mean of course RemovalPolicy. I use RemovalPolicy.RETAIN because I don't want to lose production data.

What I think is happening is that when you use DESTROY, it is fine to run the new Table() command. But when you use RETAIN, it doesn't work.

Does that make sense?

I can't imagine you every want to destroy your table in production when you want to add an index later, that would be very unhelpful and force me to manually add the index via the AWS console. But I want everything to stay managed via the CDK.

@skinny85
Copy link
Contributor

skinny85 commented Jan 7, 2021

No. RemovalPolicy.DESTROY is just for my testing ease.

Feel free to remove it from the code I posted above, and try it yourself. Just remember to manually delete the Table after you're done testing.

@mattiLeBlanc
Copy link
Author

Okay, I did some more testing an initially it didn't work, I would get :

11:21:45 AM | CREATE_FAILED        | AWS::DynamoDB::Table                | tables/app_matti-application
app_matti-application already exists
11:21:45 AM | CREATE_FAILED        | AWS::DynamoDB::Table                | tables/app_matti-individual
app_mattis-individual already exists

So I did start to test some variations like not adding a tableName to the TableProps since you don't have that, for that I had to remove my tables and recreate them and of course everything works now that I have recreated them with or without tableName in props.
I can update them using the new Table() command without the error.
Really frustrating.....

So I think what is going on that for some reason my tables where not part of the stack anymore, it's delta lost sync or something and that is why it tried to create another table with the same name, hence the name collision error.

I will have to check in Staging and production if the tables are also not in the stack anymore. I believe I can see that in Resource list and I know how to import resources back into a stack.

Adam, thanks for your patience man, this was just a difficult issue because it was not clear to me my tables have fallen outside of the stack (they where created by the stack initially, never separately).

@skinny85
Copy link
Contributor

skinny85 commented Jan 7, 2021

I think I might guess what's happening. You said you set a specific name for a Table; perhaps you're doing an update that causes a replacement of the Table (Meaning, the given change cannot be applied to the existing Table; a new Table must be created instead)? In that case, CloudFormation will first try to create a new resource, and if you're setting a specific name for the Table, then it will fail with the "Table with name 'xyz' already exists" error. You can see which changes cause a replacement by running cdk diff, or consulting this page (look for "Update requires: Replacement").

A few pointers:

  1. Don't use physical names, and let CloudFormation generate a name for you. This alleviates this problem completely.
  2. Change the physical name when you're doing a change that requires replacing the Table.

Hope this helps!

Thanks,
Adam

@mattiLeBlanc
Copy link
Author

mattiLeBlanc commented Jan 7, 2021

Hi Adam,

Strangely, when I removed my stack and tables and recreated the tables, with a tablename, and then added the GSI and deployed again, no error.
I do notice my tables in Staging and Prod are not listed as resources in the Cloudformation script. That is why I think it doesn't update the table, because it can't find the reference to the resource anymore and tries to create a new one.
Not using tableName would generate a new unique name but then you would have a duplicate table with no data. So that would not be a solution for me.

I am redeploying it now to verify if the table is listed, and then I will manually reimport the resources in my stacks and test if I can do the update.

update
Just verified, my table resources are in the Cloudformation list again and the update of a GSI works even with tableName.
So the issue was table resources missing from Stack, somehow.

@skinny85
Copy link
Contributor

skinny85 commented Jan 7, 2021

That's great to hear @mattiLeBlanc !

Anything else we can help you with on this issue?

@mattiLeBlanc
Copy link
Author

Hi Adam,
No man, that is it. Thanks again.

@skinny85 skinny85 closed this as completed Jan 7, 2021
@github-actions
Copy link

github-actions bot commented Jan 7, 2021

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants