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

fix(egress-tracking): new partition and sort key for dynamo table #443

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

fforbeck
Copy link
Member

Context
I've identified an issue in our DynamoDB table where egress events were not being stored correctly. This was traced back to the configuration of our partition key (PK) and sort key (SK). In DynamoDB, the partition key is crucial for uniquely identifying each item, while the sort key helps organize data within a partition.

This change ensures that the combination of partition key and sort key is unique for each record, preventing DynamoDB from overriding existing records in case of a collision.

Changes Made

  1. Modification of Partition Key (PK) & Sort Key (SK):

    • The PK was updated to ensure it uniquely identifies each record. Previously, key collisions occurred because multiple records were using the same PK value, leading to storage issues.
  2. Inclusion of cause in Stripe Idempotent Key:

    • Added cause as part of the Stripe idempotent key to ensure the uniqueness of requests to Stripe, preventing duplicate processing.
  3. Renaming of Table from egress-traffic to egress-traffic-events:

    • Since existing partition keys and sort keys cannot be updated, the table was renamed and the keys were updated. The existing table will be dropped to accommodate these changes.

Impact

  • These changes resolve the storage issues for egress events and improve data integrity and retrieval efficiency.
  • The renaming and restructuring of the table ensure that future records are stored and managed correctly.

Blocked by storacha/w3up#1588

@@ -5,7 +5,7 @@
"packageManager": "[email protected]+sha256.c8c61ba0fa0ab3b5120efd5ba97fdaf0e0b495eef647a97c4413919eda0a878b",
"type": "module",
"scripts": {
"start": "sst start",
"start": "sst dev",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sst start command is deprecated.

Copy link

seed-deploy bot commented Nov 22, 2024

View stack outputs

@seed-deploy seed-deploy bot temporarily deployed to pr443 November 25, 2024 12:17 Inactive
@fforbeck fforbeck merged commit 180842f into main Nov 25, 2024
3 checks passed
@fforbeck fforbeck deleted the fix/egress-record branch November 25, 2024 13:20
fforbeck added a commit to storacha/freeway that referenced this pull request Nov 25, 2024
Added the `nonce` as suggested per @alanshaw, but also set it to not
expire, so we can process the invocation any time. The `servedAt` field
doesn't need to be converted to seconds.

Blocked by storacha/w3infra#443 &
storacha/w3up#1588
@@ -191,7 +191,7 @@ export const createEgressTrafficTestContext = async () => {
})
}

const egressTrafficTable = await createTable(awsServices.dynamo.client, egressTrafficTableProps, 'egress-traffic-')
const egressTrafficTable = await createTable(awsServices.dynamo.client, egressTrafficTableProps, 'egress-traffic-events-')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why dash suffix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how tables and queue names are defined in the test context. Then it adds some random value at the end for each time you execute the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants