feat: handle messages concurrently in pickup worker #119

olizilla · 2023-03-08T22:23:14Z

switch to an sqs lib that polls for new messages concurrently rather than in batches. This is rad as now we'll make better use of each container!
treat timeouts as a regular failure. Let the message go back on the queue for another node to try. After 3 goes it'll go to the dead letter queue and be marked as failed. This is fine, and simplifies the pickup worker a lot, as it doesn't need to talk to dynamo or determine the cause of an error.
rewrite pickup worker so we can compose it out of single-responsibility pieces instead of having to pass through the giant config ball. It's so much simpler now! You can figure our what it does from it's parts! sqsPoller + carFetcher + s3Uploader

  const pickup = createPickup({
    sqsPoller: createSqsPoller({
      queueUrl: SQS_QUEUE_URL,
      maxInFlight: BATCH_SIZE
    }),
    carFetcher: new CarFetcher({
      ipfsApiUrl: IPFS_API_URL,
      fetchTimeoutMs: TIMEOUT_FETCH
    }),
    s3Uploader: new S3Uploader({
      bucket: VALIDATION_BUCKET
    })
  })

see: https://github.com/PruvoNet/squiss-ts/

fixes #13
fixes #116
fixes #101

License: MIT

WIP - switch to an sqs lib that polls for new messages concurrently rather than in batches - rewrite pickup worker so we can compose it out of single-responsibilty pieces instead of having to pass through the giant config ball. License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

github-actions · 2023-03-08T22:23:30Z

`package-lock.json` changes

Click to toggle table visibility

Name	Previous	Current
`@eslint/js`	-	8.35.0
`@humanwhocodes/module-importer`	-	1.0.1
`builtins`	-	5.0.1
`eslint-plugin-n`	-	15.6.1
`esquery`	1.4.0	1.5.0
`globals`	13.18.0	13.20.0
`grapheme-splitter`	-	1.0.4
`js-sdsl`	-	4.3.0
`linked-list`	-	2.1.0
`squiss-ts`	-	4.4.1
`ts-type-guards`	-	0.7.0

seed-deploy · 2023-03-08T22:32:27Z

View stack outputs

pr119-pickup-BasicApiStack

Name	Value
ApiEndpoint	https://ojhw356dr0.execute-api.us-west-2.amazonaws.com
CustomDomain	https://pr119.pickup.dag.haus
S3EventsTopicARN	arn:aws:sns:us-west-2:505595374361:pr119-pickup-S3Events

pr119-pickup-PickupStack

Name Value

ServiceSQSQueue88C16836 pr119-pickup-Pin

ServiceSQSQueueArnDDC422AE arn:aws:sqs:us-west-2:505595374361:pr119-pickup-Pin

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

test the api is available when start is called. License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

olizilla · 2023-03-09T17:05:49Z

Dockerfile

put less npm noise in the logs

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

fixes #101 License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy · 2023-03-14T11:15:34Z

Stack outputs updated

pr119-pickup-BasicApiStack

Name	Value
ApiEndpoint	https://ojhw356dr0.execute-api.us-west-2.amazonaws.com
CustomDomain	https://pr119.pickup.dag.haus
S3EventsTopicARN	arn:aws:sns:us-west-2:505595374361:pr119-pickup-S3Events

pr119-pickup-PickupStack

Name	Value
ServiceSQSQueue88C16836	pr119-pickup-Pin
ServiceSQSQueueArnDDC422AE	arn:aws:sqs:us-west-2:505595374361:pr119-pickup-Pin
ServiceValidatorSQSQueue5667BEDA	pr119-pickup-ValidationPinQueue
ServiceValidatorSQSQueueArnCF57A57B	arn:aws:sqs:us-west-2:505595374361:pr119-pickup-ValidationPinQueue

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

2 mins is still timeout after node restarts License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

vasco-santos

LGTM! Great work @olizilla , as you mentioned things got way simpler and make better use of each container 🙌🏼 Also nice finding with squiss SQS Poller, much nicer!

Leaving some non blocking feedback below. Would also be great if follow up PRs would be more scoped, instead of such big code changes when possible.

vasco-santos · 2023-03-14T14:11:31Z

README.md

+2/3rs of home internet users can upload faster than 20Mbit/s (fixed broadband), at which 32GiB would transfer in 3.5hrs.
+
+see: https://www.speedtest.net/global-index
+see: https://www.omnicalculator.com/other/download-time?c=GBP&v=fileSize:32!gigabyte,downloadSpeed:5!megabit


Shouldn't this be a note in the subsection above? ### FETCH_TIMEOUT_MS

vasco-santos · 2023-03-14T14:13:30Z

validator/package.json

@@ -16,12 +16,12 @@
    "@aws-sdk/lib-dynamodb": "^3.112.0",
    "@aws-sdk/lib-storage": "^3.97.0",
    "@ipld/car": "5.1.0",
-    "async-retry": "1.3.3",


cool that we moved to p-* 👍🏼

vasco-santos · 2023-03-14T14:22:20Z

pickup/lib/ipfs.js

 */
-export async function fetchCar (cid, ipfsApiUrl, downloadError, timeoutMs = 30000, downloadStatusManager) {
+export async function fetchCar ({ cid, ipfsApiUrl, signal }) {


Should we do default for ipfsApiUrl like below?

vasco-santos · 2023-03-14T14:31:48Z

pickup/lib/pickup.js

+        if (!msg.isHandled) {
+          await msg.release() // back to the queue, try again
+        }


looks like we do release everywhere? Should we put this after the else instead?

i'm likely to bring back "directly update pin status in db and fail where dag is too big" so left it so that each condition decides what to do with the message.

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 8, 2023 22:23 Inactive

chore: tests pass

49b0ab3

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 9, 2023 12:51 Inactive

chore: docs

7bc7d91

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 9, 2023 13:48 Inactive

chore: fix init order and docker test

4342ef8

test the api is available when start is called. License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 9, 2023 14:55 Inactive

chore: fix test race

6a9a8f5

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 9, 2023 16:39 Inactive

olizilla requested a review from vasco-santos March 9, 2023 17:02

olizilla marked this pull request as ready for review March 9, 2023 17:02

olizilla changed the title ~~feat: handle concurrent batches~~ feat: handle messages concurrently Mar 9, 2023

olizilla commented Mar 9, 2023

View reviewed changes

Dockerfile

CMD [ "npm", "start", "-w", "pickup", "--silent"]

Copy link

Contributor Author

olizilla Mar 9, 2023

Choose a reason for hiding this comment

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

put less npm noise in the logs

olizilla changed the title ~~feat: handle messages concurrently~~ feat: handle messages concurrently in pickup worker Mar 9, 2023

olizilla requested a review from alanshaw March 9, 2023 17:09

chore: revert err and remove unused fn§

b747064

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 9, 2023 21:44 Inactive

olizilla added 2 commits March 13, 2023 11:50

fix: add max timeout for fetching

027943c

fixes #101 License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

fix: add tests. fix bugs

e621bd2

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 13, 2023 20:17 Inactive

chore: not testing message count

8ed4f00

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 13, 2023 22:02 Inactive

olizilla added this to the w3up phase 3 milestone Mar 14, 2023

olizilla mentioned this pull request Mar 14, 2023

Rollout for dotStorage #68

Closed

10 tasks

chore: only set optional env if set

f08cf6b

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 14, 2023 11:08 Inactive

chore: IPFS_API_URL is not required

5a83796

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 14, 2023 11:26 Inactive

chore: bump up fetchChunkTimeoutMs

4b6a00c

2 mins is still timeout after node restarts License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 14, 2023 11:43 Inactive

chore: add origins to logs

9f7dc1b

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 14, 2023 12:55 Inactive

vasco-santos approved these changes Mar 14, 2023

View reviewed changes

chore: fix readme

20f6b35

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr119 March 14, 2023 15:38 Inactive

olizilla merged commit 92935f8 into main Mar 14, 2023

olizilla deleted the concurrently branch March 14, 2023 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: handle messages concurrently in pickup worker #119

feat: handle messages concurrently in pickup worker #119

olizilla commented Mar 8, 2023 •

edited

Loading

github-actions bot commented Mar 8, 2023 •

edited

Loading

seed-deploy bot commented Mar 8, 2023

olizilla Mar 9, 2023

seed-deploy bot commented Mar 14, 2023

vasco-santos left a comment

vasco-santos Mar 14, 2023

vasco-santos Mar 14, 2023

vasco-santos Mar 14, 2023

vasco-santos Mar 14, 2023

olizilla Mar 14, 2023

feat: handle messages concurrently in pickup worker #119

feat: handle messages concurrently in pickup worker #119

Conversation

olizilla commented Mar 8, 2023 • edited Loading

github-actions bot commented Mar 8, 2023 • edited Loading

package-lock.json changes

seed-deploy bot commented Mar 8, 2023

olizilla Mar 9, 2023

Choose a reason for hiding this comment

seed-deploy bot commented Mar 14, 2023

vasco-santos left a comment

Choose a reason for hiding this comment

vasco-santos Mar 14, 2023

Choose a reason for hiding this comment

vasco-santos Mar 14, 2023

Choose a reason for hiding this comment

vasco-santos Mar 14, 2023

Choose a reason for hiding this comment

vasco-santos Mar 14, 2023

Choose a reason for hiding this comment

olizilla Mar 14, 2023

Choose a reason for hiding this comment

olizilla commented Mar 8, 2023 •

edited

Loading

github-actions bot commented Mar 8, 2023 •

edited

Loading

`package-lock.json` changes