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
[fix] Wait for run_lock when freeing it #4195
Conversation
A store event inserts a line to the run_lock table in order to prevent concurrent storage to the same run. The run_lock record is removed at the end of storage. The insertion and deletion of the run_lock record are locked operations on the database level. In CodeChecker we used "nowait" lock which means that these insert and delete operations fail and throw an exception if they can't happen immediately. This is too strict in case of removing the run_lock object, because the thrown exception is forwarded to the user who can't handle it anyway. For this reason the run_lock removal is waiting for the database lock to be free. In the worst case the exception will still be thrown after the configured statement_timeout, but ideally that should be an unlikely event.
Unfortunately, this adds the requirement that the server should be configured with a Can you please check if using
|
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.
Maybe it is worth to try skip locked
. (Even though most real-world database applications need to be guarded with statement timeouts.)
run_lock = session.query(RunLock) \ | ||
.filter(RunLock.name == self.__name) \ | ||
.with_for_update(nowait=True).one() | ||
.with_for_update(nowait=False).one() |
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.
Should only this one usage site be modified?
Still, guidelines are not requirements. If we have this as a requirement without which the server will misbehave, then the server startup process should query the db, check if there is a statement timeout, and show a warning to the operator. |
A different solution will be provided later. |
A store event inserts a line to the run_lock table in order to prevent concurrent storage to the same run. The run_lock record is removed at the end of storage. The insertion and deletion of the run_lock record are locked operations on the database level. In CodeChecker we used "nowait" lock which means that these insert and delete operations fail and throw an exception if they can't happen immediately.
This is too strict in case of removing the run_lock object, because the thrown exception is forwarded to the user who can't handle it anyway. For this reason the run_lock removal is waiting for the database lock to be free. In the worst case the exception will still be thrown after the configured statement_timeout, but ideally that should be an unlikely event.