Merge pull request #6061 from sfc-gh-satherton/delay-prioritized-eviction

Delay prioritized eviction of updated pages in Redwood until after commit completes

fdbclient/ServerKnobs.cpp

@@ -774,6 +774,7 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi
 	init( REDWOOD_PAGEFILE_GROWTH_SIZE_PAGES,                  20000 ); if( randomize && BUGGIFY ) { REDWOOD_PAGEFILE_GROWTH_SIZE_PAGES = deterministicRandom()->randomInt(200, 1000); }
 	init( REDWOOD_METRICS_INTERVAL,                              5.0 );
 	init( REDWOOD_HISTOGRAM_INTERVAL,                           30.0 );
+	init( REDWOOD_EVICT_UPDATED_PAGES,                          true ); if( randomize && BUGGIFY ) { REDWOOD_EVICT_UPDATED_PAGES = false; }
 
 	// Server request latency measurement
 	init( LATENCY_SAMPLE_SIZE,                                100000 );

fdbclient/ServerKnobs.h

@@ -727,6 +727,7 @@ public:
 	int REDWOOD_PAGEFILE_GROWTH_SIZE_PAGES; // Number of pages to grow page file by
 	double REDWOOD_METRICS_INTERVAL;
 	double REDWOOD_HISTOGRAM_INTERVAL;
+	bool REDWOOD_EVICT_UPDATED_PAGES; // Whether to prioritize eviction of updated pages from cache.
 
 	// Server request latency measurement
 	int LATENCY_SAMPLE_SIZE;

fdbserver/VersionedBTree.actor.cpp

@@ -1880,6 +1880,7 @@ class ObjectCache : NonCopyable {
 		ObjectType item;
 		int hits;
 		int size;
+		bool evictionPrioritized;
 	};
 
 	typedef std::unordered_map<IndexType, Entry> CacheT;
@@ -1905,13 +1906,13 @@ public:
 		return nullptr;
 	}
 
-	// If index is in cache, move it to the front of the eviction order
+	// If index is in cache and not on the prioritized eviction order list, move it there.
 	void prioritizeEviction(const IndexType& index) {
 		auto i = cache.find(index);
-		if (i != cache.end()) {
-			auto ei = evictionOrder.iterator_to(i->second);
-			evictionOrder.erase(ei);
-			evictionOrder.push_front(i->second);
+		if (i != cache.end() && !i->second.evictionPrioritized) {
+			prioritizedEvictions.splice(
+			    prioritizedEvictions.end(), evictionOrder, EvictionOrderT::s_iterator_to(i->second));
+			i->second.evictionPrioritized = true;
 		}
 	}
 
@@ -1926,9 +1927,10 @@ public:
 		if (entry.is_linked()) {
 			if (!noHit) {
 				++entry.hits;
-				// Move the entry to the back of the eviction order
-				evictionOrder.erase(evictionOrder.iterator_to(entry));
-				evictionOrder.push_back(entry);
+				// If item eviction is not prioritized, move to back of eviction order
+				if (!entry.evictionPrioritized) {
+					evictionOrder.splice(evictionOrder.end(), evictionOrder, EvictionOrderT::s_iterator_to(entry));
+				}
 			}
 		} else {
 			// Otherwise it was a cache miss
@@ -1939,6 +1941,7 @@ public:
 			currentSize += size;
 			// Insert the newly created Entry at the back of the eviction order
 			evictionOrder.push_back(entry);
+			entry.evictionPrioritized = false;
 
 			// While the cache is too big, evict the oldest entry until the oldest entry can't be evicted.
 			while (currentSize > sizeLimit) {
@@ -1986,6 +1989,10 @@ public:
 		state EvictionOrderT evictionOrder;
 		state int64_t currentSize;
 
+		// Flush all prioritized evictions to the main eviction order
+		self->flushPrioritizedEvictions();
+		ASSERT(cache.size() == evictionOrder.size());
+
 		// Swap cache contents to local state vars
 		// After this, no more entries will be added to or read from these
 		// structures so we know for sure that no page will become unevictable
@@ -2012,17 +2019,21 @@ public:
 	}
 
 	Future<Void> clear() {
-		ASSERT(evictionOrder.size() == cache.size());
+		ASSERT(evictionOrder.size() + prioritizedEvictions.size() == cache.size());
 		return clear_impl(this);
 	}
 
 	int count() const { return currentSize; }
 
+	// Move the prioritized evictions queued to the front of the eviction order
+	void flushPrioritizedEvictions() { evictionOrder.splice(evictionOrder.begin(), prioritizedEvictions); }
+
 private:
 	int64_t sizeLimit;
 	int64_t currentSize;
 	CacheT cache;
 	EvictionOrderT evictionOrder;
+	EvictionOrderT prioritizedEvictions;
 };
 
 ACTOR template <class T>
@@ -2725,10 +2736,12 @@ public:
 			remapQueue.pushBack(r);
 			auto& versionedMap = remappedPages[pageID];
 
-			// An update page is unlikely to have its old version read again soon, so prioritize its cache eviction
-			// If the versioned map is empty for this page then the prior version of the page is at stored at the
-			// PhysicalPageID pageID, otherwise it is the last mapped value in the version-ordered map.
-			pageCache.prioritizeEviction(versionedMap.empty() ? pageID : versionedMap.rbegin()->second);
+			if (SERVER_KNOBS->REDWOOD_EVICT_UPDATED_PAGES) {
+				// An update page is unlikely to have its old version read again soon, so prioritize its cache eviction
+				// If the versioned map is empty for this page then the prior version of the page is at stored at the
+				// PhysicalPageID pageID, otherwise it is the last mapped value in the version-ordered map.
+				pageCache.prioritizeEviction(versionedMap.empty() ? pageID : versionedMap.rbegin()->second);
+			}
 			versionedMap[v] = newPageID;
 
 			debug_printf("DWALPager(%s) pushed %s\n", filename.c_str(), RemappedPage(r).toString().c_str());
@@ -2759,7 +2772,9 @@ public:
 		}
 
 		// A freed page is unlikely to be read again soon so prioritize its cache eviction
-		pageCache.prioritizeEviction(pageID);
+		if (SERVER_KNOBS->REDWOOD_EVICT_UPDATED_PAGES) {
+			pageCache.prioritizeEviction(pageID);
+		}
 	}
 
 	LogicalPageID detachRemappedPage(LogicalPageID pageID, Version v) override {
@@ -2813,8 +2828,10 @@ public:
 			remapQueue.pushBack(RemappedPage{ v, pageID, invalidLogicalPageID });
 
 			// A freed page is unlikely to be read again soon so prioritize its cache eviction
-			PhysicalPageID previousPhysicalPage = i->second.rbegin()->second;
-			pageCache.prioritizeEviction(previousPhysicalPage);
+			if (SERVER_KNOBS->REDWOOD_EVICT_UPDATED_PAGES) {
+				PhysicalPageID previousPhysicalPage = i->second.rbegin()->second;
+				pageCache.prioritizeEviction(previousPhysicalPage);
+			}
 
 			i->second[v] = invalidLogicalPageID;
 			return;
@@ -3577,6 +3594,9 @@ public:
 		// Start unmapping pages for expired versions
 		self->remapCleanupFuture = remapCleanup(self);
 
+		// If there are prioritized evictions queued, flush them to the regular eviction order.
+		self->pageCache.flushPrioritizedEvictions();
+
 		return Void();
 	}