feat: aggregator service and events #61

vasco-santos · 2023-10-30T13:29:30Z

This PR wires up aggregator service and its events. All old code from aggregator was removed given it is now in filecoin-api, so there is a huge diff of removed code. data folder, workflows folders and processor folder were dropped

Note that:

Given most bucket stores are quite identical, an abstraction layer for them was created so that we do not need to keep all the similar code and just focus on extending the differences
Created a InclusionProofStore where Proof bytes are stored. From filecoin-api interface standpoint, proof is available via inclusion-store, but we can't stick it in DynamoDB. While there are 2 PUT ops behind now (note that if the first fails to the proof bucket, there is no issue as nothing will happen with it)
Removed most of integration tests as they need to be rewritten as follow up

seed-deploy · 2023-10-31T11:06:14Z

View stack outputs

pr61-w3filecoin-ApiStack

Name	Value
AggregatorApiCustomDomain	https://pr61.aggregator.web3.storage
AggregatorApiEndpoint	https://jokiw6fv8h.execute-api.us-east-2.amazonaws.com
DealerApiCustomDomain	Set HOSTED_ZONE in env to deploy to a custom domain
DealerApiEndpoint	https://gutlk6t0z6.execute-api.us-east-2.amazonaws.com
DealTrackerApiCustomDomain	https://pr61.tracker.web3.storage
DealTrackerApiEndpoint	https://kuiim9nzdf.execute-api.us-east-2.amazonaws.com

pr61-w3filecoin-DataStack

Name	Value
AggregatorAggregateTableName	aggregator-aggregate-store
AggregatorBufferBucketName	aggregator-buffer-store-pr61-0
AggregatorInclusionProofBucketName	aggregator-inclusion-proof-store-pr61-0
AggregatorInclusionTableName	aggregator-inclusion-store
AggregatorPieceTableName	aggregator-piece-store
DealerAggregateStoreTableName	dealer-aggregate-store
DealerOfferStoreBucketName	dealer-offer-store-pr61-0
DealTrackerDealArchiveBucketName	deal-tracker-deal-archive-store-pr61-0
DealTrackerDealStoreTableName	deal-tracker-deal-store

pr61-w3filecoin-AggregatorStack
pr61-w3filecoin-DealerStack
pr61-w3filecoin-DealTrackerStack

alanshaw

I think nothing blocking just some suggestions 🚀

alanshaw · 2023-10-31T16:47:03Z

.env.tpl

@@ -1,8 +1,8 @@
 # These variables are only available in your SST code.

-# uncomment to try out deploying the api under a custom domain.
+# uncomment to try out deploying the `aggregator-api`` under a custom domain.


Suggested change

# uncomment to try out deploying the `aggregator-api`` under a custom domain.

# uncomment to try out deploying the `aggregator-api` under a custom domain.

alanshaw · 2023-10-31T16:48:48Z

package.json

+    "@ucanto/client": "9.0.0",
+    "@ucanto/principal": "9.0.0",
+    "@ucanto/transport": "9.0.0",


Missing hats?

Suggested change

"@ucanto/client": "9.0.0",

"@ucanto/principal": "9.0.0",

"@ucanto/transport": "9.0.0",

"@ucanto/client": "^9.0.0",

"@ucanto/principal": "^9.0.0",

"@ucanto/transport": "^9.0.0",

alanshaw · 2023-10-31T16:54:09Z

packages/core/src/queue/aggregate-offer-queue.js

+  const encodedBytes = JSONencode(aggregateOfferMessage)
+  return {
+    MessageBody: toString(encodedBytes),


I believe dag-json has stringify and parse functions:

import * as dagJSON from '@ipld/dag-json' const str = dagJSON.stringify(aggregateOfferMessage)

alanshaw · 2023-10-31T16:55:43Z

packages/core/src/queue/aggregate-offer-queue.js

+  return {
+    MessageBody: toString(encodedBytes),
+    // FIFO Queue message group id
+    MessageGroupId: options.disableMessageGroupId ? undefined : aggregateOfferMessage.group


I'd argue this is not part of the encoded message and is an option you'd sent when sending it...

I will create an issue to do this in separate: #63

alanshaw · 2023-10-31T16:57:14Z

packages/core/src/queue/piece-accept-queue.js

+export const decodeMessage = (message) => {
+  const decodedBytes = fromString(message.MessageBody)
+  return JSONdecode(decodedBytes)
+}


Seems to be repeated a lot - would be nice to reuse something here?

alanshaw · 2023-11-01T10:29:07Z

packages/functions/src/aggregator/handle-buffer-queue-message.js

+  // if one we should put back in queue
+  if (sqsEvent.Records.length === 1) {
+    return {
+      batchItemFailures: sqsEvent.Records.map(r => ({


What is the difference between this and just throwing and error?

so that we can return 200 and account for metrics in a different way if we would like to. This is because it is possible to receive 1 item in the batch if not enough trhoughput

alanshaw · 2023-11-01T10:30:52Z

packages/functions/src/aggregator/handle-inclusion-insert-to-issue-piece-accept.js

+  if (eventRawRecords.length !== 1) {
+    return {
+      statusCode: 400,
+      body: `Expected 1 DynamoDBStreamEvent per invocation but received ${eventRawRecords.length}`


Suggested change

body: `Expected 1 DynamoDBStreamEvent per invocation but received ${eventRawRecords.length}`

body: `Expected 1 DynamoDB record per invocation but received ${eventRawRecords.length}`

alanshaw · 2023-11-01T10:37:12Z

packages/functions/src/aggregator/handle-inclusion-insert-to-update-state.js

+  const { ok, error } = await aggregatorEvents.handleInclusionInsertToUpdateState(context, record)
+  if (error) {
+    return {
+      statusCode: 500,
+      body: error.message || 'failed to handle inclusion insert event'
+    }
+  }


Not throwing here will remove the stack from the logs, so while you get a nice message in the body, it might be harder to debug.

I'm using this helper (Irakli suggested) to make throwing a little easier:

https://github.com/web3-storage/w3infra/blob/8bdd528a284446aed93db024ee1323aabc122dbf/billing/functions/lib.js#L11-L22

The code above becomes:

Suggested change

const { ok, error } = await aggregatorEvents.handleInclusionInsertToUpdateState(context, record)

if (error) {

return {

statusCode: 500,

body: error.message || 'failed to handle inclusion insert event'

}

}

const ok = expect(

await aggregatorEvents.handleInclusionInsertToUpdateState(context, record),

'failed to handle inclusion insert event'

)

Cool, I will see how to adopt this.
One thing I would like, is to rely on status codes to infer metrics and this would make it difficult unless we explore this a bit further. Will leave in scope of this PR

alanshaw · 2023-11-01T10:38:50Z

packages/functions/src/aggregator/handle-piece-accept-message.js

+  if (sqsEvent.Records.length !== 1) {
+    return {
+      statusCode: 400,
+      body: `Expected 1 sqsEvent per invocation but received ${sqsEvent.Records.length}`


Suggested change

body: `Expected 1 sqsEvent per invocation but received ${sqsEvent.Records.length}`

body: `Expected 1 SQS message per invocation but received ${sqsEvent.Records.length}`

alanshaw · 2023-11-01T10:39:28Z

packages/functions/src/aggregator/handle-piece-message.js

+  if (sqsEvent.Records.length !== 1) {
+    return {
+      statusCode: 400,
+      body: `Expected 1 sqsEvent per invocation but received ${sqsEvent.Records.length}`


Suggested change

body: `Expected 1 sqsEvent per invocation but received ${sqsEvent.Records.length}`

body: `Expected 1 SQS message per invocation but received ${sqsEvent.Records.length}`

sentry-io · 2023-11-02T16:06:59Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ SyntaxError: Non-base32 character vcs-w3filecoin-Aggregator-Consumerbufferqueue03... View Issue
‼️ Error: Unsupported type passed: bafyreifoblfxjiphxpq5jgmn3gjvol7ao4auhzj4kj6dfczph5gskpricq. Pass option... vcs-w3filecoin-Aggregator-Consumeraggregateoffe... View Issue
‼️ HTTPError: HTTP Request failed prod-w3filecoin-Aggregato-Consumeraggregatorinc... View Issue

_{Did you find this useful? React with a 👍 or 👎}

seed-deploy bot had a problem deploying to pr61 October 30, 2023 13:29 Failure

vasco-santos force-pushed the feat/aggregator-service branch from cefd541 to bc0f4f8 Compare October 30, 2023 16:33

seed-deploy bot had a problem deploying to pr61 October 30, 2023 16:33 Failure

vasco-santos force-pushed the feat/aggregator-service branch from bc0f4f8 to 71f31eb Compare October 30, 2023 19:39

seed-deploy bot had a problem deploying to pr61 October 30, 2023 19:39 Failure

vasco-santos force-pushed the feat/aggregator-service branch from 71f31eb to e0ca58e Compare October 31, 2023 08:28

seed-deploy bot had a problem deploying to pr61 October 31, 2023 08:28 Failure

vasco-santos force-pushed the feat/aggregator-service branch from e0ca58e to acef701 Compare October 31, 2023 09:06

seed-deploy bot had a problem deploying to pr61 October 31, 2023 09:06 Failure

feat: aggregator service and events

38650dc

vasco-santos force-pushed the feat/aggregator-service branch from acef701 to 38650dc Compare October 31, 2023 10:35

seed-deploy bot had a problem deploying to pr61 October 31, 2023 10:35 Failure

seed-deploy bot temporarily deployed to pr61 October 31, 2023 10:49 Inactive

vasco-santos marked this pull request as ready for review October 31, 2023 11:04

vasco-santos changed the title ~~feat: aggregator service~~ feat: aggregator service and events Oct 31, 2023

vasco-santos requested a review from alanshaw October 31, 2023 15:11

alanshaw approved these changes Nov 1, 2023

View reviewed changes

vasco-santos mentioned this pull request Nov 1, 2023

Queue messagegroup should be an option instead of encoded message return value #63

Open

seed-deploy bot temporarily deployed to pr61 November 1, 2023 18:20 Inactive

vasco-santos force-pushed the feat/aggregator-service branch from 57b2b20 to 247bac6 Compare November 1, 2023 18:49

seed-deploy bot temporarily deployed to pr61 November 1, 2023 18:49 Inactive

chore: address review comments

e7720a6

vasco-santos force-pushed the feat/aggregator-service branch from 247bac6 to e7720a6 Compare November 1, 2023 18:58

seed-deploy bot temporarily deployed to pr61 November 1, 2023 18:58 Inactive

vasco-santos merged commit e50b866 into main Nov 1, 2023
3 checks passed

vasco-santos deleted the feat/aggregator-service branch November 1, 2023 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: aggregator service and events #61

feat: aggregator service and events #61

vasco-santos commented Oct 30, 2023 •

edited

Loading

seed-deploy bot commented Oct 31, 2023

alanshaw left a comment

alanshaw Oct 31, 2023

alanshaw Oct 31, 2023

alanshaw Oct 31, 2023

alanshaw Oct 31, 2023

vasco-santos Nov 1, 2023 •

edited

Loading

alanshaw Oct 31, 2023

alanshaw Nov 1, 2023

vasco-santos Nov 1, 2023

alanshaw Nov 1, 2023

alanshaw Nov 1, 2023

vasco-santos Nov 1, 2023

alanshaw Nov 1, 2023

alanshaw Nov 1, 2023

sentry-io bot commented Nov 2, 2023 •

edited

Loading

	# uncomment to try out deploying the `aggregator-api`` under a custom domain.
	# uncomment to try out deploying the `aggregator-api` under a custom domain.

	body: `Expected 1 DynamoDBStreamEvent per invocation but received ${eventRawRecords.length}`
	body: `Expected 1 DynamoDB record per invocation but received ${eventRawRecords.length}`

	body: `Expected 1 sqsEvent per invocation but received ${sqsEvent.Records.length}`
	body: `Expected 1 SQS message per invocation but received ${sqsEvent.Records.length}`

feat: aggregator service and events #61

feat: aggregator service and events #61

Conversation

vasco-santos commented Oct 30, 2023 • edited Loading

seed-deploy bot commented Oct 31, 2023

alanshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vasco-santos Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sentry-io bot commented Nov 2, 2023 • edited Loading

Suspect Issues

vasco-santos commented Oct 30, 2023 •

edited

Loading

vasco-santos Nov 1, 2023 •

edited

Loading

sentry-io bot commented Nov 2, 2023 •

edited

Loading