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: 2 people

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

@@ -968,3 +968,8 @@ FB_IMPL_MSG(JRD, 965, ods_upgrade_err, -901, "HY", "000", "ODS upgrade failed wh
 FB_IMPL_MSG(JRD, 966, bad_par_workers, -924, "HY", "000", "Wrong parallel workers value @1, valid range are from 1 to @2")
 FB_IMPL_MSG(JRD, 967, idx_expr_not_found, -902, "42", "000", "Definition of index expression is not found for index @1")
 FB_IMPL_MSG(JRD, 968, idx_cond_not_found, -902, "42", "000", "Definition of index condition is not found for index @1")
+// Codes 969..987 are used in v6
+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

@@ -5755,6 +5755,10 @@ 	IProfilerStatsImpl = class(IProfilerStats)
 	 isc_bad_par_workers = 335545286;
 	 isc_idx_expr_not_found = 335545287;
 	 isc_idx_cond_not_found = 335545288;
+	 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
@@ -296,35 +304,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))
@@ -333,17 +341,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

@@ -658,8 +658,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();
 		}