replace DynamoDB Streams for the notifications-lambda with SQS and a AppSync pipeline resolver

[twrichards]

Co-Authored-By: @andrew-nowak

https://trello.com/c/VzkeAK2f/554-migrate-pinboard-to-relational-database

In the database ADR (specifically

pinboard/ADRs/database.md

Line 39 in ce8309a

This addresses all of the limitations outlined above. The primary concern was how to replicate the behaviour we get from 'DynamoDB Streams' (to invoke the `notifications-lambda` on inserts into the Item table), fortunately [RDS Aurora supports invoking lambdas directly from within the DB engine](https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/PostgreSQL-Lambda.html) and is in fact better, because we can perform all the joins and filters in the DB engine (in the insert trigger logic) to build the lambda payload (so the lambda needn't do queries back to the database - as it does at the moment) and conditionally invoke.

) we had stated that we could replace 'DynamoDB Streams' with an invoke_lambda (from within the Aurora DB engine) when we move from Dynamo to RDS. However upon further investigation/experimentation, due to the fact we must use Aurora ServerlessV1 (because it's the only thing which supports the data-api, which AppSync relies upon) this doesn't support attaching IAM roles and so we cannot permission the RDS cluster to invoke the lambda - putting an end to that approach. As the changes to the database ADR in this PR explain, that leaves us with two choices...

• add a lambda and RDS proxy between AppSync and RDS (as explained in https://aws.amazon.com/blogs/mobile/appsync-graphql-sql-rds-proxy) - this seems like too much infrastructure complexity (at this point)
• convert the createItem AppSync resolver to an AppSync 'pipeline' resolver, where the first function does the DB insert as before, then the second function invokes the lambda (and we leave the lambda to look-up what it needs to from the DB, a shame but worth it) - this is the solution we went for in this PR and can be done before any re-platforming from DynamoDB to RDS (to make that task simpler later on).

What does this change?

• convert the existing createItem resolver (defined in CDK) into a 'resolver function'
• add notifications-lambda as an AppSync 'data source' and add a 'resolver function' to invoke it, with the output of the insert (which includes the inserted item's generated ID)
• wire the above together into a 'pipeline' resolver, which is assigned to the createItem Mutation
• update the notifications-lambda to receive the payload sent from the new 'resolver function' above (rather than the 'DynamoDB Streams' payload) and then queue it on an SQS queue, so that the pipeline resolver can return quickly (and the user/client isn't waiting on all the notifications being sent before they know their message has been inserted)...
• ... add an SQS queue to receive items which need notifications sending, which is wired up to the notifications-lambda...
• ... have the notifications-lambda also be able to be invoked with an SQSEvent (containing the item that it has just queued) to then do roughly what it used to (i.e. lookup which users need to receive a push notification for that item and send out all the push notifications)

How to test

With this branch deployed to CODE (you'll need to check the pipeline resolver gets attached OK, as AWS' cloudformation of AppSync things is buggy, see aws/aws-appsync-community#146 (comment) for example)...

• ensure you've subscribed to desktop notifications in pinboard (you'll see a button for it at the top of the pinboard panel if you haven't already subscribed)
• send a message mentioning yourself (mention people by typing @ and selecting yourself)
  - ... you should still receive a desktop notification

How can we measure success?

This is kind of a no-op (functionality wise) but removes this complexity from the process of re-platforming from DynamoDB to RDS (where otherwise we would've had to do the lambda_invoke thing along the way.

Have we considered potential risks?

There will be a performance hit when sending a message (more so when the notifications-lambda is cold), but from our testing this is tolerable.

[twrichards]

Closing, since this approach is much more convoluted and the increased response time of this pipeline resolver is not really acceptable (when someone just wants to know their message is sent) - this decision is bolstered by anecdotal reports of woeful latency overhead of the data-api.