Do not allow run concurrent sweep instances (#8320)

* Do not allow run concurrent sweep instances

* Silently ignore error about concurrent sweep instance when we try to run auto sweep

* Add more error messages that describe a reason of failed sweep startup

* Pass ISC_STATUS instead of Arg::Gds to function

---------

Co-authored-by: Artyom Ivanov <[email protected]>

Author: TreeHunter9

src/include/firebird/impl/msg/jrd.h

@@ -987,3 +987,7 @@ FB_IMPL_MSG(JRD, 984, incompatible_format_patterns, -901, "HY", "000", "@1 incom
 FB_IMPL_MSG(JRD, 985, only_one_pattern_can_be_used, -901, "HY", "000", "Can use only one of these patterns @1")
 FB_IMPL_MSG(JRD, 986, can_not_use_same_pattern_twice, -901, "HY", "000", "Cannot use the same pattern twice: @1")
 FB_IMPL_MSG(JRD, 987, sysf_invalid_gen_uuid_version, -833, "42", "000", "Invalid GEN_UUID version (@1). Must be 4 or 7")
+FB_IMPL_MSG(JRD, 988, sweep_unable_to_run, -901, "42", "000", "Unable to run sweep")
+FB_IMPL_MSG(JRD, 989, sweep_concurrent_instance, -901, "42", "000", "Another instance of sweep is already running")
+FB_IMPL_MSG(JRD, 990, sweep_read_only, -901, "42", "000", "Database in read only state")
+FB_IMPL_MSG(JRD, 991, sweep_attach_no_cleanup, -901, "42", "000", "Attachment has no cleanup flag set")

src/include/gen/Firebird.pas

@@ -5777,6 +5777,10 @@ 	IProfilerStatsImpl = class(IProfilerStats)
 	 isc_only_one_pattern_can_be_used = 335545305;
 	 isc_can_not_use_same_pattern_twice = 335545306;
 	 isc_sysf_invalid_gen_uuid_version = 335545307;
+	 isc_sweep_unable_to_run = 335545308;
+	 isc_sweep_concurrent_instance = 335545309;
+	 isc_sweep_read_only = 335545310;
+	 isc_sweep_attach_no_cleanup = 335545311;
 	 isc_gfix_db_name = 335740929;
 	 isc_gfix_invalid_sw = 335740930;
 	 isc_gfix_incmp_sw = 335740932;

src/jrd/Database.cpp

@@ -47,6 +47,14 @@
 
 using namespace Firebird;
 
+namespace
+{
+	void unableToRunSweepException(ISC_STATUS reason)
+	{
+		ERR_post(Arg::Gds(isc_sweep_unable_to_run) << Arg::Gds(reason));
+	}
+}
+
 namespace Jrd
 {
 	bool Database::onRawDevice() const
@@ -299,35 +307,35 @@ namespace Jrd
 		}
 	}
 
-	bool Database::allowSweepRun(thread_db* tdbb)
+	void Database::initiateSweepRun(thread_db* tdbb)
 	{
-		SPTHR_DEBUG(fprintf(stderr, "allowSweepRun %p\n", this));
+		SPTHR_DEBUG(fprintf(stderr, FB_FUNCTION " %p\n", this));
 
 		if (readOnly())
-			return false;
+			unableToRunSweepException(isc_sweep_read_only);
 
 		Jrd::Attachment* const attachment = tdbb->getAttachment();
 		if (attachment->att_flags & ATT_no_cleanup)
-			return false;
+			unableToRunSweepException(isc_sweep_attach_no_cleanup);
 
 		while (true)
 		{
 			AtomicCounter::counter_type old = dbb_flags;
 			if (old & DBB_sweep_in_progress)
 			{
 				clearSweepStarting();
-				return false;
+				unableToRunSweepException(isc_sweep_concurrent_instance);
 			}
 
 			if (dbb_flags.compareExchange(old, old | DBB_sweep_in_progress))
 				break;
 		}
 
-		SPTHR_DEBUG(fprintf(stderr, "allowSweepRun - set DBB_sweep_in_progress\n"));
+		SPTHR_DEBUG(fprintf(stderr, FB_FUNCTION " - set DBB_sweep_in_progress\n"));
 
 		if (!(dbb_flags & DBB_sweep_starting))
 		{
-			SPTHR_DEBUG(fprintf(stderr, "allowSweepRun - createSweepLock\n"));
+			SPTHR_DEBUG(fprintf(stderr, FB_FUNCTION " - createSweepLock\n"));
 
 			createSweepLock(tdbb);
 			if (!LCK_lock(tdbb, dbb_sweep_lock, LCK_EX, -1))
@@ -336,17 +344,15 @@ namespace Jrd
 				fb_utils::init_status(tdbb->tdbb_status_vector);
 
 				dbb_flags &= ~DBB_sweep_in_progress;
-				return false;
+				unableToRunSweepException(isc_sweep_concurrent_instance);
 			}
 		}
 		else
 		{
-			SPTHR_DEBUG(fprintf(stderr, "allowSweepRun - clearSweepStarting\n"));
+			SPTHR_DEBUG(fprintf(stderr, FB_FUNCTION " - clearSweepStarting\n"));
 			attachment->att_flags |= ATT_from_thread;
 			clearSweepStarting();
 		}
-
-		return true;
 	}
 
 	void Database::clearSweepFlags(thread_db* tdbb)

src/jrd/Database.h

@@ -657,8 +657,8 @@ class Database : public pool_alloc<type_dbb>
 
 	// returns true if sweeper thread could start
 	bool allowSweepThread(thread_db* tdbb);
-	// returns true if sweep could run
-	bool allowSweepRun(thread_db* tdbb);
+	// Throw an exception if sweep cannot be run
+	void initiateSweepRun(thread_db* tdbb);
 	// reset sweep flag and release sweep lock
 	void clearSweepFlags(thread_db* tdbb);
 	// reset sweep starting flag, release thread starting mutex

src/jrd/tra.cpp

@@ -1846,11 +1846,7 @@ void TRA_sweep(thread_db* tdbb)
 	Database* const dbb = tdbb->getDatabase();
 	CHECK_DBB(dbb);
 
-	if (!dbb->allowSweepRun(tdbb))
-	{
-		dbb->clearSweepFlags(tdbb);
-		return;
-	}
+	dbb->initiateSweepRun(tdbb);
 
 	fb_assert(dbb->dbb_flags & DBB_sweep_in_progress);
 
@@ -2756,6 +2752,9 @@ namespace {
 			status.check();
 
 			AutoRelease<IAttachment> att(prov->attachDatabase(&status, dbName.c_str(), dpbLen, dpbBytes));
+			if (fb_utils::containsErrorCode(status->getErrors(), isc_sweep_unable_to_run))
+				return;
+
 			status.check();
 		}