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

Documentation for ignore_changes_global_secondary_index should warn that it will destroy the table #82

Closed
mnylensc opened this issue Feb 8, 2024 · 6 comments · Fixed by #83

Comments

@mnylensc
Copy link
Contributor

mnylensc commented Feb 8, 2024

Is your request related to a problem? Please describe.

Documentation states that

Warning: autoscaling with global secondary indexes

When using an autoscaled provisioned table with GSIs you may find that applying TF changes whilst a GSI is scaled up will reset the capacity, there is an open issue for this on the AWS Provider. To get around this issue you can enable the ignore_changes_global_secondary_index setting however, using this setting means that any changes to GSIs will be ignored by Terraform and will hence have to be applied manually (or via some other automation).

However, it should maybe also warn you that trying to set this variable after the table is already created will cause the table to be recreated.

Describe the solution you'd like.

Mention this caveat and maybe offer a migration path in the documentation.

It seems that using terraform state mv "module.dynamodb.aws_dynamodb_table.autoscaled[0]" "module.dynamodb.aws_dynamodb_table.autoscaled_gsi_ignore[0]" before applying the change from ignore_changes_global_secondary_index = false -> true will preserve the table without recreating it.

@bryantbiggs
Copy link
Member

bryantbiggs commented Feb 8, 2024

I don't see the benefit of adding anything additional here. The API documentation is clear, the provider documentation is clear, and the Terraform plan/apply output is quite clear - am I missing anything?

@mnylensc
Copy link
Contributor Author

mnylensc commented Feb 8, 2024

Well, the documentation does feel it necessary to mention this:

Warning: enabling or disabling autoscaling can cause your table to be recreated

There are two separate Terraform resources used for the DynamoDB table: one is for when any autoscaling is enabled the other when disabled. If your table is already created and then you change the variable autoscaling_enabled then your table will be recreated by Terraform. In this case you will need to move the old aws_dynamodb_table resource that is being destroyed to the new resource that is being created. For example:

terraform state mv module.dynamodb_table.aws_dynamodb_table.this module.dynamodb_table.aws_dynamodb_table.autoscaled

So if the documentation warns about enabling or disabling autoscaling will cause your tables to be recreated, why the similar warning shouldn't be extended to the case where you use ignore_changes_global_secondary_index, which has the same small side effect of potentially recreating your table.

Then, the inputs section says just:

ignore_changes_global_secondary_index - Whether to ignore changes lifecycle to global secondary indices, useful for provisioned tables with scaling

Not even a mention that there's a bit more to this, unlike with autoscaling_enabled where it at least mentions to read a note:

autoscaling_enabled - Whether or not to enable autoscaling. See note in README about this setting

Just my two cents, but the only way for average person to know this will happen is to try it and then see terraform plan suggesting to delete your production table. Could use some warning at the documentation level.

@bryantbiggs
Copy link
Member

Terraform, and modules like this, are just the implementation. Users need to know the services and their API behaviors regardless of the implementation. We're not going to duplicate the service and API docs here, we focus on documenting this specific implementation

@mnylensc
Copy link
Contributor Author

mnylensc commented Feb 9, 2024

I created a PR with my proposed changes for the documentation - #83

@antonbabenko
Copy link
Member

This issue has been resolved in version 4.0.1 🎉

Copy link

github-actions bot commented Apr 7, 2024

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants