-
Notifications
You must be signed in to change notification settings - Fork 102
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
Atomically downgrade lock when committing tx to prevent deadlock #1252
Conversation
…e lock when committing tx to prevent deadlock
Ack it. Here is another visualisation focusing on main part
So, the reducer 2 lock will not be acquired but It will block the queue (further reads) due to fairness. |
todo!("Write skew, you need to implement retries my man, T-dawg."); | ||
} | ||
} | ||
Ok(Ok(())) => EventStatus::Committed(DatabaseUpdate::default()), |
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.
EventStatus::Commit
name confused me while reveiwing, It may be better choice to name it EventStatus::YetToCommit
or EventStatus::ReducerSuccess
now
ctx: &ExecutionContext, | ||
tx: MutTx, | ||
) -> Result<Result<Arc<ModuleEvent>, WriteSkew>, DBError> { | ||
let subscriptions = self.subscriptions.read(); |
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.
let subscriptions = self.subscriptions.read(); | |
// Take a read lock on `subscriptions` before committing tx | |
// else it can result in subscriber receiving duplicate updates. | |
let subscriptions = self.subscriptions.read(); |
Will prefer not to drop this comment.
} | ||
} | ||
|
||
pub struct WriteSkew; |
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.
nit: I would recommend to use name WriteConflict
because by defination WriteSkew
anomoly is something transaction doesn't really detect while commiting unless transaction is tracking its reads.
Description of Changes
Also, move the logic with committing and taking the subscription lock into module_subscription_actor. Best reviewed ignoring whitespace.
before/after: