-
Notifications
You must be signed in to change notification settings - Fork 658
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
When adding a new tombstone record, delete all the older ones #2533
Conversation
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.
lgtm
cool finding! 👍 |
This PR seems to be causing consistent Reformatted output from CI job above
|
@aramprice @jpalermo should we open an issue for the above mentioned deadlocks or it is addressed already? |
The tombstone creation happens in a larger transaction, so if you both insert and clean tombstone records at the same time you can end up in a deadlock state if that is happening in multiple threads at the same time. This commit moves the tombstone cleanup into a scheduled job that runs once a day instead.
New PR to move it to a scheduled job |
Thanks! @jpalermo this was faster than the light :-) |
Convenient timing 😀 |
The tombstone creation happens in a larger transaction, so if you both insert and clean tombstone records at the same time you can end up in a deadlock state if that is happening in multiple threads at the same time. This commit moves the tombstone cleanup into a scheduled job that runs once a day instead.
Noticed that these are never cleaned up currently. There may be a performance hit when the first new tombstone record is added and thousands of old ones are destroyed but it shouldn't be terrible.