Carefully watch for cache version

Author: AlexPeshkoff

src/jrd/CacheVector.cpp

@@ -72,11 +72,22 @@ ULONG* TransactionNumber::getFlags(thread_db* tdbb)
 }
 
 
-// class VersionSupport
+// class VersionIncr
 
-MdcVersion VersionSupport::next(thread_db* tdbb)
+VersionIncr::VersionIncr(thread_db* tdbb)
+	: mdc(MetadataCache::get(tdbb))
 {
-	return MetadataCache::get(tdbb)->nextVersion();
+	current = ++(mdc->mdc_front);
+}
+
+VersionIncr::~VersionIncr()
+{
+	++(mdc->mdc_back);
+}
+
+MdcVersion VersionIncr::getVersion()
+{
+	return current;
 }
 
 

src/jrd/CacheVector.h

@@ -44,6 +44,7 @@ namespace Jrd {
 
 class thread_db;
 class Lock;
+class MetadataCache;
 enum lck_t : UCHAR;
 
 class ObjectBase
@@ -120,10 +121,18 @@ namespace CacheFlag
 }
 
 
-class VersionSupport
+// Controls growth of from & back values in metadata cache
+
+class VersionIncr
 {
 public:
-	static MdcVersion next(thread_db* tdbb);
+	VersionIncr(thread_db* tdbb);
+	~VersionIncr();
+	MdcVersion getVersion();
+
+private:
+	MetadataCache* mdc;
+	MdcVersion current;
 };
 
 
@@ -344,18 +353,25 @@ class ListEntry : public HazardObject
 
 			// RETIRED is set here to avoid illegal gc until traNumber correction
 			auto newFlags = flags | CacheFlag::COMMITTED | CacheFlag::RETIRED;
+
 			// NOCOMMIT cleared to avoid extra ASTs
 			newFlags &= ~CacheFlag::NOCOMMIT;
+
+			// Handle front & back of MDC
+			VersionIncr incr(tdbb);
+
+			// And finally try to make object version world-visible
 			if (cacheFlags.compare_exchange_weak(flags, newFlags,
 				atomics::memory_order_release, atomics::memory_order_acquire))
 			{
+				traNumber = nextTrans;
+				version = incr.getVersion();
+				cacheFlags &= ~CacheFlag::RETIRED;		// Enable GC
+
 				break;
 			}
 		}
 
-		traNumber = nextTrans;
-		version = VersionSupport::next(tdbb);
-		cacheFlags &= ~CacheFlag::RETIRED;		// Enable GC
 
 		return flags;
 	}

src/jrd/Statement.cpp

@@ -205,8 +205,9 @@ Statement::Statement(thread_db* tdbb, MemoryPool* p, CompilerScratch* csb)
 
 void Statement::loadResources(thread_db* tdbb, Request* req, bool withLock)
 {
-	const MdcVersion currentMdcVersion = MetadataCache::get(tdbb)->getVersion();
-	if ((!latestVer) || (latestVer->version != currentMdcVersion))
+	auto* mdc = MetadataCache::get(tdbb);
+	const MdcVersion backVersion = mdc->getBackVersion();
+	if ((!latestVer) || (latestVer->version != backVersion))
 	{
 		MutexEnsureUnlock guard(lvMutex, FB_FUNCTION);
 
@@ -216,8 +217,10 @@ void Statement::loadResources(thread_db* tdbb, Request* req, bool withLock)
 			guard.enter();
 		}
 
-		if ((!withLock) || (latestVer->version != currentMdcVersion))
+		if ((!latestVer) || (latestVer->version != backVersion))
 		{
+		  for(MetadataCache::StableVersion sv(mdc); sv; sv.next())
+		  {
 			// Also check for changed streams from known sources
 			if (withLock && !streamsFormatCompare(tdbb))
 				ERR_post(Arg::Gds(isc_random) << "Statement format outdated, need to be reprepared");
@@ -227,8 +230,9 @@ void Statement::loadResources(thread_db* tdbb, Request* req, bool withLock)
 				resources->charSets.getCount() + resources->relations.getCount() + resources->procedures.getCount() +
 				resources->functions.getCount() + resources->triggers.getCount();
 
-			latestVer = FB_NEW_RPT(*pool, resourceCount) VersionedObjects(resourceCount, currentMdcVersion);
+			latestVer = FB_NEW_RPT(*pool, resourceCount) VersionedObjects(resourceCount, sv.getBackVersion());
 			resources->transfer(tdbb, latestVer, flags & FLAG_INTERNAL);
+		  }
 		}
 	}
 

src/jrd/met.h

@@ -102,6 +102,10 @@ inline constexpr int TFB_array				= 2;
 
 namespace Jrd {
 
+// Forward decl
+
+class VersionIncr;
+
 // Procedure block
 
 class jrd_prc : public Routine
@@ -233,7 +237,6 @@ class MetadataCache : public Firebird::PermanentStorage
 		  mdc_procedures(getPool()),
 		  mdc_functions(getPool()),
 		  mdc_charsets(getPool()),
-		  mdc_version(0),
 		  mdc_cleanup_queue(pool)
 	{
 		memset(mdc_triggers, 0, sizeof(mdc_triggers));
@@ -336,15 +339,46 @@ class MetadataCache : public Firebird::PermanentStorage
 	static void release_temp_tables(thread_db* tdbb, jrd_tra* transaction);
 	static void retain_temp_tables(thread_db* tdbb, jrd_tra* transaction, TraNumber new_number);
 
-	MdcVersion getVersion()
+	// Is used to check for comleted changes in metadata cache since some previous moment
+	MdcVersion getBackVersion()
 	{
-		return mdc_version.load(std::memory_order_relaxed);
+		return mdc_back.load(std::memory_order_relaxed);
 	}
 
-	MdcVersion nextVersion()
+	// Checks for version's drift during resources load for request(s)
+	friend class StableVersion;
+	class StableVersion
 	{
-		return ++mdc_version;
-	}
+	public:
+		StableVersion(MetadataCache* mdc)
+			: mdc(mdc), back(mdc->mdc_back), front(back + 1)
+		{ }
+
+		operator bool()
+		{
+			if (front == back)
+				return false;
+			back = mdc->mdc_back;
+			return true;
+		}
+
+		void next()
+		{
+			front = mdc->mdc_front;
+		}
+
+		MdcVersion getBackVersion()
+		{
+			return back;
+		}
+
+	private:
+		MetadataCache* mdc;
+		MdcVersion back, front;
+	};
+
+	// In CacheVector.h
+	friend class VersionIncr;
 
 	SLONG lookupSequence(thread_db*, const QualifiedName& genName)
 	{
@@ -541,8 +575,11 @@ class MetadataCache : public Firebird::PermanentStorage
 	CacheVector<Cached::Function>		mdc_functions;	// User defined functions
 	CacheVector<Cached::CharSet>		mdc_charsets;	// intl character set descriptions
 	TriggersSet							mdc_triggers[DB_TRIGGERS_COUNT];
-
-	std::atomic<MdcVersion>				mdc_version;	// Current version of metadata cache (should have 2 nums !!!!!!!!!!!!!!!)
+	// Two numbers are required because commit into cache is not atomic event.
+	// Front value is incremented before commit, back - after commit.
+	// To ensure cache remained in stable state compare
+	// back before action to protect and front after it.
+	std::atomic<MdcVersion>				mdc_front = 0, mdc_back = 0;
 	CleanupQueue						mdc_cleanup_queue;
 };