baseline icon indicating copy to clipboard operation
baseline copied to clipboard

SRI - Introduce Prisma transactions

Open ognjenkurtic opened this issue 1 year ago • 8 comments

Overview

We are currently not working with transactions, meaning that a failed execution of a command could leave the db in a incomplete state. In order to allow for transactions, we need to refactor the current approach we do persistence in a way that all db updates are collected and commited and the end of the execution of a command handler.

This is one approach we can use : https://www.prisma.io/docs/guides/performance-and-optimization/prisma-client-transactions-guide#scenario-pre-computed-ids-and-the-transaction-api

We precompute Ids, collect all storage actions from the relevant storage agents and then execute a single prisma transaction at the end of the command handler. Best way to achieve this is to have a provider called i.e dbContext, which is scoped as REQUEST that is injected in every agent and serves as the place where we collect all the db actions created by storage agents which are invoked by regular agents. This dbContext is in the end passed to prisma.transaction call, so that db actions are executed in order as part of a single transaction.

Reference

Given above

Questions

  • Any drawbacks to this approach? Are we ok with assigning ids in app for every type of domain object?

Acceptance

  • New approach is applied to a single command handler that performs multiple db updates
  • New approach is unit tested both for success and failuer scenarios

Tasks

  • [ ] Research and validate the above approach
  • [ ] Implement on a level of a single command handler
  • [ ] Write tests

ognjenkurtic avatar Sep 10 '23 20:09 ognjenkurtic

NestJS Scope hierarchy The REQUEST scope bubbles up the injection chain. A controller that depends on a request-scoped provider will, itself, be request-scoped.

Imagine the following dependency graph: CatsController <- CatsService <- CatsRepository. If CatsService is request-scoped (and the others are default singletons), the CatsController will become request-scoped as it is dependent on the injected service. The CatsRepository, which is not dependent, would remain singleton-scoped.

Transient-scoped dependencies don't follow that pattern. If a singleton-scoped DogsService injects a transient LoggerService provider, it will receive a fresh instance of it. However, DogsService will stay singleton-scoped, so injecting it anywhere would not resolve to a new instance of DogsService. In case it's desired behavior, DogsService must be explicitly marked as TRANSIENT as well.

nj-request-scope

Kasshern avatar Oct 19 '23 13:10 Kasshern

What is the issue if the controller ends up request scoped because an agent injects a dbContext which is also request scoped?

Could we just use the default SINGLETON scope for the dbContext? How would it behave when there are multiple concurrent requests?

ognjenkurtic avatar Oct 24 '23 07:10 ognjenkurtic

What is the issue if the controller ends up request scoped because an agent injects a dbContext which is also request scoped?

For a request-scoped controller, a new instance is created for each inbound request, and garbage-collected when the request has completed processing.

Could we just use the default SINGLETON scope for the dbContext? How would it behave when there are multiple concurrent requests?

Its a good question. With prisma transaction implementation we will probably avoid race condition-like problem with accessing and altering stuff in the DB, I guess there is no guarantee of the order of the concurrent requests. hopefully that is the worst of it. I suppose it doesn't do much to hurt our current implementation, but would inevitably need a Queue-ish tool. Only one way to find out.

Kasshern avatar Oct 26 '23 13:10 Kasshern

For a request-scoped controller, a new instance is created for each inbound request, and garbage-collected when the request has completed processing.

Exactly, so the only issue could be that your memory footprint is larger in high-load scenarios, right? This is not something we need to worry about right now in my eyes

ognjenkurtic avatar Oct 26 '23 20:10 ognjenkurtic

Update on this issue:

  1. Current work done on prisma transactions branch B
  2. There are known issues when doing a prisma transaction and creating two entities in the DB when those entities have a foreign key constraint between them, even if ids are pre-calculated. prisma issue here
  3. As far as I can tell work around is to disable referential integrity in the DB - NoAction referential action
  4. Nested writes could still work - currently there are only two places in the repo that have multiple DB writes needing atomicity. Each Nested write basically require a separate unique query to be created unless storage agent further abstracted.
  5. Interactive transactions are also an option but have their downsides seen here
  6. FYI Modules that have circular dependencies and using sing request-scoped providers in combination with circular dependencies is very unpredictable.

Kasshern avatar Nov 30 '23 14:11 Kasshern

@Kasshern is there any other place where we need to apply the agreed approach or this one can be closed?

ognjenkurtic avatar Feb 28 '24 21:02 ognjenkurtic

@Kasshern please check the question above. Would like to close this one if possible.

ognjenkurtic avatar Jun 20 '24 14:06 ognjenkurtic

Closing due to inactivity. Please reopen if anything is left to cover with this topic.

ognjenkurtic avatar Jul 18 '24 20:07 ognjenkurtic