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

Concurrent updates that don't interfere with each other #4224

Closed
nikgraf opened this issue Nov 11, 2020 · 8 comments
Closed

Concurrent updates that don't interfere with each other #4224

nikgraf opened this issue Nov 11, 2020 · 8 comments
Assignees
Milestone

Comments

@nikgraf
Copy link

nikgraf commented Nov 11, 2020

Problem

I need to get the first entry of a list of oneTimeKeys for a specific user and update it to be connected to the user that “claims” it.

What I did until now:

const oneTimeKeyResult = await prisma.oneTimeKey.findMany({ // could be findFirst
  where: {
    user: { id: userId },
    claimedByUserId: { equals: null },
  },
  take: 1,
});
const oneTimeKey = await prisma.oneTimeKey.update({
  where: { id: oneTimeKeyResult[0].id },
  data: {
    claimedByUser: { connect: { id: currentUser.id } },
  },
});

The problem here is that if two or more users try to claim the same oneTimeKey at the same time they can can and it actually happens sometimes in production.

Suggested solution

Running the find and update in a transaction would be nice, but this is not supported currently.

Alternatives

Since I'm running on Postgres (if I understand correctly) I could do a executeRaw with an update using FOR UPDATE SKIP LOCKED

https://stackoverflow.com/a/56441907/837709
https://dba.stackexchange.com/questions/69471/postgres-update-limit-1

@mavilein
Copy link
Contributor

Hey @nikgraf ,

thanks for reaching out. Can you also provide your prisma schema with the 2 models? Wanna be sure I have the right relations in mind.

@muxahuk
Copy link
Contributor

muxahuk commented Nov 11, 2020

as far as i understand this scenario can happen only when user is one to many relation. So we have many users that can claim one key.

const oneTimeKey = await prisma.oneTimeKey.update({
  where: {
    id: oneTimeKeyResult[0].id,
    claimedByUserId: { equals: null },
  },
  data: {
    claimedByUser: { connect: { id: currentUser.id } },
  },
});

this should fix ur issue.. adding claimedByUserId: { equals: null }, to the update - should fail to update the record if it has been already updated (i.e. won't be able to find one).

Not sure thou if generated client allows passing that property to the where condition..

@nikgraf
Copy link
Author

nikgraf commented Nov 11, 2020

sure @mavilein

model OneTimeKey {
  id                String   @id @default(uuid())
  claimedByUser     User?  @relation(name: "ClaimedUser", fields: [claimedByUserId], references: [id])
  claimedByUserId   String?
}

model User {
  id                 String               @id @default(uuid())
  claimedOneTimeKeys OneTimeKey[]         @relation(name: "ClaimedUser")
}

(I have to add I changed some names and simplified the Models to focus on the issue)

@muxahuk I haven't tried, maybe it's just wrongly typed, but there where only accepts an id property.

@nikgraf
Copy link
Author

nikgraf commented Nov 11, 2020

Then I though updateMany would help, but it doesn't allow to update the relationship:

const oneTimeKey = await prisma.oneTimeKey.updateMany({
  where: {
    id: oneTimeKeyResult[0].id,
    claimedByUserId: { equals: null },
  },
  data: {
    claimedByUser: { connect: { id: currentUser.id } }, // THIS IS NOT ALLOWED
  },
});

@muxahuk
Copy link
Contributor

muxahuk commented Nov 12, 2020

yep, can confirm that there's no way to do what u want using generated prisma client.

but with some tricks i was able to achieve a workaround:

  1. we need to add new column to table that would be not null and by default = 0
  2. we need to add constraint on that column so that it is less then or equal to 1
  3. when updating key we also increment that column by 1 (using atomic update)

I was testing on sqlite and here's what i added to the end of table creation (usually we can use alter table to add check/constraint)

"connectedTimes" INTEGER NOT NULL DEFAULT 0,
 CHECK("connectedTimes" <= 1) // we should also do "AND "connectedTimes" >= 0" but for test enaugh

then i modified update call to:

await prisma.oneTimeKey.update({
		where: {
			id: keyId,
		},
		data: {
			connectedTimes: { // here we use atomic update
				increment: 1, 
			},
			claimedByUser: {
				connect: {
					id: userId,
				},
			},
		},
	});

so, basically what we are trying to achieve is so that whenever we try to connect a user we also increment "connectedTimes", if we try to do that 2nd time - it will fail bcs of check/constraint we added before resulting in error that we can catch and retry with different key or reply to user that error...

only downside that there is - when we disconnect user or delete - we should reset "connectedTimes" to 0 so that new user could be connected. That should be achievable via trigger (on db level) or somewhere in your application logic

PS: this seems over engineered. Not DB specialist, but if we could add a check/constraint on "claimedByUser" column so that once it is set it can not be changed to something else then what it is now or null.. i guess can be achieved with trigger..

@nikolasburk
Copy link
Member

nikolasburk commented Nov 12, 2020

Then I though updateMany would help, but it doesn't allow to update the relationship:

const oneTimeKey = await prisma.oneTimeKey.updateMany({
  where: {
    id: oneTimeKeyResult[0].id,
    claimedByUserId: { equals: null },
  },
  data: {
    claimedByUser: { connect: { id: currentUser.id } }, // THIS IS NOT ALLOWED
  },
});

Ah sorry, this is indeed not possible. However, with the new preview feature uncheckedScalarInput which allows to set foreign keys directly this would work:

await prisma.post.updateMany({
  where: {
    id: oneTimeKeyResult[0].id,
    claimedByUserId: { equals: null }, 
  },
  data: {
    claimedByUserId: currentUser.id // this is possible since v2.11.0
  }
})

To use this feature, you need to set the uncheckedScalarInput on the Prisma Client generator:

generator client {
  provider = "prisma-client-js"
  previewFeatures = ["uncheckedScalarInputs"]
}

Don't forget to run npx prisma generate afterwards (sometimes I also need to rm -rf node_modules for the latest version to show up).

@nikgraf
Copy link
Author

nikgraf commented Nov 12, 2020

@nikolasburk ahh nice, makes a lot of sense.

Just for the record in the end I went for a raw query since the queries above would have required me to write my own retry logic. If I understand correctly using FOR UPDATE SKIP LOCKED for Postgres it's not necessary.

This post explains how queues with Postgres are possible using FOR UPDATE SKIP LOCKED https://www.2ndquadrant.com/en/blog/what-is-select-skip-locked-for-in-postgresql-9-5/

@matthewmueller
Copy link
Contributor

Glad to hear you found a solution! I think you could also use the OCC pattern for cases like this: https://www.prisma.io/docs/guides/prisma-guides/prisma-client-transactions-guide#optimistic-concurrency-control

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

No branches or pull requests

8 participants