Page MenuHomePhabricator

[services] add GSI to users table for identity service
ClosedPublic

Authored by varun on Jul 26 2022, 9:20 PM.
Tags
None
Referenced Files
F3284517: D4647.diff
Sat, Nov 16, 12:14 PM
Unknown Object (File)
Sun, Oct 27, 6:19 PM
Unknown Object (File)
Sun, Oct 27, 6:19 PM
Unknown Object (File)
Sun, Oct 27, 6:19 PM
Unknown Object (File)
Sun, Oct 27, 6:19 PM
Unknown Object (File)
Sun, Oct 27, 6:16 PM
Unknown Object (File)
Oct 5 2024, 12:08 AM
Unknown Object (File)
Oct 3 2024, 7:57 AM

Details

Summary

in order to look up userIDs by username or wallet address efficiently, we need a couple global secondary indexes. these will let us query by attribute instead of scanning the entire table.

Test Plan

terraform validate and terraform fmt and terraform plan

note: learned that validate only checks the syntax. we should run plan to make sure DynamoDB-specific requirements are met

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun requested review of this revision.Jul 26 2022, 9:32 PM
max added inline comments.
services/terraform/dynamodb.tf
182 ↗(On Diff #15000)

Are we really KEYS_ONLY? In this case, you can search for a username key only without projection for attributes, e.g. query for the only purpose: if the certain key exists in the table.

tomek requested changes to this revision.Jul 27 2022, 4:16 AM
tomek added inline comments.
services/terraform/dynamodb.tf
166–191 ↗(On Diff #15000)

Maybe I'm missing something, but according to the docs https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/dynamodb_table#global_secondary_index

hash_key - (Required) Name of the hash key in the index; must be defined as an attribute in the resource.

But both username and walletAddress aren't defined as attributes of identity-users. Could you explain how this works?

This revision now requires changes to proceed.Jul 27 2022, 4:16 AM
varun added inline comments.
services/terraform/dynamodb.tf
166–191 ↗(On Diff #15000)

good catch

varun@varuns-MacBook-Pro terraform % terraform plan
╷
│ Error: 1 error occurred:
│       * all indexes must match a defined attribute. Unmatched indexes: ["username" "walletAddress"]
│ 
│ 
│ 
│   with aws_dynamodb_table.identity-users,
│   on dynamodb.tf line 166, in resource "aws_dynamodb_table" "identity-users":
│  166: resource "aws_dynamodb_table" "identity-users" {
│

i'll update this now

182 ↗(On Diff #15000)

Yeah this is intentional. The purpose of this index is to look up the primary key of the table (userID) with the username attribute. We don't need to pull other attributes into the index.

define table attributes used in indexes

Do you think we can include terraform validate and terraform plan in our CI?

This revision is now accepted and ready to land.Jul 28 2022, 1:34 AM
In D4647#133940, @tomek wrote:

Do you think we can include terraform validate and terraform plan in our CI?

yeah it's a backlogged task https://linear.app/comm/issue/ENG-1392/add-terraform-pre-commit-hook-to-github-actions-buildkite