-
Notifications
You must be signed in to change notification settings - Fork 59
Merge 3rd stream #8
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
base: main
Are you sure you want to change the base?
Merge 3rd stream #8
Conversation
| const requestHash = hashArguments(sender_id, recipient_id, amount); | ||
|
|
||
| const redisCheckResult = await checkRedisHash(sender_id, requestHash); | ||
|
|
||
| if (!redisCheckResult.success) { | ||
| return { | ||
| success: false, | ||
| error: 'Duplicate transaction', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uncoooloj this is where I used the implementation for #3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seen it, what if transactions are just a few milli seconds apart?
That's my fear. The system won't be done with this IO so both will pass.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, @uncoooloj Redis also has implementations for transactions and optimistic locking.
I'll update it in the final version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so quick question, what if there was a request throttling to one request per 10 ms for example? sounds like a lazy hack but that + a transaction has solves our problem no?
| const addElement = promisify(client.sadd).bind(client); | ||
| const expireElement = promisify(client.expire).bind(client); | ||
|
|
||
| async function checkRedisHash(accountId, hash) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uncoooloj this is the implementation for #3.
No description provided.