From 67f17ccf5c8f1f6ff1da15c1f6b5f95d9e86fa47 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Mon, 6 Jul 2020 23:14:34 -0700
Subject: [PATCH 01/14] Remove duplicate BinaryReader code

---
 flow/serialize.h | 186 +++++++++++++++++++++--------------------------
 1 file changed, 84 insertions(+), 102 deletions(-)

diff --git a/flow/serialize.h b/flow/serialize.h
index 1adc93dfa1..300410186d 100644
--- a/flow/serialize.h
+++ b/flow/serialize.h
@@ -520,132 +520,42 @@ private:
 	}
 };
 
-
-class ArenaReader {
+template<class Impl>
+class _Reader {
 public:
 	static const int isDeserializing = 1;
 	static constexpr bool isSerializing = false;
-	typedef ArenaReader READER;
+	using READER = Impl;
 
-	const void* readBytes( int bytes ) {
-		const char* b = begin;
-		const char* e = b + bytes;
-		ASSERT( e <= end );
-		begin = e;
-		return b;
-	}
-
-	const void* peekBytes( int bytes ) {
-		ASSERT( begin + bytes <= end );
+	const void *peekBytes(int bytes) {
+		ASSERT(begin + bytes <= end);
 		return begin;
 	}
 
-	void serializeBytes(void* data, int bytes) {
-		memcpy(data, readBytes(bytes), bytes);
-	}
-	
-	const uint8_t* arenaRead( int bytes ) {
-		return (const uint8_t*)readBytes(bytes);
-	}
-
-	StringRef arenaReadAll() const {
-		return StringRef(reinterpret_cast<const uint8_t*>(begin), end - begin);
+	void serializeBytes(void *data, int bytes) {
+		memcpy(data, static_cast<Impl*>(this)->readBytes(bytes), bytes);
 	}
 
 	template <class T>
 	void serializeBinaryItem( T& t ) {
-		t = *(T*)readBytes(sizeof(T));
-	}
-
-	template <class VersionOptions>
-	ArenaReader( Arena const& arena, const StringRef& input, VersionOptions vo ) : m_pool(arena), check(NULL) {
-		begin = (const char*)input.begin();
-		end = begin + input.size();
-		vo.read(*this);
-	}
-
-	Arena& arena() { return m_pool; }
-
-	ProtocolVersion protocolVersion() const { return m_protocolVersion; }
-	void setProtocolVersion(ProtocolVersion pv) { m_protocolVersion = pv; }
-
-	bool empty() const { return begin == end; }
-
-	void checkpoint() {
-		check = begin;
-	}
-
-	void rewind() {
-		ASSERT(check != NULL);
-		begin = check;
-		check = NULL;
-	}
-
-private:
-	const char *begin, *end, *check;
-	Arena m_pool;
-	ProtocolVersion m_protocolVersion;
-};
-
-class BinaryReader {
-public:
-	static const int isDeserializing = 1;
-	static constexpr bool isSerializing = false;
-	typedef BinaryReader READER;
-
-	const void* readBytes( int bytes );
-
-	const void* peekBytes( int bytes ) {
-		ASSERT( begin + bytes <= end );
-		return begin;
-	}
-
-	void serializeBytes(void* data, int bytes) {
-		memcpy(data, readBytes(bytes), bytes);
-	}
-	
-	template <class T>
-	void serializeBinaryItem( T& t ) {
-		t = *(T*)readBytes(sizeof(T));
+		t = *(T*)(static_cast<Impl*>(this)->readBytes(sizeof(T)));
 	}
 
 	const uint8_t* arenaRead( int bytes ) {
 		// Reads and returns the next bytes.
 		// The returned pointer has the lifetime of this.arena()
 		// Could be implemented zero-copy if [begin,end) was in this.arena() already; for now is a copy
-		if (!bytes) return NULL;
+		if (!bytes) return nullptr;
 		uint8_t* dat = new (arena()) uint8_t[ bytes ];
 		serializeBytes( dat, bytes );
 		return dat;
 	}
 
-	template <class VersionOptions>
-	BinaryReader( const void* data, int length, VersionOptions vo ) {
-		begin = (const char*)data;
-		end = begin + length;
-		check = nullptr;
-		vo.read(*this);
-	}
-	template <class VersionOptions>
-	BinaryReader( const StringRef& s, VersionOptions vo ) { begin = (const char*)s.begin(); end = begin + s.size(); vo.read(*this); }
-	template <class VersionOptions>
-	BinaryReader( const std::string& v, VersionOptions vo ) { begin = v.c_str(); end = begin + v.size(); vo.read(*this); }
-
-	Arena& arena() { return m_pool; }
-
-	template <class T, class VersionOptions>
-	static T fromStringRef( StringRef sr, VersionOptions vo ) {
-		T t;
-		BinaryReader r(sr, vo);
-		r >> t;
-		return t;
-	}
+	Arena &arena() { return m_pool; }
 
 	ProtocolVersion protocolVersion() const { return m_protocolVersion; }
 	void setProtocolVersion(ProtocolVersion pv) { m_protocolVersion = pv; }
 
-	void assertEnd() { ASSERT( begin == end ); }
-
 	bool empty() const { return begin == end; }
 
 	void checkpoint() {
@@ -658,13 +568,85 @@ public:
 		check = nullptr;
 	}
 
-
-private:
+protected:
 	const char *begin, *end, *check;
 	Arena m_pool;
 	ProtocolVersion m_protocolVersion;
 };
 
+class ArenaReader : public _Reader<ArenaReader> {
+public:
+	const void* readBytes( int bytes ) {
+		const char* b = begin;
+		const char* e = b + bytes;
+		ASSERT( e <= end );
+		begin = e;
+		return b;
+	}
+
+	const uint8_t* arenaRead( int bytes ) {
+		return (const uint8_t*)readBytes(bytes);
+	}
+
+	StringRef arenaReadAll() const {
+		return StringRef(reinterpret_cast<const uint8_t*>(begin), end - begin);
+	}
+
+	template <class VersionOptions>
+	ArenaReader( Arena const& arena, const StringRef& input, VersionOptions vo ) {
+		m_pool = arena;
+		check = nullptr;
+		begin = (const char*)input.begin();
+		end = begin + input.size();
+		vo.read(*this);
+	}
+};
+
+class BinaryReader : public _Reader<BinaryReader> {
+public:
+	const void* readBytes( int bytes );
+
+	const uint8_t* arenaRead( int bytes ) {
+		// Reads and returns the next bytes.
+		// The returned pointer has the lifetime of this.arena()
+		// Could be implemented zero-copy if [begin,end) was in this.arena() already; for now is a copy
+		if (!bytes) return nullptr;
+		uint8_t* dat = new (arena()) uint8_t[ bytes ];
+		serializeBytes( dat, bytes );
+		return dat;
+	}
+
+	template <class T, class VersionOptions>
+	static T fromStringRef( StringRef sr, VersionOptions vo ) {
+		T t;
+		BinaryReader r(sr, vo);
+		r >> t;
+		return t;
+	}
+
+	void assertEnd() { ASSERT(begin == end); }
+
+	template <class VersionOptions>
+	BinaryReader( const void* data, int length, VersionOptions vo ) {
+		begin = (const char*)data;
+		end = begin + length;
+		check = nullptr;
+		vo.read(*this);
+	}
+	template <class VersionOptions>
+	BinaryReader( const StringRef& s, VersionOptions vo ) {
+		begin = (const char*)s.begin();
+		end = begin + s.size();
+		vo.read(*this);
+	}
+	template <class VersionOptions>
+	BinaryReader( const std::string& v, VersionOptions vo ) {
+		begin = v.c_str();
+		end = begin + v.size();
+		vo.read(*this);
+	}
+};
+
 struct SendBuffer {
 	uint8_t const* data;
 	SendBuffer* next;

From 080a67d76fc7176bd7e9b93eecdccbc41b130422 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Tue, 7 Jul 2020 16:03:12 -0700
Subject: [PATCH 02/14] Added BinaryReader::objectReader field

---
 flow/ObjectSerializer.h |  5 +++++
 flow/serialize.h        | 31 +++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h
index fbeee1e67d..2ef06e0299 100644
--- a/flow/ObjectSerializer.h
+++ b/flow/ObjectSerializer.h
@@ -68,6 +68,7 @@ struct SaveContext {
 
 template <class ReaderImpl>
 class _ObjectReader {
+protected:
 	ProtocolVersion mProtocolVersion;
 public:
 
@@ -103,6 +104,10 @@ class ObjectReader : public _ObjectReader<ObjectReader> {
 public:
 	static constexpr bool ownsUnderlyingMemory = false;
 
+	ObjectReader(const uint8_t *data, ProtocolVersion protocolVersion) : _data(data) {
+		mProtocolVersion = protocolVersion;
+	}
+
 	template<class VersionOptions>
 	ObjectReader(const uint8_t* data, VersionOptions vo) : _data(data) {
 		vo.read(*this);
diff --git a/flow/serialize.h b/flow/serialize.h
index 300410186d..a78bf8ab38 100644
--- a/flow/serialize.h
+++ b/flow/serialize.h
@@ -603,6 +603,8 @@ public:
 };
 
 class BinaryReader : public _Reader<BinaryReader> {
+	std::unique_ptr<ObjectReader> objectReader;
+
 public:
 	const void* readBytes( int bytes );
 
@@ -632,18 +634,47 @@ public:
 		end = begin + length;
 		check = nullptr;
 		vo.read(*this);
+		if (m_protocolVersion.hasObjectSerializerFlag()) {
+			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
+		}
 	}
 	template <class VersionOptions>
 	BinaryReader( const StringRef& s, VersionOptions vo ) {
 		begin = (const char*)s.begin();
 		end = begin + s.size();
+		check = nullptr;
 		vo.read(*this);
+		if (m_protocolVersion.hasObjectSerializerFlag()) {
+			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
+		}
 	}
 	template <class VersionOptions>
 	BinaryReader( const std::string& v, VersionOptions vo ) {
 		begin = v.c_str();
 		end = begin + v.size();
+		check = nullptr;
 		vo.read(*this);
+		if (m_protocolVersion.hasObjectSerializerFlag()) {
+			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
+		}
+	}
+
+	template<class T>
+	void deserialize(T &t) {
+		if (objectReader) {
+			objectReader->deserialize(t);
+		} else {
+			t.serialize(*this);
+		}
+	}
+};
+
+template<class T>
+class Serializer<BinaryReader, T, typename std::enable_if_t<HasFileIdentifier<T>::value>> {
+public:
+	static void serialize( BinaryReader& ar, T& t ) {
+		ar.deserialize(t);
+		ASSERT( ar.protocolVersion().isValid() );
 	}
 };
 

From d3d7ad6de58b28d8c7e2bfbd8410e4a2f547dc11 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Tue, 7 Jul 2020 16:35:46 -0700
Subject: [PATCH 03/14] Simplify _Reader constructors

---
 flow/serialize.h | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/flow/serialize.h b/flow/serialize.h
index a78bf8ab38..9959055729 100644
--- a/flow/serialize.h
+++ b/flow/serialize.h
@@ -568,8 +568,12 @@ public:
 		check = nullptr;
 	}
 
+	_Reader(const char* begin, const char* end) : begin(begin), end(end) {}
+	_Reader(const char* begin, const char* end, const Arena& arena) : begin(begin), end(end), m_pool(arena) {}
+
 protected:
-	const char *begin, *end, *check;
+	const char *begin, *end;
+	const char* check = nullptr;
 	Arena m_pool;
 	ProtocolVersion m_protocolVersion;
 };
@@ -593,11 +597,8 @@ public:
 	}
 
 	template <class VersionOptions>
-	ArenaReader( Arena const& arena, const StringRef& input, VersionOptions vo ) {
-		m_pool = arena;
-		check = nullptr;
-		begin = (const char*)input.begin();
-		end = begin + input.size();
+	ArenaReader(Arena const& arena, const StringRef& input, VersionOptions vo)
+	  : _Reader(reinterpret_cast<const char*>(input.begin()), reinterpret_cast<const char*>(input.end()), arena) {
 		vo.read(*this);
 	}
 };
@@ -629,30 +630,23 @@ public:
 	void assertEnd() { ASSERT(begin == end); }
 
 	template <class VersionOptions>
-	BinaryReader( const void* data, int length, VersionOptions vo ) {
-		begin = (const char*)data;
-		end = begin + length;
-		check = nullptr;
+	BinaryReader(const void* data, int length, VersionOptions vo)
+	  : _Reader(reinterpret_cast<const char*>(data), reinterpret_cast<const char*>(data) + length) {
 		vo.read(*this);
 		if (m_protocolVersion.hasObjectSerializerFlag()) {
 			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
 		}
 	}
 	template <class VersionOptions>
-	BinaryReader( const StringRef& s, VersionOptions vo ) {
-		begin = (const char*)s.begin();
-		end = begin + s.size();
-		check = nullptr;
+	BinaryReader(const StringRef& s, VersionOptions vo)
+	  : _Reader(reinterpret_cast<const char*>(s.begin()), reinterpret_cast<const char*>(s.end())) {
 		vo.read(*this);
 		if (m_protocolVersion.hasObjectSerializerFlag()) {
 			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
 		}
 	}
 	template <class VersionOptions>
-	BinaryReader( const std::string& v, VersionOptions vo ) {
-		begin = v.c_str();
-		end = begin + v.size();
-		check = nullptr;
+	BinaryReader(const std::string& s, VersionOptions vo) : _Reader(s.c_str(), s.c_str() + s.size()) {
 		vo.read(*this);
 		if (m_protocolVersion.hasObjectSerializerFlag()) {
 			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);

From 373b5ffb4fd5c872970107bda14e3da4ddfef7a2 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Thu, 9 Jul 2020 14:26:23 -0700
Subject: [PATCH 04/14] Added forward compatibility and Downgrade workload

---
 fdbserver/CMakeLists.txt                |   1 +
 fdbserver/workloads/Downgrade.actor.cpp | 152 ++++++++++++++++++++++++
 flow/ObjectSerializer.h                 |   4 -
 flow/serialize.h                        |  72 ++++++-----
 tests/CMakeLists.txt                    |   1 +
 tests/fast/Downgrade.txt                |   4 +
 6 files changed, 197 insertions(+), 37 deletions(-)
 create mode 100644 fdbserver/workloads/Downgrade.actor.cpp
 create mode 100644 tests/fast/Downgrade.txt

diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt
index a495cec12d..b3c5fb4cd6 100644
--- a/fdbserver/CMakeLists.txt
+++ b/fdbserver/CMakeLists.txt
@@ -139,6 +139,7 @@ set(FDBSERVER_SRCS
   workloads/DDMetricsExclude.actor.cpp
   workloads/DiskDurability.actor.cpp
   workloads/DiskDurabilityTest.actor.cpp
+  workloads/Downgrade.actor.cpp
   workloads/DummyWorkload.actor.cpp
   workloads/ExternalWorkload.actor.cpp
   workloads/FastTriggeredWatches.actor.cpp
diff --git a/fdbserver/workloads/Downgrade.actor.cpp b/fdbserver/workloads/Downgrade.actor.cpp
new file mode 100644
index 0000000000..f09401dd99
--- /dev/null
+++ b/fdbserver/workloads/Downgrade.actor.cpp
@@ -0,0 +1,152 @@
+/*
+ * Downgrade.actor.cpp
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2013-2020 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "fdbclient/NativeAPI.actor.h"
+#include "fdbserver/TesterInterface.actor.h"
+#include "fdbserver/workloads/workloads.actor.h"
+#include "flow/serialize.h"
+#include "flow/actorcompiler.h" // This must be the last #include.
+
+struct DowngradeWorkload : TestWorkload {
+
+	static constexpr const char* NAME = "Downgrade";
+	Key oldKey, newKey;
+
+	DowngradeWorkload(WorkloadContext const& wcx) : TestWorkload(wcx) {
+		oldKey = getOption(options, LiteralStringRef("oldKey"), LiteralStringRef("oldKey"));
+		newKey = getOption(options, LiteralStringRef("newKey"), LiteralStringRef("newKey"));
+	}
+
+	template <class Impl>
+	struct _Struct {
+		static constexpr FileIdentifier file_identifier = 2340487;
+		int oldField = 0;
+	};
+
+	struct OldStruct : public _Struct<OldStruct> {
+		void setFields() { oldField = 1; }
+
+		template <class Archive>
+		void serialize(Archive& ar) {
+			serializer(ar, oldField);
+		}
+	};
+
+	struct NewStruct : public _Struct<NewStruct> {
+		int newField = 0;
+
+		void setFields() {
+			oldField = 1;
+			newField = 2;
+		}
+
+		template <class Archive>
+		void serialize(Archive& ar) {
+			serializer(ar, oldField, newField);
+		}
+	};
+
+	ACTOR static Future<Void> writeOld(Database cx, Key key) {
+		BinaryWriter writer(IncludeVersion(currentProtocolVersion));
+		std::vector<OldStruct> data(10);
+		for (auto& oldStruct : data) {
+			oldStruct.setFields();
+		}
+		writer << data;
+		state Value value = writer.toValue();
+
+		state Transaction tr(cx);
+		loop {
+			try {
+				tr.set(key, value);
+				wait(tr.commit());
+				return Void();
+			} catch (Error& e) {
+				wait(tr.onError(e));
+			}
+		}
+	}
+
+	ACTOR static Future<Void> writeNew(Database cx, Key key) {
+		ProtocolVersion protocolVersion = currentProtocolVersion;
+		protocolVersion.addObjectSerializerFlag();
+		ObjectWriter writer(IncludeVersion(protocolVersion));
+		std::vector<NewStruct> data(10);
+		for (auto& newObject : data) {
+			newObject.setFields();
+		}
+		writer.serialize(data);
+		state Value value = writer.toStringRef();
+
+		state Transaction tr(cx);
+		loop {
+			try {
+				tr.set(key, value);
+				wait(tr.commit());
+				return Void();
+			} catch (Error& e) {
+				wait(tr.onError(e));
+			}
+		}
+	}
+
+	ACTOR static Future<Void> readData(Database cx, Key key) {
+		state Transaction tr(cx);
+		state Value value;
+
+		loop {
+			try {
+				Optional<Value> _value = wait(tr.get(key));
+				ASSERT(_value.present());
+				value = _value.get();
+				break;
+			} catch (Error& e) {
+				wait(tr.onError(e));
+			}
+		}
+
+		BinaryReader reader(value, IncludeVersion());
+		std::vector<OldStruct> data;
+		reader >> data;
+		ASSERT(data.size() == 10);
+		for (const auto& oldObject : data) {
+			ASSERT(oldObject.oldField == 1);
+		}
+		return Void();
+	}
+
+	std::string description() override { return NAME; }
+
+	Future<Void> setup(Database const& cx) override {
+		return clientId ? Void() : (writeOld(cx, oldKey) && writeNew(cx, newKey));
+	}
+
+	Future<Void> start(Database const& cx) override {
+		return clientId ? Void() : (readData(cx, oldKey) && readData(cx, newKey));
+	}
+
+	Future<bool> check(Database const& cx) override {
+		// Failures are checked with assertions
+		return true;
+	}
+	void getMetrics(vector<PerfMetric>& m) override {}
+};
+
+WorkloadFactory<DowngradeWorkload> DowngradeWorkloadFactory(DowngradeWorkload::NAME);
diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h
index 2ef06e0299..7682d88c7b 100644
--- a/flow/ObjectSerializer.h
+++ b/flow/ObjectSerializer.h
@@ -104,10 +104,6 @@ class ObjectReader : public _ObjectReader<ObjectReader> {
 public:
 	static constexpr bool ownsUnderlyingMemory = false;
 
-	ObjectReader(const uint8_t *data, ProtocolVersion protocolVersion) : _data(data) {
-		mProtocolVersion = protocolVersion;
-	}
-
 	template<class VersionOptions>
 	ObjectReader(const uint8_t* data, VersionOptions vo) : _data(data) {
 		vo.read(*this);
diff --git a/flow/serialize.h b/flow/serialize.h
index 9959055729..7c85f538f4 100644
--- a/flow/serialize.h
+++ b/flow/serialize.h
@@ -78,7 +78,7 @@ inline typename Archive::WRITER& operator << (Archive& ar, const Item& item ) {
 
 template <class Archive, class Item>
 inline typename Archive::READER& operator >> (Archive& ar, Item& item ) {
-	load(ar, item);
+	ar.deserialize(item);
 	return ar;
 }
 
@@ -286,14 +286,7 @@ struct _IncludeVersion {
 			TraceEvent(SevWarnAlways, "InvalidSerializationVersion").error(err).detailf("Version", "%llx", v.versionWithFlags());
 			throw err;
 		}
-		if (v > currentProtocolVersion) {
-			// For now, no forward compatibility whatsoever is supported.  In the future, this check may be weakened for
-			// particular data structures (e.g. to support mismatches between client and server versions when the client
-			// must deserialize zookeeper and database structures)
-			auto err = incompatible_protocol_version();
-			TraceEvent(SevError, "FutureProtocolVersion").error(err).detailf("Version", "%llx", v.versionWithFlags());
-			throw err;
-		}
+		// TODO: Add the add maximum readable version.
 		ar.setProtocolVersion(v);
 	}
 };
@@ -568,10 +561,10 @@ public:
 		check = nullptr;
 	}
 
+protected:
 	_Reader(const char* begin, const char* end) : begin(begin), end(end) {}
 	_Reader(const char* begin, const char* end, const Arena& arena) : begin(begin), end(end), m_pool(arena) {}
 
-protected:
 	const char *begin, *end;
 	const char* check = nullptr;
 	Arena m_pool;
@@ -579,6 +572,8 @@ protected:
 };
 
 class ArenaReader : public _Reader<ArenaReader> {
+	Optional<ArenaObjectReader> arenaObjectReader;
+
 public:
 	const void* readBytes( int bytes ) {
 		const char* b = begin;
@@ -600,11 +595,27 @@ public:
 	ArenaReader(Arena const& arena, const StringRef& input, VersionOptions vo)
 	  : _Reader(reinterpret_cast<const char*>(input.begin()), reinterpret_cast<const char*>(input.end()), arena) {
 		vo.read(*this);
+		if (m_protocolVersion.hasObjectSerializerFlag()) {
+			arenaObjectReader = ArenaObjectReader(arena, input, vo);
+		}
+	}
+
+	template <class T>
+	void deserialize(T& t) {
+		if constexpr (HasFileIdentifier<T>::value) {
+			if (arenaObjectReader.present()) {
+				arenaObjectReader.get().deserialize(t);
+			} else {
+				load(*this, t);
+			}
+		} else {
+			load(*this, t);
+		}
 	}
 };
 
 class BinaryReader : public _Reader<BinaryReader> {
-	std::unique_ptr<ObjectReader> objectReader;
+	Optional<ObjectReader> objectReader;
 
 public:
 	const void* readBytes( int bytes );
@@ -632,43 +643,38 @@ public:
 	template <class VersionOptions>
 	BinaryReader(const void* data, int length, VersionOptions vo)
 	  : _Reader(reinterpret_cast<const char*>(data), reinterpret_cast<const char*>(data) + length) {
-		vo.read(*this);
-		if (m_protocolVersion.hasObjectSerializerFlag()) {
-			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
-		}
+		readVersion(vo);
 	}
 	template <class VersionOptions>
 	BinaryReader(const StringRef& s, VersionOptions vo)
 	  : _Reader(reinterpret_cast<const char*>(s.begin()), reinterpret_cast<const char*>(s.end())) {
-		vo.read(*this);
-		if (m_protocolVersion.hasObjectSerializerFlag()) {
-			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
-		}
+		readVersion(vo);
 	}
 	template <class VersionOptions>
 	BinaryReader(const std::string& s, VersionOptions vo) : _Reader(s.c_str(), s.c_str() + s.size()) {
-		vo.read(*this);
-		if (m_protocolVersion.hasObjectSerializerFlag()) {
-			objectReader = std::make_unique<ObjectReader>(reinterpret_cast<const uint8_t*>(begin), m_protocolVersion);
-		}
+		readVersion(vo);
 	}
 
 	template<class T>
 	void deserialize(T &t) {
-		if (objectReader) {
-			objectReader->deserialize(t);
+		if constexpr (HasFileIdentifier<T>::value) {
+			if (objectReader.present()) {
+				objectReader.get().deserialize(t);
+			} else {
+				load(*this, t);
+			}
 		} else {
-			t.serialize(*this);
+			load(*this, t);
 		}
 	}
-};
 
-template<class T>
-class Serializer<BinaryReader, T, typename std::enable_if_t<HasFileIdentifier<T>::value>> {
-public:
-	static void serialize( BinaryReader& ar, T& t ) {
-		ar.deserialize(t);
-		ASSERT( ar.protocolVersion().isValid() );
+private:
+	template <class VersionOptions>
+	void readVersion(VersionOptions vo) {
+		vo.read(*this);
+		if (m_protocolVersion.hasObjectSerializerFlag()) {
+			objectReader = ObjectReader(reinterpret_cast<const uint8_t*>(begin), AssumeVersion(m_protocolVersion));
+		}
 	}
 };
 
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 4b095cd78d..fd3cc6ffb3 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -116,6 +116,7 @@ if(WITH_PYTHON)
   add_fdb_test(TEST_FILES fast/ConstrainedRandomSelector.txt)
   add_fdb_test(TEST_FILES fast/CycleAndLock.txt)
   add_fdb_test(TEST_FILES fast/CycleTest.txt)
+  add_fdb_test(TEST_FILES fast/Downgrade.txt)
   add_fdb_test(TEST_FILES fast/FuzzApiCorrectness.txt)
   add_fdb_test(TEST_FILES fast/FuzzApiCorrectnessClean.txt)
   add_fdb_test(TEST_FILES fast/IncrementTest.txt)
diff --git a/tests/fast/Downgrade.txt b/tests/fast/Downgrade.txt
new file mode 100644
index 0000000000..6dd25105c8
--- /dev/null
+++ b/tests/fast/Downgrade.txt
@@ -0,0 +1,4 @@
+testTitle=Downgrade
+    testName=Downgrade
+    oldKey=oldKey
+    newKey=newKey

From 949a17359ab4d31ed1ce79d3b35e173e3df94de9 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Wed, 17 Jun 2020 23:04:04 -0700
Subject: [PATCH 05/14] Added to_6.3.0 downgrade test to test harness

---
 contrib/TestHarness/Program.cs                | 11 ++++---
 tests/CMakeLists.txt                          |  3 ++
 .../to_6.3.0/CycleTestRestart-1.txt           | 30 +++++++++++++++++++
 .../to_6.3.0/CycleTestRestart-2.txt           | 26 ++++++++++++++++
 4 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 tests/restarting/to_6.3.0/CycleTestRestart-1.txt
 create mode 100644 tests/restarting/to_6.3.0/CycleTestRestart-2.txt

diff --git a/contrib/TestHarness/Program.cs b/contrib/TestHarness/Program.cs
index 93b14d176c..ab905deb89 100644
--- a/contrib/TestHarness/Program.cs
+++ b/contrib/TestHarness/Program.cs
@@ -261,7 +261,7 @@ namespace SummarizeTest
                     testFile = random.Choice(uniqueFiles);
                     string oldBinaryVersionLowerBound = "0.0.0";
                     string lastFolderName = Path.GetFileName(Path.GetDirectoryName(testFile));
-                    if (lastFolderName.Contains("from_")) // Only perform upgrade tests from certain versions
+                    if (lastFolderName.Contains("from_") || lastFolderName.Contains("to_")) // Only perform upgrade/downgrade tests from certain versions
                     {
                         oldBinaryVersionLowerBound = lastFolderName.Split('_').Last();
                     }
@@ -295,14 +295,17 @@ namespace SummarizeTest
 
                 if (testDir.EndsWith("restarting"))
                 {
+                    bool isDowngrade = Path.GetFileName(Path.GetDirectoryName(testFile)).Contains("to_");
+                    string firstServerName = isDowngrade ? fdbserverName : oldServerName;
+                    string secondServerName = isDowngrade ? oldServerName : fdbserverName;
                     int expectedUnseed = -1;
                     int unseed;
                     string uid = Guid.NewGuid().ToString();
-                    bool useNewPlugin = oldServerName == fdbserverName || versionGreaterThanOrEqual(oldServerName.Split('-').Last(), "5.2.0");
-                    result = RunTest(oldServerName, useNewPlugin ? tlsPluginFile : tlsPluginFile_5_1, summaryFileName, errorFileName, seed, buggify, testFile + "-1.txt", runDir, uid, expectedUnseed, out unseed, out retryableError, logOnRetryableError, useValgrind, false, true, oldServerName, traceToStdout);
+                    bool useNewPlugin = isDowngrade || versionGreaterThanOrEqual(oldServerName.Split('-').Last(), "5.2.0");
+                    result = RunTest(firstServerName, useNewPlugin ? tlsPluginFile : tlsPluginFile_5_1, summaryFileName, errorFileName, seed, buggify, testFile + "-1.txt", runDir, uid, expectedUnseed, out unseed, out retryableError, logOnRetryableError, useValgrind, false, true, oldServerName, traceToStdout);
                     if (result == 0)
                     {
-                        result = RunTest(fdbserverName, tlsPluginFile, summaryFileName, errorFileName, seed+1, buggify, testFile + "-2.txt", runDir, uid, expectedUnseed, out unseed, out retryableError, logOnRetryableError, useValgrind, true, false, oldServerName, traceToStdout);
+                        result = RunTest(secondServerName, tlsPluginFile, summaryFileName, errorFileName, seed+1, buggify, testFile + "-2.txt", runDir, uid, expectedUnseed, out unseed, out retryableError, logOnRetryableError, useValgrind, true, false, oldServerName, traceToStdout);
                     }
                 }
                 else
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index fd3cc6ffb3..733cb38757 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -190,6 +190,9 @@ if(WITH_PYTHON)
   add_fdb_test(
     TEST_FILES restarting/from_5.2.0/ClientTransactionProfilingCorrectness-1.txt
                restarting/from_5.2.0/ClientTransactionProfilingCorrectness-2.txt)
+  add_fdb_test(
+    TEST_FILES restarting/to_6.3.0/CycleTestRestart-1.txt
+               restarting/to_6.3.0/CycleTestRestart-2.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectness.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectnessAtomicRestore.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectnessSwitchover.txt)
diff --git a/tests/restarting/to_6.3.0/CycleTestRestart-1.txt b/tests/restarting/to_6.3.0/CycleTestRestart-1.txt
new file mode 100644
index 0000000000..647c2f3fe3
--- /dev/null
+++ b/tests/restarting/to_6.3.0/CycleTestRestart-1.txt
@@ -0,0 +1,30 @@
+testTitle=Clogged
+    clearAfterTest=false
+    testName=Cycle
+    transactionsPerSecond=500.0
+    nodeCount=2500
+    testDuration=10.0
+    expectedRate=0
+
+    testName=RandomClogging
+    testDuration=10.0
+
+    testName=Rollback
+    meanDelay=10.0
+    testDuration=10.0
+
+    testName=Attrition
+    machinesToKill=10
+    machinesToLeave=3
+    reboot=true
+    testDuration=10.0
+
+    testName=Attrition
+    machinesToKill=10
+    machinesToLeave=3
+    reboot=true
+    testDuration=10.0
+
+    testName=SaveAndKill
+    restartInfoLocation=simfdb/restartInfo.ini
+    testDuration=10.0
diff --git a/tests/restarting/to_6.3.0/CycleTestRestart-2.txt b/tests/restarting/to_6.3.0/CycleTestRestart-2.txt
new file mode 100644
index 0000000000..7d498f2be1
--- /dev/null
+++ b/tests/restarting/to_6.3.0/CycleTestRestart-2.txt
@@ -0,0 +1,26 @@
+testTitle=Clogged
+    runSetup=false
+    testName=Cycle
+    transactionsPerSecond=2500.0
+    nodeCount=2500
+    testDuration=10.0
+    expectedRate=0
+
+    testName=RandomClogging
+    testDuration=10.0
+
+    testName=Rollback
+    meanDelay=10.0
+    testDuration=10.0
+
+    testName=Attrition
+    machinesToKill=10
+    machinesToLeave=3
+    reboot=true
+    testDuration=10.0
+
+    testName=Attrition
+    machinesToKill=10
+    machinesToLeave=3
+    reboot=true
+    testDuration=10.0

From e69f299a5c09abe4a1374bb33c6cf98c2c720a22 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Thu, 9 Jul 2020 14:39:35 -0700
Subject: [PATCH 06/14] Added minInvalidProtocolVersion

---
 flow/ProtocolVersion.h | 3 +++
 flow/serialize.h       | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/flow/ProtocolVersion.h b/flow/ProtocolVersion.h
index b63c91561f..90d046f150 100644
--- a/flow/ProtocolVersion.h
+++ b/flow/ProtocolVersion.h
@@ -135,3 +135,6 @@ constexpr ProtocolVersion currentProtocolVersion(0x0FDB00B063010001LL);
 // This assert is intended to help prevent incrementing the leftmost digits accidentally. It will probably need to
 // change when we reach version 10.
 static_assert(currentProtocolVersion.version() < 0x0FDB00B100000000LL, "Unexpected protocol version");
+
+// Downgrades are only supported for one minor version
+constexpr ProtocolVersion minInvalidProtocolVersion(0x0FDB00B071000000LL);
diff --git a/flow/serialize.h b/flow/serialize.h
index 7c85f538f4..bf9a38229e 100644
--- a/flow/serialize.h
+++ b/flow/serialize.h
@@ -286,7 +286,12 @@ struct _IncludeVersion {
 			TraceEvent(SevWarnAlways, "InvalidSerializationVersion").error(err).detailf("Version", "%llx", v.versionWithFlags());
 			throw err;
 		}
-		// TODO: Add the add maximum readable version.
+		if (v >= minInvalidProtocolVersion) {
+			// Downgrades are only supported for one minor version
+			auto err = incompatible_protocol_version();
+			TraceEvent(SevError, "FutureProtocolVersion").error(err).detailf("Version", "%llx", v.versionWithFlags());
+			throw err;
+		}
 		ar.setProtocolVersion(v);
 	}
 };

From cf4f753836e7957e941d7031f81d483ad92381b2 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Thu, 9 Jul 2020 16:26:27 -0700
Subject: [PATCH 07/14] Test ArenaReader in Downgrade workload

---
 fdbserver/workloads/Downgrade.actor.cpp | 25 +++++++++++++++++++------
 flow/serialize.h                        | 10 ----------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/fdbserver/workloads/Downgrade.actor.cpp b/fdbserver/workloads/Downgrade.actor.cpp
index f09401dd99..00c0f86d45 100644
--- a/fdbserver/workloads/Downgrade.actor.cpp
+++ b/fdbserver/workloads/Downgrade.actor.cpp
@@ -122,12 +122,25 @@ struct DowngradeWorkload : TestWorkload {
 			}
 		}
 
-		BinaryReader reader(value, IncludeVersion());
-		std::vector<OldStruct> data;
-		reader >> data;
-		ASSERT(data.size() == 10);
-		for (const auto& oldObject : data) {
-			ASSERT(oldObject.oldField == 1);
+		{
+			// use BinaryReader
+			BinaryReader reader(value, IncludeVersion());
+			std::vector<OldStruct> data;
+			reader >> data;
+			ASSERT(data.size() == 10);
+			for (const auto& oldObject : data) {
+				ASSERT(oldObject.oldField == 1);
+			}
+		}
+		{
+			// use ArenaReader
+			ArenaReader reader(Arena(), value, IncludeVersion());
+			std::vector<OldStruct> data;
+			reader >> data;
+			ASSERT(data.size() == 10);
+			for (const auto& oldObject : data) {
+				ASSERT(oldObject.oldField == 1);
+			}
 		}
 		return Void();
 	}
diff --git a/flow/serialize.h b/flow/serialize.h
index bf9a38229e..b98b6e9a99 100644
--- a/flow/serialize.h
+++ b/flow/serialize.h
@@ -539,16 +539,6 @@ public:
 		t = *(T*)(static_cast<Impl*>(this)->readBytes(sizeof(T)));
 	}
 
-	const uint8_t* arenaRead( int bytes ) {
-		// Reads and returns the next bytes.
-		// The returned pointer has the lifetime of this.arena()
-		// Could be implemented zero-copy if [begin,end) was in this.arena() already; for now is a copy
-		if (!bytes) return nullptr;
-		uint8_t* dat = new (arena()) uint8_t[ bytes ];
-		serializeBytes( dat, bytes );
-		return dat;
-	}
-
 	Arena &arena() { return m_pool; }
 
 	ProtocolVersion protocolVersion() const { return m_protocolVersion; }

From 8a5fb5ec15d509a48b84965b9aef50224c283a64 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Thu, 9 Jul 2020 17:24:43 -0700
Subject: [PATCH 08/14] Move to_6.3.0 restarting test to to_6.3.3

---
 tests/CMakeLists.txt                                          | 4 ++--
 .../restarting/{to_6.3.0 => to_6.3.3}/CycleTestRestart-1.txt  | 0
 .../restarting/{to_6.3.0 => to_6.3.3}/CycleTestRestart-2.txt  | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename tests/restarting/{to_6.3.0 => to_6.3.3}/CycleTestRestart-1.txt (100%)
 rename tests/restarting/{to_6.3.0 => to_6.3.3}/CycleTestRestart-2.txt (100%)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 733cb38757..289e666a1d 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -191,8 +191,8 @@ if(WITH_PYTHON)
     TEST_FILES restarting/from_5.2.0/ClientTransactionProfilingCorrectness-1.txt
                restarting/from_5.2.0/ClientTransactionProfilingCorrectness-2.txt)
   add_fdb_test(
-    TEST_FILES restarting/to_6.3.0/CycleTestRestart-1.txt
-               restarting/to_6.3.0/CycleTestRestart-2.txt)
+    TEST_FILES restarting/to_6.3.3/CycleTestRestart-1.txt
+               restarting/to_6.3.3/CycleTestRestart-2.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectness.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectnessAtomicRestore.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectnessSwitchover.txt)
diff --git a/tests/restarting/to_6.3.0/CycleTestRestart-1.txt b/tests/restarting/to_6.3.3/CycleTestRestart-1.txt
similarity index 100%
rename from tests/restarting/to_6.3.0/CycleTestRestart-1.txt
rename to tests/restarting/to_6.3.3/CycleTestRestart-1.txt
diff --git a/tests/restarting/to_6.3.0/CycleTestRestart-2.txt b/tests/restarting/to_6.3.3/CycleTestRestart-2.txt
similarity index 100%
rename from tests/restarting/to_6.3.0/CycleTestRestart-2.txt
rename to tests/restarting/to_6.3.3/CycleTestRestart-2.txt

From bb54f8ab8acee7c41ca0fe1932a5a0dda90accad Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Thu, 9 Jul 2020 17:25:16 -0700
Subject: [PATCH 09/14] Parametrize numObjects in Downgrade workload

---
 fdbserver/workloads/Downgrade.actor.cpp | 30 ++++++++++++++++---------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/fdbserver/workloads/Downgrade.actor.cpp b/fdbserver/workloads/Downgrade.actor.cpp
index 00c0f86d45..cc2c35abf5 100644
--- a/fdbserver/workloads/Downgrade.actor.cpp
+++ b/fdbserver/workloads/Downgrade.actor.cpp
@@ -28,10 +28,12 @@ struct DowngradeWorkload : TestWorkload {
 
 	static constexpr const char* NAME = "Downgrade";
 	Key oldKey, newKey;
+	int numObjects;
 
 	DowngradeWorkload(WorkloadContext const& wcx) : TestWorkload(wcx) {
 		oldKey = getOption(options, LiteralStringRef("oldKey"), LiteralStringRef("oldKey"));
 		newKey = getOption(options, LiteralStringRef("newKey"), LiteralStringRef("newKey"));
+		numObjects = getOption(options, LiteralStringRef("numOptions"), deterministicRandom()->randomInt(0,100));
 	}
 
 	template <class Impl>
@@ -43,6 +45,8 @@ struct DowngradeWorkload : TestWorkload {
 	struct OldStruct : public _Struct<OldStruct> {
 		void setFields() { oldField = 1; }
 
+		bool isSet() const { return oldField == 1; }
+
 		template <class Archive>
 		void serialize(Archive& ar) {
 			serializer(ar, oldField);
@@ -52,6 +56,10 @@ struct DowngradeWorkload : TestWorkload {
 	struct NewStruct : public _Struct<NewStruct> {
 		int newField = 0;
 
+		bool isSet() const {
+			return oldField == 1 && newField == 2;
+		}
+
 		void setFields() {
 			oldField = 1;
 			newField = 2;
@@ -63,9 +71,9 @@ struct DowngradeWorkload : TestWorkload {
 		}
 	};
 
-	ACTOR static Future<Void> writeOld(Database cx, Key key) {
+	ACTOR static Future<Void> writeOld(Database cx, int numObjects, Key key) {
 		BinaryWriter writer(IncludeVersion(currentProtocolVersion));
-		std::vector<OldStruct> data(10);
+		std::vector<OldStruct> data(numObjects);
 		for (auto& oldStruct : data) {
 			oldStruct.setFields();
 		}
@@ -84,11 +92,11 @@ struct DowngradeWorkload : TestWorkload {
 		}
 	}
 
-	ACTOR static Future<Void> writeNew(Database cx, Key key) {
+	ACTOR static Future<Void> writeNew(Database cx, int numObjects, Key key) {
 		ProtocolVersion protocolVersion = currentProtocolVersion;
 		protocolVersion.addObjectSerializerFlag();
 		ObjectWriter writer(IncludeVersion(protocolVersion));
-		std::vector<NewStruct> data(10);
+		std::vector<NewStruct> data(numObjects);
 		for (auto& newObject : data) {
 			newObject.setFields();
 		}
@@ -107,7 +115,7 @@ struct DowngradeWorkload : TestWorkload {
 		}
 	}
 
-	ACTOR static Future<Void> readData(Database cx, Key key) {
+	ACTOR static Future<Void> readData(Database cx, int numObjects, Key key) {
 		state Transaction tr(cx);
 		state Value value;
 
@@ -127,9 +135,9 @@ struct DowngradeWorkload : TestWorkload {
 			BinaryReader reader(value, IncludeVersion());
 			std::vector<OldStruct> data;
 			reader >> data;
-			ASSERT(data.size() == 10);
+			ASSERT(data.size() == numObjects);
 			for (const auto& oldObject : data) {
-				ASSERT(oldObject.oldField == 1);
+				ASSERT(oldObject.isSet());
 			}
 		}
 		{
@@ -137,9 +145,9 @@ struct DowngradeWorkload : TestWorkload {
 			ArenaReader reader(Arena(), value, IncludeVersion());
 			std::vector<OldStruct> data;
 			reader >> data;
-			ASSERT(data.size() == 10);
+			ASSERT(data.size() == numObjects);
 			for (const auto& oldObject : data) {
-				ASSERT(oldObject.oldField == 1);
+				ASSERT(oldObject.isSet());
 			}
 		}
 		return Void();
@@ -148,11 +156,11 @@ struct DowngradeWorkload : TestWorkload {
 	std::string description() override { return NAME; }
 
 	Future<Void> setup(Database const& cx) override {
-		return clientId ? Void() : (writeOld(cx, oldKey) && writeNew(cx, newKey));
+		return clientId ? Void() : (writeOld(cx, numObjects, oldKey) && writeNew(cx, numObjects, newKey));
 	}
 
 	Future<Void> start(Database const& cx) override {
-		return clientId ? Void() : (readData(cx, oldKey) && readData(cx, newKey));
+		return clientId ? Void() : (readData(cx, numObjects, oldKey) && readData(cx, numObjects, newKey));
 	}
 
 	Future<bool> check(Database const& cx) override {

From 10a4b8e321fae5e665e222e82f5d2ca2f4fca7d6 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Thu, 9 Jul 2020 17:29:51 -0700
Subject: [PATCH 10/14] Make Downgrade test rare

---
 tests/CMakeLists.txt     | 2 +-
 tests/fast/Downgrade.txt | 4 ----
 tests/rare/Downgrade.txt | 4 ++++
 3 files changed, 5 insertions(+), 5 deletions(-)
 delete mode 100644 tests/fast/Downgrade.txt
 create mode 100644 tests/rare/Downgrade.txt

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 289e666a1d..3861ad66f9 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -116,7 +116,6 @@ if(WITH_PYTHON)
   add_fdb_test(TEST_FILES fast/ConstrainedRandomSelector.txt)
   add_fdb_test(TEST_FILES fast/CycleAndLock.txt)
   add_fdb_test(TEST_FILES fast/CycleTest.txt)
-  add_fdb_test(TEST_FILES fast/Downgrade.txt)
   add_fdb_test(TEST_FILES fast/FuzzApiCorrectness.txt)
   add_fdb_test(TEST_FILES fast/FuzzApiCorrectnessClean.txt)
   add_fdb_test(TEST_FILES fast/IncrementTest.txt)
@@ -152,6 +151,7 @@ if(WITH_PYTHON)
   add_fdb_test(TEST_FILES rare/ConflictRangeRYOWCheck.txt)
   add_fdb_test(TEST_FILES rare/CycleRollbackClogged.txt)
   add_fdb_test(TEST_FILES rare/CycleWithKills.txt)
+  add_fdb_test(TEST_FILES rare/Downgrade.txt)
   add_fdb_test(TEST_FILES rare/FuzzTest.txt)
   add_fdb_test(TEST_FILES rare/InventoryTestHeavyWrites.txt)
   add_fdb_test(TEST_FILES rare/LargeApiCorrectness.txt)
diff --git a/tests/fast/Downgrade.txt b/tests/fast/Downgrade.txt
deleted file mode 100644
index 6dd25105c8..0000000000
--- a/tests/fast/Downgrade.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-testTitle=Downgrade
-    testName=Downgrade
-    oldKey=oldKey
-    newKey=newKey
diff --git a/tests/rare/Downgrade.txt b/tests/rare/Downgrade.txt
new file mode 100644
index 0000000000..9dfe605de4
--- /dev/null
+++ b/tests/rare/Downgrade.txt
@@ -0,0 +1,4 @@
+testTitle=Downgrade
+  testName=Downgrade
+  oldKey=oldKey
+  newKey=newKey

From 369d1be562644010d64fa3be8fd8fb28229868d5 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Sun, 12 Jul 2020 16:06:23 -0700
Subject: [PATCH 11/14] Fixed test harness bug

---
 contrib/TestHarness/Program.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/TestHarness/Program.cs b/contrib/TestHarness/Program.cs
index ab905deb89..8d58d8fe2a 100644
--- a/contrib/TestHarness/Program.cs
+++ b/contrib/TestHarness/Program.cs
@@ -301,7 +301,7 @@ namespace SummarizeTest
                     int expectedUnseed = -1;
                     int unseed;
                     string uid = Guid.NewGuid().ToString();
-                    bool useNewPlugin = isDowngrade || versionGreaterThanOrEqual(oldServerName.Split('-').Last(), "5.2.0");
+                    bool useNewPlugin = (oldServerName == fdbserverName) || versionGreaterThanOrEqual(oldServerName.Split('-').Last(), "5.2.0");
                     result = RunTest(firstServerName, useNewPlugin ? tlsPluginFile : tlsPluginFile_5_1, summaryFileName, errorFileName, seed, buggify, testFile + "-1.txt", runDir, uid, expectedUnseed, out unseed, out retryableError, logOnRetryableError, useValgrind, false, true, oldServerName, traceToStdout);
                     if (result == 0)
                     {

From de2b35f72788ff89431032df8c2b362da040a428 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Sun, 12 Jul 2020 16:06:56 -0700
Subject: [PATCH 12/14] Simplified Downgrade workload

---
 fdbserver/workloads/Downgrade.actor.cpp | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fdbserver/workloads/Downgrade.actor.cpp b/fdbserver/workloads/Downgrade.actor.cpp
index cc2c35abf5..4ec5704f6f 100644
--- a/fdbserver/workloads/Downgrade.actor.cpp
+++ b/fdbserver/workloads/Downgrade.actor.cpp
@@ -36,15 +36,13 @@ struct DowngradeWorkload : TestWorkload {
 		numObjects = getOption(options, LiteralStringRef("numOptions"), deterministicRandom()->randomInt(0,100));
 	}
 
-	template <class Impl>
 	struct _Struct {
 		static constexpr FileIdentifier file_identifier = 2340487;
 		int oldField = 0;
 	};
 
-	struct OldStruct : public _Struct<OldStruct> {
+	struct OldStruct : public _Struct {
 		void setFields() { oldField = 1; }
-
 		bool isSet() const { return oldField == 1; }
 
 		template <class Archive>
@@ -53,13 +51,12 @@ struct DowngradeWorkload : TestWorkload {
 		}
 	};
 
-	struct NewStruct : public _Struct<NewStruct> {
+	struct NewStruct : public _Struct {
 		int newField = 0;
 
 		bool isSet() const {
 			return oldField == 1 && newField == 2;
 		}
-
 		void setFields() {
 			oldField = 1;
 			newField = 2;
@@ -74,8 +71,8 @@ struct DowngradeWorkload : TestWorkload {
 	ACTOR static Future<Void> writeOld(Database cx, int numObjects, Key key) {
 		BinaryWriter writer(IncludeVersion(currentProtocolVersion));
 		std::vector<OldStruct> data(numObjects);
-		for (auto& oldStruct : data) {
-			oldStruct.setFields();
+		for (auto& oldObject : data) {
+			oldObject.setFields();
 		}
 		writer << data;
 		state Value value = writer.toValue();

From 92ac2a99b97886f28c334f250065de8bcc6c6c37 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Mon, 20 Jul 2020 18:45:28 -0700
Subject: [PATCH 13/14] Ignore file identifier mismatches during 7.0 -> 6.3
 downgrade

---
 flow/ObjectSerializer.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/flow/ObjectSerializer.h b/flow/ObjectSerializer.h
index 7682d88c7b..e9326e61ad 100644
--- a/flow/ObjectSerializer.h
+++ b/flow/ObjectSerializer.h
@@ -80,8 +80,19 @@ public:
 		const uint8_t* data = static_cast<ReaderImpl*>(this)->data();
 		LoadContext<ReaderImpl> context(static_cast<ReaderImpl*>(this));
 		if(read_file_identifier(data) != file_identifier) {
-			TraceEvent(SevError, "MismatchedFileIdentifier").detail("Expected", file_identifier).detail("Read", read_file_identifier(data));
-			ASSERT(false);
+			// Some file identifiers are changed in 7.0, so file identifier mismatches
+			// are expected during a downgrade from 7.0 to 6.3
+			bool expectMismatch = mProtocolVersion >= ProtocolVersion(0x0FDB00B070000000LL);
+			{
+				TraceEvent te(expectMismatch ? SevInfo : SevError, "MismatchedFileIdentifier");
+				if (expectMismatch) {
+					te.suppressFor(1.0);
+				}
+				te.detail("Expected", file_identifier).detail("Read", read_file_identifier(data));
+			}
+			if (!expectMismatch) {
+				ASSERT(false);
+			}
 		}
 		load_members(data, context, items...);
 	}

From a1d3496d7d9be61aa1273b1b380f05e69be5ca15 Mon Sep 17 00:00:00 2001
From: sfc-gh-tclinkenbeard <trevor.clinkenbeard@snowflake.com>
Date: Mon, 20 Jul 2020 18:47:43 -0700
Subject: [PATCH 14/14] Update downgrade tests to use 6.3.5 instead of 6.3.3

---
 tests/CMakeLists.txt                                          | 4 ++--
 .../restarting/{to_6.3.3 => to_6.3.5}/CycleTestRestart-1.txt  | 0
 .../restarting/{to_6.3.3 => to_6.3.5}/CycleTestRestart-2.txt  | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename tests/restarting/{to_6.3.3 => to_6.3.5}/CycleTestRestart-1.txt (100%)
 rename tests/restarting/{to_6.3.3 => to_6.3.5}/CycleTestRestart-2.txt (100%)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 3861ad66f9..91a695ab58 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -191,8 +191,8 @@ if(WITH_PYTHON)
     TEST_FILES restarting/from_5.2.0/ClientTransactionProfilingCorrectness-1.txt
                restarting/from_5.2.0/ClientTransactionProfilingCorrectness-2.txt)
   add_fdb_test(
-    TEST_FILES restarting/to_6.3.3/CycleTestRestart-1.txt
-               restarting/to_6.3.3/CycleTestRestart-2.txt)
+    TEST_FILES restarting/to_6.3.5/CycleTestRestart-1.txt
+               restarting/to_6.3.5/CycleTestRestart-2.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectness.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectnessAtomicRestore.txt)
   add_fdb_test(TEST_FILES slow/ApiCorrectnessSwitchover.txt)
diff --git a/tests/restarting/to_6.3.3/CycleTestRestart-1.txt b/tests/restarting/to_6.3.5/CycleTestRestart-1.txt
similarity index 100%
rename from tests/restarting/to_6.3.3/CycleTestRestart-1.txt
rename to tests/restarting/to_6.3.5/CycleTestRestart-1.txt
diff --git a/tests/restarting/to_6.3.3/CycleTestRestart-2.txt b/tests/restarting/to_6.3.5/CycleTestRestart-2.txt
similarity index 100%
rename from tests/restarting/to_6.3.3/CycleTestRestart-2.txt
rename to tests/restarting/to_6.3.5/CycleTestRestart-2.txt