@WebDevCody relying on the time passed from the app might cause a double reservation 'once in a million, i.e. next tuesday' due to clock skew between the servers. Those race conditions would also typically occur when you have high traffic spike and therefore there's a high chance your servers and network will be overloaded and then it could in theory take more than 5 seconds between 'now = new Date' and the final update statement
@WebDevCody yes, using now() + interval '5 seconds' (or whatever the correct syntax is in postgres) I would also go with insert first and if it fails do the update. with the check first, your insert can still fail so it really brings no value however, as you mentioned in the video 5 seconds is a bit short. most of the booking systems I've seen would block the seat for minutes, so that you can finish your add-ons and payment process and in that scenario clock skew or system being busy are much less likely to cause problems
@@tajkris it's still a potential race condition, even though it might have a window size of milliseconds, it may still occur. thanks for pointing it out
Depending on what DB you are using I think you can go with repeatable read instead of serializable as you arw not doing a range query and you probably should not worry about phantom reads. I think this way the code will be cleaner and shorter. The issue with optimistic concurrency control is that in the short period between checking if somebody has modified the record and actually updating it, there can be an update which is missed
You should be able to create some kind of database constraint that backs up this code and makes it impossible to have multiple people booking the same seat at the same time. A unique index across multiple columns may do the trick. depending on your schema.
I am not sure I understand why you need a transaction here. As you pointed out the part that prevent multiple updates is the where clause. Correct me if I am wrong but a transaction is useful when you try to make multiple queries and revert the whole thing if one of them fails. I guess to make sure a seat is reserved only once, you could use the seatId as a unique value for a row in some seat_reservation table. Once the reserveration expires you could use a TRIGGER to delete that row. Again not sure that is best practice either.
I think you are correct. I looked into this more and I think this can also be achieved using upsert const now = new Date(); const expiresAt = new Date(now.getTime() + RESERVATION_TIME); const updatedSeats = await db .insert(seats) .values({ seatId: requestBody.seatId, userId: requestBody.userId, expiresAt, }) .onConflictDoUpdate({ target: seats.seatId, set: { userId: requestBody.userId, expiresAt, }, where: and( eq(seats.seatId, requestBody.seatId), lte(seats.expiresAt, now) ), }) .returning({ seatId: seats.seatId }); if (updatedSeats.length === 0) { throw new Error("seat is reserved, try again later"); } return Response.json({ message: `successfully reserved seat for ${RESERVATION_TIME} ms`, });
@@WebDevCody I guess you can do an UPSERT or two queries for INSERT and UPDATE. For the UPSERT to work, seatId needs to be UNIQUE on the table so the INSERT fails and the ON CONFLICT is triggered.
Nice video. Just curious why you mentioned versioning separetly from your current implementation. The latter seems like versioning using time. Maybe because the prior is more general solution?
Why do you need the `lte()` comparison at line 33? You are doing this comparison at the line 23. I thought that if you encapsulated your whole code into a DB transaction, the line 23 is enough because the DB would block somehow tables during the transaction. Or just the condition on line 23 is redundant because the line 33 deals with the same condition?
Frankly, I do not know what your like conveys here. Does it say that the statement "I thought that if you encapsulated your whole code into a DB transaction, the line 23 is enough because the DB would block somehow tables during the transaction." is true? Or does it say that this one "Or just the condition on line 23 is redundant because the line 33 deals with the same condition?" is true?
I like how you use the lock to query the entity. Another idea is successive posts to acquire a lock with the same user will increase the timeout, but that depends on use case.
What's the purpose of the if clause at 23-25 since you're already throwing an error for future expirestAt via the where clause? Is this just an early return to prevent unnecessary db queries?
there where clause does not throw errors, it will update all records in the database that match the where; therefore, it'll return an array of affected items (or an empty list)
I learned a lot from your video, so thanks for doing it. You asked for feedback and one thing that struck me is that it looks like you're using exceptions for control flow which is generally considered an antipattern. Use exceptions for things that you absolutely can't handle. Exceptions are a bit too much like 'goto' statements and have the same drawbacks.
Exceptions from the DB give you atomicity. You can not check and then insert if the check passes because the state could have changed after the check. Exceptions are perfect
A queue sounds like extra complexity but it all depends on traffic. You’d probably end up needing a separate queue for each individual flight and process the messages one at a time. Maybe if you have tons of users you may need to add a queue system to prevent bringing your database down
I feel like all of these problems can be solved by simply using a queue. That will ensure that each request happens one at a time, and then when the seats are filled, just update the DB. Once the DB is updated, go to the next person in queue and if their request is the same seat, there is no chance whatsoever of them overlapping the same seat. It’s also very performant and scales very well. You could even introduce concurrency models that check are queues for specific areas of the seating if you truly wanted to get fancy.
That said, a message broker might be too complex to implement for certain projects that don’t truly need one or don’t want to add the complexity to the app for whatever reason. In that case, I think your solution is perfect fit because it ensures that only one request will be updating the DB at a time (in terms of how node is accessing it at least), drastically reducing the chances of a race condition!
If we push that request to the queue then what we return to the user? Because we are not sure yet about the reservation will be successful or not when their entity will be processed by the queue
@@thesemicolon1971 you can use polling (basically client fetches status every once in a while to check if it is fullfilled) another solution is using Notification service (i.e SNS) that notify your client that it has been fulfilled or just send it the API client needs to fetch (because there is update on it)
Learning ACID rules for storing anything in a databases is very crucial, same with understanding and creating database tables using 3rd normal form
Great video, glad you mentioned isolation levels and versioning as a possible solution.
@WebDevCody relying on the time passed from the app might cause a double reservation 'once in a million, i.e. next tuesday' due to clock skew between the servers. Those race conditions would also typically occur when you have high traffic spike and therefore there's a high chance your servers and network will be overloaded and then it could in theory take more than 5 seconds between 'now = new Date' and the final update statement
Good point, the date should be calculated inside the db query you are saying?
@WebDevCody yes, using now() + interval '5 seconds' (or whatever the correct syntax is in postgres)
I would also go with insert first and if it fails do the update. with the check first, your insert can still fail so it really brings no value
however, as you mentioned in the video 5 seconds is a bit short. most of the booking systems I've seen would block the seat for minutes, so that you can finish your add-ons and payment process and in that scenario clock skew or system being busy are much less likely to cause problems
@@tajkris it's still a potential race condition, even though it might have a window size of milliseconds, it may still occur. thanks for pointing it out
Depending on what DB you are using I think you can go with repeatable read instead of serializable as you arw not doing a range query and you probably should not worry about phantom reads. I think this way the code will be cleaner and shorter. The issue with optimistic concurrency control is that in the short period between checking if somebody has modified the record and actually updating it, there can be an update which is missed
You should be able to create some kind of database constraint that backs up this code and makes it impossible to have multiple people booking the same seat at the same time. A unique index across multiple columns may do the trick. depending on your schema.
This would help me a lot in my side project I mentioned in your last video . Thank you so much
Can this example be applied if two people are trying to buy the same product assuming there is only one quantity of the product in the inventory.
I am not sure I understand why you need a transaction here. As you pointed out the part that prevent multiple updates is the where clause. Correct me if I am wrong but a transaction is useful when you try to make multiple queries and revert the whole thing if one of them fails.
I guess to make sure a seat is reserved only once, you could use the seatId as a unique value for a row in some seat_reservation table. Once the reserveration expires you could use a TRIGGER to delete that row. Again not sure that is best practice either.
I think you are correct. I looked into this more and I think this can also be achieved using upsert
const now = new Date();
const expiresAt = new Date(now.getTime() + RESERVATION_TIME);
const updatedSeats = await db
.insert(seats)
.values({
seatId: requestBody.seatId,
userId: requestBody.userId,
expiresAt,
})
.onConflictDoUpdate({
target: seats.seatId,
set: {
userId: requestBody.userId,
expiresAt,
},
where: and(
eq(seats.seatId, requestBody.seatId),
lte(seats.expiresAt, now)
),
})
.returning({ seatId: seats.seatId });
if (updatedSeats.length === 0) {
throw new Error("seat is reserved, try again later");
}
return Response.json({
message: `successfully reserved seat for ${RESERVATION_TIME} ms`,
});
@@WebDevCody I guess you can do an UPSERT or two queries for INSERT and UPDATE. For the UPSERT to work, seatId needs to be UNIQUE on the table so the INSERT fails and the ON CONFLICT is triggered.
Nice video. Just curious why you mentioned versioning separetly from your current implementation. The latter seems like versioning using time. Maybe because the prior is more general solution?
One thing I would like to see is if we want to check reservation history by user or by seat. I would like to see that from tables perspective.
what vs code theme are you using? I really loved it.
Bearded theme stained blue
Why do you need the `lte()` comparison at line 33? You are doing this comparison at the line 23. I thought that if you encapsulated your whole code into a DB transaction, the line 23 is enough because the DB would block somehow tables during the transaction. Or just the condition on line 23 is redundant because the line 33 deals with the same condition?
Frankly, I do not know what your like conveys here.
Does it say that the statement "I thought that if you encapsulated your whole code into a DB transaction, the line 23 is enough because the DB would block somehow tables during the transaction." is true?
Or does it say that this one "Or just the condition on line 23 is redundant because the line 33 deals with the same condition?" is true?
I love this kind of videos. Liked!
I like how you use the lock to query the entity. Another idea is successive posts to acquire a lock with the same user will increase the timeout, but that depends on use case.
Ive never been able to understand the concept of isolation level, especially when there is a lock! 5:06
th-cam.com/video/W5FFiI5ALTc/w-d-xo.html
wooww i Just facing the same issue, i was using knexjs and forUpdate with retry on deadlock response
what theme do you use
What's the purpose of the if clause at 23-25 since you're already throwing an error for future expirestAt via the where clause? Is this just an early return to prevent unnecessary db queries?
there where clause does not throw errors, it will update all records in the database that match the where; therefore, it'll return an array of affected items (or an empty list)
I learned a lot from your video, so thanks for doing it. You asked for feedback and one thing that struck me is that it looks like you're using exceptions for control flow which is generally considered an antipattern. Use exceptions for things that you absolutely can't handle. Exceptions are a bit too much like 'goto' statements and have the same drawbacks.
Exceptions from the DB give you atomicity. You can not check and then insert if the check passes because the state could have changed after the check. Exceptions are perfect
Yeah I think in some cases you have no choice but to catch a db exception. Maybe there was a different way to structure this code to not need it.
What are the pros/cons of doing it this way, instead of setting it up via a queue of some kinda in say AWS?
A queue sounds like extra complexity but it all depends on traffic. You’d probably end up needing a separate queue for each individual flight and process the messages one at a time. Maybe if you have tons of users you may need to add a queue system to prevent bringing your database down
can you share the code
As someone who rarely codes (by following a tutorial) this title was wild.😂
What’s wild about it?
Good job babe!!!! First! 👸🏿
I feel like all of these problems can be solved by simply using a queue. That will ensure that each request happens one at a time, and then when the seats are filled, just update the DB. Once the DB is updated, go to the next person in queue and if their request is the same seat, there is no chance whatsoever of them overlapping the same seat. It’s also very performant and scales very well. You could even introduce concurrency models that check are queues for specific areas of the seating if you truly wanted to get fancy.
That said, a message broker might be too complex to implement for certain projects that don’t truly need one or don’t want to add the complexity to the app for whatever reason. In that case, I think your solution is perfect fit because it ensures that only one request will be updating the DB at a time (in terms of how node is accessing it at least), drastically reducing the chances of a race condition!
If we push that request to the queue then what we return to the user? Because we are not sure yet about the reservation will be successful or not when their entity will be processed by the queue
@@Sindokucan you pls explain more in the context of my above comment
That’s the issue with a queue, you now need to add websockets or polling so the user knows when or if their request failed
@@thesemicolon1971 you can use polling (basically client fetches status every once in a while to check if it is fullfilled) another solution is using Notification service (i.e SNS) that notify your client that it has been fulfilled or just send it the API client needs to fetch (because there is update on it)
might look like*
Thank you
Amazing concept, shitty code.