-
-
Notifications
You must be signed in to change notification settings - Fork 220
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
Do not allow run concurrent sweep instances #8320
base: master
Are you sure you want to change the base?
Do not allow run concurrent sweep instances #8320
Conversation
As far as I can see isc_sweep_concurrent_instance is returned to the client which started sweep. Where is is handled in a case when sweep started automatically? IMO it should be silently ignored. |
As I can see, |
OK. I reviewed the code once again. Initial problem can be fixed much simpler: diff --git a/src/jrd/tra.cpp b/src/jrd/tra.cpp
But suggested patch contains new feature - user is notified what happened, without it sweep silently finishes in a moment. But I do not understand - why no diags in cases of RO database and explicitly set ATT_no_cleanup? If you want to add diags for sweep startup failure all cases should better be taken into an account. |
Yeah, I agree. I will add more error messages. |
How it works for auto-sweep ? |
See |
Old code has a bug where if we already have a sweep instance running, and some one call sweep once more,
allowSweepRun()
will return false and we are gonna callclearSweepFlags()
, that will reset sweep flags and release a sweep lock. This will open an opportunity to run a second sweep instance for both Super and Classic server.