Do not allow run concurrent sweep instances

[TreeHunter9]

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 call clearSweepFlags(), 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.

[AlexPeshkoff]

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.

[TreeHunter9]

As I can see, allowSweepThread() will not allow us to run auto sweep if we have concurrent instances, but I added a check for isc_sweep_concurrent_instance anyway, just in case.

[AlexPeshkoff]

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
index 3acd848976..f289c0d923 100644
--- a/src/jrd/tra.cpp
+++ b/src/jrd/tra.cpp
@@ -1848,7 +1848,6 @@ void TRA_sweep(thread_db* tdbb)

        if (!dbb->allowSweepRun(tdbb))
        {
-               dbb->clearSweepFlags(tdbb);
                return;
        }

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.

[TreeHunter9]

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.

[hvlad]

But suggested patch contains new feature - user is notified what happened, without it sweep silently finishes in a moment.

How it works for auto-sweep ?

[AlexPeshkoff]

But suggested patch contains new feature - user is notified what happened, without it sweep silently finishes in a moment.

How it works for auto-sweep ?

See
#8320 (comment)