From edf52e8c97050e62f0b9f7d07688ab3ca7ce6b65 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 10 Oct 2019 15:42:52 -0700
Subject: [PATCH 01/45] First version for reporting conflicting keys

---
 bindings/flow/tester/Tester.actor.cpp |  2 ++
 fdbclient/CommitTransaction.h         |  9 ++++--
 fdbclient/MasterProxyInterface.h      | 16 +++++++---
 fdbclient/NativeAPI.actor.cpp         | 16 +++++++++-
 fdbclient/NativeAPI.actor.h           |  3 ++
 fdbclient/ReadYourWrites.actor.cpp    | 18 +++++++++++
 fdbclient/vexillographer/fdb.options  |  5 +++
 fdbserver/ConflictSet.h               |  3 +-
 fdbserver/MasterProxyServer.actor.cpp | 35 +++++++++++++++++++--
 fdbserver/Resolver.actor.cpp          |  5 ++-
 fdbserver/ResolverInterface.h         |  3 +-
 fdbserver/SkipList.cpp                | 44 +++++++++++++++++++--------
 fdbserver/workloads/Mako.actor.cpp    | 16 ++++++++--
 tests/Mako.txt                        |  6 ++--
 14 files changed, 150 insertions(+), 31 deletions(-)

diff --git a/bindings/flow/tester/Tester.actor.cpp b/bindings/flow/tester/Tester.actor.cpp
index 52d193320e..508d3ae30f 100644
--- a/bindings/flow/tester/Tester.actor.cpp
+++ b/bindings/flow/tester/Tester.actor.cpp
@@ -1584,6 +1584,7 @@ struct UnitTestsFunc : InstructionFunc {
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_RETRY_LIMIT, Optional<StringRef>(StringRef((const uint8_t*)&noRetryLimit, 8)));
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_CAUSAL_READ_RISKY);
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_INCLUDE_PORT_IN_ADDRESS);
+		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_REPORT_CONFLICTING_KEYS);
 
 		state Reference<Transaction> tr = data->db->createTransaction();
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_PRIORITY_SYSTEM_IMMEDIATE);
@@ -1603,6 +1604,7 @@ struct UnitTestsFunc : InstructionFunc {
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_READ_LOCK_AWARE);
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_LOCK_AWARE);
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_INCLUDE_PORT_IN_ADDRESS);
+		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_REPORT_CONFLICTING_KEYS);
 
 		Optional<FDBStandalone<ValueRef> > _ = wait(tr->get(LiteralStringRef("\xff")));
 		tr->cancel();
diff --git a/fdbclient/CommitTransaction.h b/fdbclient/CommitTransaction.h
index 5ebb245c72..700cf75a4f 100644
--- a/fdbclient/CommitTransaction.h
+++ b/fdbclient/CommitTransaction.h
@@ -137,21 +137,23 @@ static inline bool isNonAssociativeOp(MutationRef::Type mutationType) {
 }
 
 struct CommitTransactionRef {
-	CommitTransactionRef() : read_snapshot(0) {}
+	CommitTransactionRef() : read_snapshot(0), report_conflicting_keys(false) {}
 	CommitTransactionRef(Arena &a, const CommitTransactionRef &from)
 	  : read_conflict_ranges(a, from.read_conflict_ranges),
 		write_conflict_ranges(a, from.write_conflict_ranges),
 		mutations(a, from.mutations),
-		read_snapshot(from.read_snapshot) {
+		read_snapshot(from.read_snapshot),
+		report_conflicting_keys(from.report_conflicting_keys) {
 	}
 	VectorRef< KeyRangeRef > read_conflict_ranges;
 	VectorRef< KeyRangeRef > write_conflict_ranges;
 	VectorRef< MutationRef > mutations;
 	Version read_snapshot;
+	bool report_conflicting_keys;
 
 	template <class Ar>
 	force_inline void serialize( Ar& ar ) {
-		serializer(ar, read_conflict_ranges, write_conflict_ranges, mutations, read_snapshot);
+		serializer(ar, read_conflict_ranges, write_conflict_ranges, mutations, read_snapshot, report_conflicting_keys);
 	}
 
 	// Convenience for internal code required to manipulate these without the Native API
@@ -161,6 +163,7 @@ struct CommitTransactionRef {
 	}
 
 	void clear( Arena& arena, KeyRangeRef const& keys ) {
+		// TODO: check do I need to clear flag here
 		mutations.push_back_deep(arena, MutationRef(MutationRef::ClearRange, keys.begin, keys.end));
 		write_conflict_ranges.push_back_deep(arena, keys);
 	}
diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index 5b00fd5008..ae0e76ce36 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -103,26 +103,30 @@ struct CommitID {
 	constexpr static FileIdentifier file_identifier = 14254927;
 	Version version; 			// returns invalidVersion if transaction conflicts
 	uint16_t txnBatchId;
-	Optional<Value> metadataVersion;
+	Optional<Value> metadataVersion;	
+	// TODO : data structure okay here ?
+	Optional<Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
 
 	template <class Ar>
 	void serialize(Ar& ar) {
-		serializer(ar, version, txnBatchId, metadataVersion);
+		serializer(ar, version, txnBatchId, metadataVersion, conflictingKeyRanges);
 	}
 
 	CommitID() : version(invalidVersion), txnBatchId(0) {}
-	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion) {}
+	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<Standalone<VectorRef<KeyRangeRef>>>& conflictingKeyRanges = Optional<Standalone<VectorRef<KeyRangeRef>>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKeyRanges(conflictingKeyRanges) {}
 };
 
 struct CommitTransactionRequest : TimedRequest {
 	constexpr static FileIdentifier file_identifier = 93948;
 	enum { 
 		FLAG_IS_LOCK_AWARE = 0x1,
-		FLAG_FIRST_IN_BATCH = 0x2
+		FLAG_FIRST_IN_BATCH = 0x2,
+		FLAG_REPORT_CONFLICTING_KEYS = 0x4
 	};
 
 	bool isLockAware() const { return (flags & FLAG_IS_LOCK_AWARE) != 0; }
 	bool firstInBatch() const { return (flags & FLAG_FIRST_IN_BATCH) != 0; }
+	bool isReportConflictingKeys()  const { return (flags & FLAG_REPORT_CONFLICTING_KEYS) != 0; }
 	
 	Arena arena;
 	CommitTransactionRef transaction;
@@ -136,6 +140,10 @@ struct CommitTransactionRequest : TimedRequest {
 	void serialize(Ar& ar) { 
 		serializer(ar, transaction, reply, arena, flags, debugID);
 	}
+
+	void reportConflictingKeys(){
+		transaction.report_conflicting_keys = true;
+	}
 };
 
 static inline int getBytes( CommitTransactionRequest const& r ) {
diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 34bbc60ed3..6195434854 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2613,7 +2613,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 				proxy_memory_limit_exceeded(),
 				commit_unknown_result()});
 	}
-
+	
 	try {
 		Version v = wait( readVersion );
 		req.transaction.read_snapshot = v;
@@ -2673,6 +2673,10 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					}
 					return Void();
 				} else {
+					if (ci.conflictingKeyRanges.present()){
+						tr->info.conflictingKeyRanges.push_back_deep(tr->info.conflictingKeyRanges.arena(), ci.conflictingKeyRanges.get());
+					}
+
 					if (info.debugID.present())
 						TraceEvent(interval.end()).detail("Conflict", 1);
 
@@ -2784,6 +2788,11 @@ Future<Void> Transaction::commitMutations() {
 		if(options.firstInBatch) {
 			tr.flags = tr.flags | CommitTransactionRequest::FLAG_FIRST_IN_BATCH;
 		}
+		if(options.reportConflictingKeys) {
+			// TODO : Is it better to keep it as a flag?
+			tr.flags = tr.flags | CommitTransactionRequest::FLAG_REPORT_CONFLICTING_KEYS;
+			tr.reportConflictingKeys();
+		}
 
 		Future<Void> commitResult = tryCommit( cx, trLogInfo, tr, readVersion, info, &this->committedVersion, this, options );
 
@@ -2974,6 +2983,11 @@ void Transaction::setOption( FDBTransactionOptions::Option option, Optional<Stri
 			validateOptionValue(value, false);
 			options.includePort = true;
 			break;
+		
+		case FDBTransactionOptions::REPORT_CONFLICTING_KEYS:
+			validateOptionValue(value, false);
+			options.reportConflictingKeys = true;
+			break;
 
 		default:
 			break;
diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h
index eadca69fe0..503ffc7f73 100644
--- a/fdbclient/NativeAPI.actor.h
+++ b/fdbclient/NativeAPI.actor.h
@@ -132,6 +132,7 @@ struct TransactionOptions {
 	bool readOnly : 1;
 	bool firstInBatch : 1;
 	bool includePort : 1;
+	bool reportConflictingKeys : 1;
 
 	TransactionOptions(Database const& cx);
 	TransactionOptions();
@@ -143,6 +144,7 @@ struct TransactionInfo {
 	Optional<UID> debugID;
 	TaskPriority taskID;
 	bool useProvisionalProxies;
+	Standalone<VectorRef<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
 
 	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false) {}
 };
@@ -271,6 +273,7 @@ public:
 	void reset();
 	void fullReset();
 	double getBackoff(int errCode);
+
 	void debugTransaction(UID dID) { info.debugID = dID; }
 
 	Future<Void> commitMutations();
diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index c41739d907..e459d05af8 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -23,6 +23,7 @@
 #include "fdbclient/DatabaseContext.h"
 #include "fdbclient/StatusClient.h"
 #include "fdbclient/MonitorLeader.h"
+#include "fdbclient/JsonBuilder.h"
 #include "flow/Util.h"
 #include "flow/actorcompiler.h"  // This must be the last #include.
 
@@ -1228,6 +1229,23 @@ Future< Optional<Value> > ReadYourWritesTransaction::get( const Key& key, bool s
 		return Optional<Value>();
 	}
 
+	// TODO : add conflict keys to special key space
+	if (key == LiteralStringRef("\xff\xff/conflicting_keys/json")){
+		if (!tr.info.conflictingKeyRanges.empty()){
+			// TODO : return a json value which represents all the values
+			JsonBuilderArray conflictingKeysArray; 
+			for (auto & cKR : tr.info.conflictingKeyRanges) {
+				for (auto & kr : cKR) {
+					conflictingKeysArray.push_back(format("[%s, %s)", kr.begin.toString().c_str(), kr.end.toString().c_str()));
+				}
+			}
+			Optional<Value> output = StringRef(conflictingKeysArray.getJson());
+			return output;
+		} else {
+			return Optional<Value>();
+		}
+	}
+
 	if(checkUsedDuringCommit()) {
 		return used_during_commit();
 	}
diff --git a/fdbclient/vexillographer/fdb.options b/fdbclient/vexillographer/fdb.options
index 890dea4864..035335e2a2 100644
--- a/fdbclient/vexillographer/fdb.options
+++ b/fdbclient/vexillographer/fdb.options
@@ -174,6 +174,9 @@ description is not currently required but encouraged.
     <Option name="transaction_include_port_in_address" code="505"
             description="Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated."
             defaultFor="23"/>
+    <Option name="transaction_report_conflicting_keys" code="506"
+            description="Report conflicting keys for transactions that are determined as conflicts by resolvers."
+            defaultFor="712"/>
   </Scope>
   
   <Scope name="TransactionOption">
@@ -250,6 +253,8 @@ description is not currently required but encouraged.
             hidden="true" />
     <Option name="use_provisional_proxies" code="711"
             description="This option should only be used by tools which change the database configuration." />
+    <Option name="report_conflicting_keys" code="712"
+            description="The transaction will save conflicting keys if conflicts happen." />
   </Scope>
 
   <!-- The enumeration values matter - do not change them without
diff --git a/fdbserver/ConflictSet.h b/fdbserver/ConflictSet.h
index 117d94785e..09e1fb5d14 100644
--- a/fdbserver/ConflictSet.h
+++ b/fdbserver/ConflictSet.h
@@ -33,7 +33,7 @@ void clearConflictSet( ConflictSet*, Version );
 void destroyConflictSet(ConflictSet*);
 
 struct ConflictBatch {
-	explicit ConflictBatch( ConflictSet* );
+	explicit ConflictBatch( ConflictSet*,  std::map< int, Standalone<VectorRef<KeyRangeRef>>>* conflictingKeyRangeMap = nullptr);
 	~ConflictBatch();
 
 	enum TransactionCommitResult {
@@ -54,6 +54,7 @@ private:
 	std::vector< std::pair<StringRef,StringRef> > combinedWriteConflictRanges;
 	std::vector< struct ReadConflictRange > combinedReadConflictRanges;
 	bool* transactionConflictStatus;
+	std::map< int, Standalone<VectorRef<KeyRangeRef>> >* conflictingKeyRangeMap;
 
 	void checkIntraBatchConflicts();
 	void combineWriteConflictRanges();
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index c573f33187..1c1867bb8b 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -356,8 +356,10 @@ struct ResolutionRequestBuilder {
 
 		vector<int> resolversUsed;
 		for (int r = 0; r<outTr.size(); r++)
-			if (outTr[r])
+			if (outTr[r]) {
 				resolversUsed.push_back(r);
+				outTr[r]->report_conflicting_keys = trIn.report_conflicting_keys;
+			}
 		transactionResolverMap.push_back(std::move(resolversUsed));
 	}
 };
@@ -1026,7 +1028,36 @@ ACTOR Future<Void> commitBatch(
 			trs[t].reply.sendError(transaction_too_old());
 		}
 		else {
-			trs[t].reply.sendError(not_committed());
+			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
+			if (trs[t].isReportConflictingKeys()) {
+				Standalone<VectorRef<KeyRangeRef>> conflictingEntries;
+				std::vector<KeyRange> unmergedConflictingKRs;
+				for (int resolverInd = 0; resolverInd < resolution.size(); ++resolverInd){
+					for (const KeyRangeRef & kr : resolution[resolverInd].conflictingKeyRangeMap[t]) {
+						unmergedConflictingKRs.emplace_back(kr);
+						// TraceEvent("ConflictingKeyRange").detail("ResolverIndex", resolverInd).detail("TrasactionIndex", t).detail("StartKey", kr.begin.toString()).detail("EndKey", kr.end.toString());
+					}
+				}
+				// Sort the keyranges by begin key, then union overlap ranges from left to right
+				std::sort(unmergedConflictingKRs.begin(), unmergedConflictingKRs.end(), [](KeyRange a, KeyRange b){
+					return a.begin < b.begin;
+				});
+				auto next = unmergedConflictingKRs.begin();
+				// At least one conflicting keyrange should be returned, which means curr is valid
+				KeyRangeRef curr = *(next++);
+				while (next != unmergedConflictingKRs.end()) {
+					if (curr.end >= next->begin) {
+						curr = KeyRangeRef(curr.begin, std::max(curr.end, next->end));
+					} else {
+						conflictingEntries.push_back_deep(conflictingEntries.arena(), curr);
+						curr = *next;
+					}
+					next++;
+				}
+				conflictingEntries.push_back_deep(conflictingEntries.arena(), curr);
+				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<KeyRangeRef>>>(conflictingEntries)));
+			} else
+				trs[t].reply.sendError(not_committed());
 		}
 
 		// TODO: filter if pipelined with large commit
diff --git a/fdbserver/Resolver.actor.cpp b/fdbserver/Resolver.actor.cpp
index 41834bb163..7f341271a1 100644
--- a/fdbserver/Resolver.actor.cpp
+++ b/fdbserver/Resolver.actor.cpp
@@ -132,11 +132,12 @@ ACTOR Future<Void> resolveBatch(
 		
 		vector<int> commitList;
 		vector<int> tooOldList;
+		std::map<int, Standalone<VectorRef<KeyRangeRef>> > conflictingKeyRangeMap;
 
 		// Detect conflicts
 		double expire = now() + SERVER_KNOBS->SAMPLE_EXPIRATION_TIME;
 		double tstart = timer();
-		ConflictBatch conflictBatch( self->conflictSet );
+		ConflictBatch conflictBatch(self->conflictSet, &conflictingKeyRangeMap);
 		int keys = 0;
 		for(int t=0; t<req.transactions.size(); t++) {
 			conflictBatch.addTransaction( req.transactions[t] );
@@ -163,6 +164,8 @@ ACTOR Future<Void> resolveBatch(
 
 		for (int c = 0; c<tooOldList.size(); c++)
 			reply.committed[tooOldList[c]] = ConflictBatch::TransactionTooOld;
+		
+		reply.conflictingKeyRangeMap = std::move(conflictingKeyRangeMap);
 
 		ASSERT(req.prevVersion >= 0 || req.txnStateTransactions.size() == 0); // The master's request should not have any state transactions
 
diff --git a/fdbserver/ResolverInterface.h b/fdbserver/ResolverInterface.h
index 029bde6475..f3b7f6c1f5 100644
--- a/fdbserver/ResolverInterface.h
+++ b/fdbserver/ResolverInterface.h
@@ -77,10 +77,11 @@ struct ResolveTransactionBatchReply {
 	VectorRef<uint8_t> committed;
 	Optional<UID> debugID;
 	VectorRef<VectorRef<StateTransactionRef>> stateMutations;  // [version][transaction#] -> (committed, [mutation#])
+	std::map<int, Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRangeMap; // resolver index -> conflicting keyRanges given by that resolver
 
 	template <class Archive>
 	void serialize(Archive& ar) {
-		serializer(ar, committed, stateMutations, arena, debugID);
+		serializer(ar, committed, stateMutations, arena, debugID, conflictingKeyRangeMap);
 	}
 
 };
diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 62f22a66b9..184f154876 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -123,8 +123,9 @@ struct ReadConflictRange {
 	StringRef begin, end;
 	Version version;
 	int transaction;
-	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction )
-		: begin(begin), end(end), version(version), transaction(transaction)
+	Standalone<VectorRef<KeyRangeRef>> * conflictingKeyRange;
+	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, Standalone<VectorRef<KeyRangeRef>> * cKR = nullptr )
+		: begin(begin), end(end), version(version), transaction(transaction), conflictingKeyRange(cKR)
 	{
 	}
 	bool operator<(const ReadConflictRange& rhs) const { return compare(begin, rhs.begin)<0; }
@@ -529,7 +530,7 @@ public:
 
 		int started = min(M,count);
 		for(int i=0; i<started; i++){
-			inProgress[i].init( ranges[i], header, transactionConflictStatus );
+			inProgress[i].init( ranges[i], header, transactionConflictStatus, ranges[i].conflictingKeyRange );
 			nextJob[i] = i+1;
 		}
 		nextJob[started-1] = 0;
@@ -544,8 +545,10 @@ public:
 					nextJob[prevJob] = nextJob[job];
 					job = prevJob;
 				}
-				else
-					inProgress[job].init( ranges[started++], header, transactionConflictStatus );
+				else {
+					int temp = started++;
+					inProgress[job].init( ranges[temp], header, transactionConflictStatus, ranges[temp].conflictingKeyRange );
+				}
 			}
 			prevJob = job;
 			job = nextJob[job];
@@ -757,17 +760,25 @@ private:
 		Version version;
 		bool *result;
 		int state;
+		Standalone<VectorRef<KeyRangeRef>>* conflictingKeyRange;
 
-		void init( const ReadConflictRange& r, Node* header, bool* tCS ) {
+		void init( const ReadConflictRange& r, Node* header, bool* tCS, Standalone<VectorRef<KeyRangeRef>>* cKR) {
 			this->start.init( r.begin, header );
 			this->end.init( r.end, header );
 			this->version = r.version;
 			result = &tCS[ r.transaction ];
+			conflictingKeyRange = cKR;
 			this->state = 0;
 		}
 
 		bool noConflict() { return true; }
-		bool conflict() { *result = true; return true; }
+		bool conflict() { 
+			*result = true;
+			if(conflictingKeyRange != nullptr){
+				conflictingKeyRange->push_back_deep(conflictingKeyRange->arena(), KeyRangeRef(start.value, end.value));
+			}
+			return true;
+		}
 
 		// Return true if finished
 		force_inline bool advance() {
@@ -961,8 +972,8 @@ void destroyConflictSet(ConflictSet* cs) {
 	delete cs;
 }
 
-ConflictBatch::ConflictBatch( ConflictSet* cs )
-	: cs(cs), transactionCount(0)
+ConflictBatch::ConflictBatch( ConflictSet* cs, std::map< int, Standalone<VectorRef<KeyRangeRef>> >* conflictingKeyRangeMap )
+	: cs(cs), transactionCount(0), conflictingKeyRangeMap(conflictingKeyRangeMap)
 {
 }
 
@@ -974,6 +985,7 @@ struct TransactionInfo {
 	VectorRef< std::pair<int,int> > readRanges;
 	VectorRef< std::pair<int,int> > writeRanges;
 	bool tooOld;
+	bool reportConflictingKeys;
 };
 
 void ConflictBatch::addTransaction( const CommitTransactionRef& tr ) {
@@ -981,6 +993,7 @@ void ConflictBatch::addTransaction( const CommitTransactionRef& tr ) {
 
 	Arena& arena = transactionInfo.arena();
 	TransactionInfo* info = new (arena) TransactionInfo;
+	info->reportConflictingKeys = tr.report_conflicting_keys;
 
 	if (tr.read_snapshot < cs->oldestVersion && tr.read_conflict_ranges.size()) {
 		info->tooOld = true;
@@ -995,7 +1008,7 @@ void ConflictBatch::addTransaction( const CommitTransactionRef& tr ) {
 			points.emplace_back(range.begin, false, true, false, t, &info->readRanges[r].first);
 			//points.back().keyEnd = StringRef(buf,range.second);
 			points.emplace_back(range.end, false, false, false, t, &info->readRanges[r].second);
-			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t);
+			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t, tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t] : nullptr);
 		}
 		for(int r=0; r<tr.write_conflict_ranges.size(); r++) {
 			const KeyRangeRef& range = tr.write_conflict_ranges[r];
@@ -1129,7 +1142,6 @@ public:
 	}
 };
 
-
 void ConflictBatch::checkIntraBatchConflicts() {
 	int index = 0;
 	for(int p=0; p<points.size(); p++)
@@ -1140,11 +1152,17 @@ void ConflictBatch::checkIntraBatchConflicts() {
 		const TransactionInfo& tr = *transactionInfo[t];
 		if (transactionConflictStatus[t]) continue;
 		bool conflict = tr.tooOld;
-		for(int i=0; i<tr.readRanges.size(); i++)
-			if ( mcs.any( tr.readRanges[i].first, tr.readRanges[i].second ) ) {
+		for(int i=0; i<tr.readRanges.size(); i++){
+			int startPointIndex = tr.readRanges[i].first;
+			int endPointIndex = tr.readRanges[i].second;
+			if ( mcs.any(startPointIndex , endPointIndex ) ) {
+				if (tr.reportConflictingKeys){
+					(*conflictingKeyRangeMap)[t].push_back_deep((*conflictingKeyRangeMap)[t].arena(), KeyRangeRef(points[startPointIndex].key, points[endPointIndex].key));
+				}
 				conflict = true;
 				break;
 			}
+		}
 		transactionConflictStatus[t] = conflict;
 		if (!conflict)
 			for(int i=0; i<tr.writeRanges.size(); i++)
diff --git a/fdbserver/workloads/Mako.actor.cpp b/fdbserver/workloads/Mako.actor.cpp
index 044ee49cbe..a1a73fcae2 100644
--- a/fdbserver/workloads/Mako.actor.cpp
+++ b/fdbserver/workloads/Mako.actor.cpp
@@ -14,7 +14,7 @@ constexpr int RANGELIMIT = 10000;
 struct MakoWorkload : TestWorkload {
 	uint64_t rowCount, seqNumLen, sampleSize, actorCountPerClient, keyBytes, maxValueBytes, minValueBytes;
 	double testDuration, loadTime, warmingDelay, maxInsertRate, transactionsPerSecond, allowedLatency, periodicLoggingInterval, zipfConstant;
-	bool enableLogging, commitGet, populateData, runBenchmark, preserveData, zipf;
+	bool enableLogging, commitGet, populateData, runBenchmark, preserveData, zipf, reportConflictingKeys;
 	PerfIntCounter xacts, retries, conflicts, commits, totalOps;
 	std::vector<PerfIntCounter> opCounters;
 	std::vector<uint64_t> insertionCountsToMeasure;
@@ -61,6 +61,8 @@ struct MakoWorkload : TestWorkload {
 		// If true, the workload will picking up keys which are zipfian distributed
 		zipf = getOption(options, LiteralStringRef("zipf"), false);
 		zipfConstant = getOption(options, LiteralStringRef("zipfConstant"), 0.99);
+		// If true, return key ranges conflicting with other txs
+		reportConflictingKeys = getOption(options, LiteralStringRef("reportConflictingKeys"), false);
 		// Specified length of keys and length range of values
 		keyBytes = std::max( getOption( options, LiteralStringRef("keyBytes"), 16 ), 16);
 		maxValueBytes = getOption( options, LiteralStringRef("valueBytes"), 16 );
@@ -290,6 +292,8 @@ struct MakoWorkload : TestWorkload {
 			// used for throttling
 			wait(poisson(&lastTime, delay));
 			try{
+				if (self->reportConflictingKeys)
+					tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
 				// user-defined value: whether commit read-only ops or not; default is false
 				doCommit = self->commitGet;
 				for (i = 0; i < MAX_OP; ++i) {
@@ -394,11 +398,19 @@ struct MakoWorkload : TestWorkload {
 				TraceEvent("FailedToExecOperations").error(e);
 				if (e.code() == error_code_operation_cancelled)
 					throw;
-				else if (e.code() == error_code_not_committed)
+				else if (e.code() == error_code_not_committed){
 					++self->conflicts;
+				}
 				
 				wait(tr.onError(e));
 				++self->retries;
+				if (self->reportConflictingKeys && deterministicRandom()->random01() < 0.01){
+					// Standalone<VectorRef<KeyRangeRef>> rCKs;
+					// tr.extractConflictingKeys(rCKs);
+					// TraceEvent("ReportConflictingKeys").detail("KeySize", rCKs.size()).detail("KeyStart", rCKs[0].begin.toString()).detail("KeyEnd", rCKs[0].end.toString());
+					Optional<Standalone<StringRef>> temp = wait(tr.get(LiteralStringRef("\xff\xff/conflicting_keys/json")));
+					TraceEvent("ReportConflictingKeys").detail("Log", temp.present() ? temp.get().toString() : "DEBUG_NO_KEYS");
+				}
 			}
 			// reset all the operations' counters to 0
 			std::fill(perOpCount.begin(), perOpCount.end(), 0);
diff --git a/tests/Mako.txt b/tests/Mako.txt
index 22927c3c85..a1efaeae62 100644
--- a/tests/Mako.txt
+++ b/tests/Mako.txt
@@ -1,15 +1,15 @@
 testTitle=MakoTest
     testName=Mako
-    testDuration=10.0
+    testDuration=20.0
     transactionsPerSecond=100000
     rows=1000000
     sampleSize=100
     valueBytes=16
     keyBytes=16
-    operations=g5gr5:10i5ir5:10grv5
+    operations=u8
     actorCountPerClient=256
     enableLogging=false
     commitGet=false
     populateData=true
     runBenchmark=true
-    preserveData=true
+    preserveData=false

From eacfdf6da3ef1471a28c20a485a7210eb3db492f Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Tue, 26 Nov 2019 00:25:37 -0800
Subject: [PATCH 02/45] Add a simple workload, ReportConflictingKeysWorkload,
 to test correctness of the API and performance overhead added to the
 resovler.

---
 bindings/flow/tester/Tester.actor.cpp         |   1 -
 fdbclient/CommitTransaction.h                 |   1 -
 fdbclient/MasterProxyInterface.h              |   7 +-
 fdbclient/NativeAPI.actor.cpp                 |   2 -
 fdbclient/ReadYourWrites.actor.cpp            |  21 +-
 fdbclient/vexillographer/fdb.options          |   3 -
 fdbserver/CMakeLists.txt                      |   1 +
 fdbserver/MasterProxyServer.actor.cpp         |  20 +-
 fdbserver/ResolverInterface.h                 |   2 +-
 fdbserver/fdbserver.vcxproj                   |   1 +
 fdbserver/fdbserver.vcxproj.filters           |   3 +
 fdbserver/workloads/Mako.actor.cpp            |  16 +-
 .../workloads/ReportConflictingKeys.actor.cpp | 188 ++++++++++++++++++
 tests/CMakeLists.txt                          |   1 +
 tests/Mako.txt                                |   6 +-
 tests/ReportConflictingKeys.txt               |  11 +
 16 files changed, 241 insertions(+), 43 deletions(-)
 create mode 100644 fdbserver/workloads/ReportConflictingKeys.actor.cpp
 create mode 100644 tests/ReportConflictingKeys.txt

diff --git a/bindings/flow/tester/Tester.actor.cpp b/bindings/flow/tester/Tester.actor.cpp
index 508d3ae30f..28a0b64f7c 100644
--- a/bindings/flow/tester/Tester.actor.cpp
+++ b/bindings/flow/tester/Tester.actor.cpp
@@ -1584,7 +1584,6 @@ struct UnitTestsFunc : InstructionFunc {
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_RETRY_LIMIT, Optional<StringRef>(StringRef((const uint8_t*)&noRetryLimit, 8)));
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_CAUSAL_READ_RISKY);
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_INCLUDE_PORT_IN_ADDRESS);
-		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_REPORT_CONFLICTING_KEYS);
 
 		state Reference<Transaction> tr = data->db->createTransaction();
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_PRIORITY_SYSTEM_IMMEDIATE);
diff --git a/fdbclient/CommitTransaction.h b/fdbclient/CommitTransaction.h
index 700cf75a4f..02b426d21f 100644
--- a/fdbclient/CommitTransaction.h
+++ b/fdbclient/CommitTransaction.h
@@ -163,7 +163,6 @@ struct CommitTransactionRef {
 	}
 
 	void clear( Arena& arena, KeyRangeRef const& keys ) {
-		// TODO: check do I need to clear flag here
 		mutations.push_back_deep(arena, MutationRef(MutationRef::ClearRange, keys.begin, keys.end));
 		write_conflict_ranges.push_back_deep(arena, keys);
 	}
diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index ae0e76ce36..1c2143a66d 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -103,8 +103,7 @@ struct CommitID {
 	constexpr static FileIdentifier file_identifier = 14254927;
 	Version version; 			// returns invalidVersion if transaction conflicts
 	uint16_t txnBatchId;
-	Optional<Value> metadataVersion;	
-	// TODO : data structure okay here ?
+	Optional<Value> metadataVersion;
 	Optional<Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
 
 	template <class Ar>
@@ -120,13 +119,11 @@ struct CommitTransactionRequest : TimedRequest {
 	constexpr static FileIdentifier file_identifier = 93948;
 	enum { 
 		FLAG_IS_LOCK_AWARE = 0x1,
-		FLAG_FIRST_IN_BATCH = 0x2,
-		FLAG_REPORT_CONFLICTING_KEYS = 0x4
+		FLAG_FIRST_IN_BATCH = 0x2
 	};
 
 	bool isLockAware() const { return (flags & FLAG_IS_LOCK_AWARE) != 0; }
 	bool firstInBatch() const { return (flags & FLAG_FIRST_IN_BATCH) != 0; }
-	bool isReportConflictingKeys()  const { return (flags & FLAG_REPORT_CONFLICTING_KEYS) != 0; }
 	
 	Arena arena;
 	CommitTransactionRef transaction;
diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 6195434854..08ac380775 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2789,8 +2789,6 @@ Future<Void> Transaction::commitMutations() {
 			tr.flags = tr.flags | CommitTransactionRequest::FLAG_FIRST_IN_BATCH;
 		}
 		if(options.reportConflictingKeys) {
-			// TODO : Is it better to keep it as a flag?
-			tr.flags = tr.flags | CommitTransactionRequest::FLAG_REPORT_CONFLICTING_KEYS;
 			tr.reportConflictingKeys();
 		}
 
diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index e459d05af8..c30bea1e1a 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -23,7 +23,6 @@
 #include "fdbclient/DatabaseContext.h"
 #include "fdbclient/StatusClient.h"
 #include "fdbclient/MonitorLeader.h"
-#include "fdbclient/JsonBuilder.h"
 #include "flow/Util.h"
 #include "flow/actorcompiler.h"  // This must be the last #include.
 
@@ -1229,17 +1228,23 @@ Future< Optional<Value> > ReadYourWritesTransaction::get( const Key& key, bool s
 		return Optional<Value>();
 	}
 
-	// TODO : add conflict keys to special key space
+	// Add conflicting keys to special key space
 	if (key == LiteralStringRef("\xff\xff/conflicting_keys/json")){
 		if (!tr.info.conflictingKeyRanges.empty()){
-			// TODO : return a json value which represents all the values
-			JsonBuilderArray conflictingKeysArray; 
-			for (auto & cKR : tr.info.conflictingKeyRanges) {
-				for (auto & kr : cKR) {
-					conflictingKeysArray.push_back(format("[%s, %s)", kr.begin.toString().c_str(), kr.end.toString().c_str()));
+			json_spirit::mArray root;
+			json_spirit::mArray keyranges;
+			json_spirit::mObject keyrange;
+			for (int i = 0; i < tr.info.conflictingKeyRanges.size(); ++i) {
+				for (const auto & kr : tr.info.conflictingKeyRanges[i]) {
+					keyrange["begin"] = kr.begin.toString();
+					keyrange["end"] = kr.end.toString();
+					keyranges.push_back(keyrange);
+					keyrange.clear();
 				}
+				root.push_back(keyranges);
+				keyranges.clear();
 			}
-			Optional<Value> output = StringRef(conflictingKeysArray.getJson());
+			Optional<Value> output = StringRef(json_spirit::write_string(json_spirit::mValue(root), json_spirit::Output_options::raw_utf8).c_str());
 			return output;
 		} else {
 			return Optional<Value>();
diff --git a/fdbclient/vexillographer/fdb.options b/fdbclient/vexillographer/fdb.options
index 035335e2a2..089bc4e1c2 100644
--- a/fdbclient/vexillographer/fdb.options
+++ b/fdbclient/vexillographer/fdb.options
@@ -174,9 +174,6 @@ description is not currently required but encouraged.
     <Option name="transaction_include_port_in_address" code="505"
             description="Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated."
             defaultFor="23"/>
-    <Option name="transaction_report_conflicting_keys" code="506"
-            description="Report conflicting keys for transactions that are determined as conflicts by resolvers."
-            defaultFor="712"/>
   </Scope>
   
   <Scope name="TransactionOption">
diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt
index 7883802e0d..e2f5a2adbe 100644
--- a/fdbserver/CMakeLists.txt
+++ b/fdbserver/CMakeLists.txt
@@ -162,6 +162,7 @@ set(FDBSERVER_SRCS
   workloads/RandomSelector.actor.cpp
   workloads/ReadWrite.actor.cpp
   workloads/RemoveServersSafely.actor.cpp
+  workloads/ReportConflictingKeys.actor.cpp
   workloads/Rollback.actor.cpp
   workloads/RyowCorrectness.actor.cpp
   workloads/RYWDisable.actor.cpp
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 1c1867bb8b..4eadeb8b1f 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -705,7 +705,9 @@ ACTOR Future<Void> commitBatch(
 	// Determine which transactions actually committed (conservatively) by combining results from the resolvers
 	state vector<uint8_t> committed(trs.size());
 	ASSERT(transactionResolverMap.size() == committed.size());
-	vector<int> nextTr(resolution.size());
+	// For each commitTransactionRef, it is only sent to resolvers specified in transactionResolverMap
+	// Thus, we use this nextTr to track the correct transaction index on each resolver.
+	state vector<int> nextTr(resolution.size());
 	for (int t = 0; t<trs.size(); t++) {
 		uint8_t commit = ConflictBatch::TransactionCommitted;
 		for (int r : transactionResolverMap[t])
@@ -1019,6 +1021,8 @@ ACTOR Future<Void> commitBatch(
 
 	// Send replies to clients
 	double endTime = timer();
+	// Reset all to zero, used to track the correct index of each commitTransacitonRef on each resolver
+	std::fill(nextTr.begin(), nextTr.end(), 0);
 	for (int t = 0; t < trs.size(); t++) {
 		if (committed[t] == ConflictBatch::TransactionCommitted && (!locked || trs[t].isLockAware())) {
 			ASSERT_WE_THINK(commitVersion != invalidVersion);
@@ -1029,15 +1033,16 @@ ACTOR Future<Void> commitBatch(
 		}
 		else {
 			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
-			if (trs[t].isReportConflictingKeys()) {
+			if (trs[t].transaction.report_conflicting_keys) {
 				Standalone<VectorRef<KeyRangeRef>> conflictingEntries;
 				std::vector<KeyRange> unmergedConflictingKRs;
-				for (int resolverInd = 0; resolverInd < resolution.size(); ++resolverInd){
-					for (const KeyRangeRef & kr : resolution[resolverInd].conflictingKeyRangeMap[t]) {
+				for (int resolverInd : transactionResolverMap[t]) {
+					for (const KeyRangeRef & kr : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
 						unmergedConflictingKRs.emplace_back(kr);
 						// TraceEvent("ConflictingKeyRange").detail("ResolverIndex", resolverInd).detail("TrasactionIndex", t).detail("StartKey", kr.begin.toString()).detail("EndKey", kr.end.toString());
 					}
 				}
+				ASSERT(unmergedConflictingKRs.size());
 				// Sort the keyranges by begin key, then union overlap ranges from left to right
 				std::sort(unmergedConflictingKRs.begin(), unmergedConflictingKRs.end(), [](KeyRange a, KeyRange b){
 					return a.begin < b.begin;
@@ -1056,10 +1061,15 @@ ACTOR Future<Void> commitBatch(
 				}
 				conflictingEntries.push_back_deep(conflictingEntries.arena(), curr);
 				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<KeyRangeRef>>>(conflictingEntries)));
-			} else
+			} else {
 				trs[t].reply.sendError(not_committed());
+			}
 		}
 
+		// Update corresponding transaction index on each resolver
+		for (int resolverInd : transactionResolverMap[t])
+			nextTr[resolverInd]++;
+
 		// TODO: filter if pipelined with large commit
 		if(self->latencyBandConfig.present()) {
 			bool filter = maxTransactionBytes > self->latencyBandConfig.get().commitConfig.maxCommitBytes.orDefault(std::numeric_limits<int>::max());
diff --git a/fdbserver/ResolverInterface.h b/fdbserver/ResolverInterface.h
index f3b7f6c1f5..7d467cc143 100644
--- a/fdbserver/ResolverInterface.h
+++ b/fdbserver/ResolverInterface.h
@@ -77,7 +77,7 @@ struct ResolveTransactionBatchReply {
 	VectorRef<uint8_t> committed;
 	Optional<UID> debugID;
 	VectorRef<VectorRef<StateTransactionRef>> stateMutations;  // [version][transaction#] -> (committed, [mutation#])
-	std::map<int, Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRangeMap; // resolver index -> conflicting keyRanges given by that resolver
+	std::map<int, Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRangeMap; // transaction index -> conflicting keyRanges given by the resolver
 
 	template <class Archive>
 	void serialize(Archive& ar) {
diff --git a/fdbserver/fdbserver.vcxproj b/fdbserver/fdbserver.vcxproj
index d2be7b965d..30eb2f74cd 100644
--- a/fdbserver/fdbserver.vcxproj
+++ b/fdbserver/fdbserver.vcxproj
@@ -169,6 +169,7 @@
     <ActorCompiler Include="workloads\SnapTest.actor.cpp" />
     <ActorCompiler Include="workloads\Mako.actor.cpp" />
     <ActorCompiler Include="workloads\ExternalWorkload.actor.cpp" />
+    <ActorCompiler Include="workloads\ReportConflictingKeys.actor.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="ApplyMetadataMutation.h" />
diff --git a/fdbserver/fdbserver.vcxproj.filters b/fdbserver/fdbserver.vcxproj.filters
index 9a1cabdec9..ed86f4a9a4 100644
--- a/fdbserver/fdbserver.vcxproj.filters
+++ b/fdbserver/fdbserver.vcxproj.filters
@@ -298,6 +298,9 @@
     <ActorCompiler Include="workloads\Mako.actor.cpp">
       <Filter>workloads</Filter>
     </ActorCompiler>
+    <ActorCompiler Include="workloads\ReportConflictingKeys.actor.cpp">
+      <Filter>workloads</Filter>
+    </ActorCompiler>
     <ActorCompiler Include="OldTLogServer.actor.cpp" />
   </ItemGroup>
   <ItemGroup>
diff --git a/fdbserver/workloads/Mako.actor.cpp b/fdbserver/workloads/Mako.actor.cpp
index a1a73fcae2..044ee49cbe 100644
--- a/fdbserver/workloads/Mako.actor.cpp
+++ b/fdbserver/workloads/Mako.actor.cpp
@@ -14,7 +14,7 @@ constexpr int RANGELIMIT = 10000;
 struct MakoWorkload : TestWorkload {
 	uint64_t rowCount, seqNumLen, sampleSize, actorCountPerClient, keyBytes, maxValueBytes, minValueBytes;
 	double testDuration, loadTime, warmingDelay, maxInsertRate, transactionsPerSecond, allowedLatency, periodicLoggingInterval, zipfConstant;
-	bool enableLogging, commitGet, populateData, runBenchmark, preserveData, zipf, reportConflictingKeys;
+	bool enableLogging, commitGet, populateData, runBenchmark, preserveData, zipf;
 	PerfIntCounter xacts, retries, conflicts, commits, totalOps;
 	std::vector<PerfIntCounter> opCounters;
 	std::vector<uint64_t> insertionCountsToMeasure;
@@ -61,8 +61,6 @@ struct MakoWorkload : TestWorkload {
 		// If true, the workload will picking up keys which are zipfian distributed
 		zipf = getOption(options, LiteralStringRef("zipf"), false);
 		zipfConstant = getOption(options, LiteralStringRef("zipfConstant"), 0.99);
-		// If true, return key ranges conflicting with other txs
-		reportConflictingKeys = getOption(options, LiteralStringRef("reportConflictingKeys"), false);
 		// Specified length of keys and length range of values
 		keyBytes = std::max( getOption( options, LiteralStringRef("keyBytes"), 16 ), 16);
 		maxValueBytes = getOption( options, LiteralStringRef("valueBytes"), 16 );
@@ -292,8 +290,6 @@ struct MakoWorkload : TestWorkload {
 			// used for throttling
 			wait(poisson(&lastTime, delay));
 			try{
-				if (self->reportConflictingKeys)
-					tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
 				// user-defined value: whether commit read-only ops or not; default is false
 				doCommit = self->commitGet;
 				for (i = 0; i < MAX_OP; ++i) {
@@ -398,19 +394,11 @@ struct MakoWorkload : TestWorkload {
 				TraceEvent("FailedToExecOperations").error(e);
 				if (e.code() == error_code_operation_cancelled)
 					throw;
-				else if (e.code() == error_code_not_committed){
+				else if (e.code() == error_code_not_committed)
 					++self->conflicts;
-				}
 				
 				wait(tr.onError(e));
 				++self->retries;
-				if (self->reportConflictingKeys && deterministicRandom()->random01() < 0.01){
-					// Standalone<VectorRef<KeyRangeRef>> rCKs;
-					// tr.extractConflictingKeys(rCKs);
-					// TraceEvent("ReportConflictingKeys").detail("KeySize", rCKs.size()).detail("KeyStart", rCKs[0].begin.toString()).detail("KeyEnd", rCKs[0].end.toString());
-					Optional<Standalone<StringRef>> temp = wait(tr.get(LiteralStringRef("\xff\xff/conflicting_keys/json")));
-					TraceEvent("ReportConflictingKeys").detail("Log", temp.present() ? temp.get().toString() : "DEBUG_NO_KEYS");
-				}
 			}
 			// reset all the operations' counters to 0
 			std::fill(perOpCount.begin(), perOpCount.end(), 0);
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
new file mode 100644
index 0000000000..78ca7e076b
--- /dev/null
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -0,0 +1,188 @@
+/*
+ * ReportConflictingKeys.actor.cpp
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2013-2018 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 "fdbserver/workloads/BulkSetup.actor.h"
+#include "fdbclient/ReadYourWrites.h"
+#include "flow/actorcompiler.h" // This must be the last #include.
+
+struct ReportConflictingKeysWorkload : TestWorkload {
+
+	double testDuration, transactionsPerSecond, addReadConflictRangeProb, addWriteConflictRangeProb;
+	Key keyPrefix;
+
+	int nodeCount, actorCount, keyBytes, valueBytes, readConflictRangeCount, writeConflictRangeCount;
+	bool reportConflictingKeys, skipCorrectnessCheck;
+
+	PerfIntCounter invalidReports, commits, conflicts, retries, xacts;
+
+	ReportConflictingKeysWorkload(WorkloadContext const& wcx)
+	  : TestWorkload(wcx), invalidReports("InvalidReports"), conflicts("Conflicts"), retries("Retries"),
+	    commits("Commits"), xacts("Transactions") {
+		testDuration = getOption(options, LiteralStringRef("testDuration"), 10.0);
+		transactionsPerSecond = getOption(options, LiteralStringRef("transactionsPerSecond"), 5000.0) / clientCount;
+		actorCount = getOption(options, LiteralStringRef("actorsPerClient"), transactionsPerSecond / 5);
+		nodeCount = getOption(options, LiteralStringRef("nodeCount"), transactionsPerSecond * clientCount);
+		keyPrefix = unprintable(
+		    getOption(options, LiteralStringRef("keyPrefix"), LiteralStringRef("ReportConflictingKeysWorkload"))
+		        .toString());
+		keyBytes = getOption(options, LiteralStringRef("keyBytes"), 16);
+		ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
+		readConflictRangeCount = getOption(options, LiteralStringRef("readConflictRangeCountPerTx"), 1);
+		writeConflictRangeCount = getOption(options, LiteralStringRef("writeConflictRangeCountPerTx"), 1);
+		// modeled by geometric distribution: (1 - prob) / prob = mean
+		addReadConflictRangeProb = readConflictRangeCount / (readConflictRangeCount + 1.0);
+		addWriteConflictRangeProb = writeConflictRangeCount / (writeConflictRangeCount + 1.0);
+		// If true, store key ranges conflicting with other txs
+		reportConflictingKeys = getOption(options, LiteralStringRef("reportConflictingKeys"), false);
+		skipCorrectnessCheck = getOption(options, LiteralStringRef("skipCorrectnessCheck"), false);
+	}
+
+	std::string description() override { return "ReportConflictingKeysWorkload"; }
+
+	Future<Void> setup(Database const& cx) override { return Void(); }
+
+	Future<Void> start(const Database& cx) override { return _start(cx->clone(), this); }
+
+	ACTOR Future<Void> _start(Database cx, ReportConflictingKeysWorkload* self) {
+		std::vector<Future<Void>> clients;
+		for (int c = 0; c < self->actorCount; ++c) {
+			clients.push_back(self->conflictingClient(cx, self, self->actorCount / self->transactionsPerSecond, c));
+		}
+
+		wait(timeout(waitForAll(clients), self->testDuration, Void()));
+		return Void();
+	}
+
+	Future<bool> check(Database const& cx) override { return invalidReports.getValue() == 0; }
+
+	void getMetrics(vector<PerfMetric>& m) override {
+		m.push_back(PerfMetric("Measured Duration", testDuration, true));
+		m.push_back(xacts.getMetric());
+		m.push_back(PerfMetric("Transactions/sec", xacts.getValue() / testDuration, true));
+		m.push_back(commits.getMetric());
+		m.push_back(PerfMetric("Commits/sec", commits.getValue() / testDuration, true));
+		m.push_back(conflicts.getMetric());
+		m.push_back(PerfMetric("Conflicts/sec", conflicts.getValue() / testDuration, true));
+		m.push_back(retries.getMetric());
+		m.push_back(PerfMetric("Retries/sec", retries.getValue() / testDuration, true));
+	}
+
+	// disable the default timeout setting
+	double getCheckTimeout() override { return std::numeric_limits<double>::max(); }
+
+	// Copied from tester.actor.cpp, added parameter to determine the key's length
+	Key keyForIndex(int n) {
+		double p = (double)n / nodeCount;
+		int paddingLen = keyBytes - 16 - keyPrefix.size();
+		// left padding by zero
+		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p)).withPrefix(keyPrefix);
+	}
+
+	void addRandomReadConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>& readConflictRanges) {
+		int startIdx, endIdx;
+		Key startKey, endKey;
+		while (deterministicRandom()->random01() < addReadConflictRangeProb) {
+			startIdx = deterministicRandom()->randomInt(0, nodeCount);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
+			startKey = keyForIndex(startIdx);
+			endKey = keyForIndex(endIdx);
+			tr->addReadConflictRange(KeyRangeRef(startKey, endKey));
+			readConflictRanges.push_back(KeyRangeRef(startKey, endKey));
+		}
+	}
+
+	void addRandomWriteConflictRange(ReadYourWritesTransaction* tr) {
+		int startIdx, endIdx;
+		Key startKey, endKey;
+		while (deterministicRandom()->random01() < addWriteConflictRangeProb) {
+			startIdx = deterministicRandom()->randomInt(0, nodeCount);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
+			startKey = keyForIndex(startIdx);
+			endKey = keyForIndex(endIdx);
+			tr->addWriteConflictRange(KeyRangeRef(startKey, endKey));
+		}
+	}
+
+	ACTOR Future<Void> conflictingClient(Database cx, ReportConflictingKeysWorkload* self, double delay,
+	                                     int actorIndex) {
+
+		state ReadYourWritesTransaction tr(cx);
+		state double lastTime = now();
+		state std::vector<KeyRange> readConflictRanges;
+
+		loop {
+			try {
+				// used for throttling
+				wait(poisson(&lastTime, delay));
+				if (self->reportConflictingKeys) tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
+				self->addRandomReadConflictRange(&tr, readConflictRanges);
+				self->addRandomWriteConflictRange(&tr);
+				++self->commits;
+				wait(tr.commit());
+				++self->xacts;
+			} catch (Error& e) {
+				TraceEvent("FailedToCommitTx").error(e);
+				state bool isConflict = false;
+				if (e.code() == error_code_operation_cancelled)
+					throw;
+				else if (e.code() == error_code_not_committed) {
+					++self->conflicts;
+					isConflict = true;
+				}
+				wait(tr.onError(e));
+				// check API correctness
+				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
+					Optional<Standalone<StringRef>> temp =
+					    wait(tr.get(LiteralStringRef("\xff\xff/conflicting_keys/json")));
+					auto jsonStr = temp.get().toString();
+					json_spirit::mValue val;
+					if (json_spirit::read_string(jsonStr, val)) {
+						auto root = val.get_array();
+						ASSERT(root.size() > 0);
+						// Only use the last entry which contains the read_conflict_ranges corresponding to current
+						// conflicts
+						for (const auto& pair : root.back().get_array()) {
+							json_spirit::mObject kr_obj = pair.get_obj();
+							KeyRange kr = KeyRangeRef(kr_obj["begin"].get_str(), kr_obj["end"].get_str());
+							if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
+								    // Read_conflict_range remains same in the resolver.
+								    // Thus, the returned keyrange is either the original read_conflict_range or merged
+								    // by several overlapped ones In either case, it contains at least one original
+								    // read_conflict_range
+								    return kr.contains(rCR);
+							    })) {
+								++self->invalidReports;
+								TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
+							}
+						}
+					}
+				}
+				++self->retries;
+			}
+			readConflictRanges.clear();
+			tr.reset();
+		}
+	}
+};
+
+WorkloadFactory<ReportConflictingKeysWorkload> ReportConflictingKeysWorkload("ReportConflictingKeys");
\ No newline at end of file
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index bf87b83c1e..8c45f4f6e4 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -65,6 +65,7 @@ add_fdb_test(TEST_FILES ReadHalfAbsent.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessUnits.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessBTree.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessPager.txt IGNORE)
+add_fdb_test(TEST_FILES ReportConflictingKeys.txt IGNORE)
 add_fdb_test(TEST_FILES fast/RedwoodCorrectnessBTree.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectness.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodPerfTests.txt IGNORE)
diff --git a/tests/Mako.txt b/tests/Mako.txt
index a1efaeae62..22927c3c85 100644
--- a/tests/Mako.txt
+++ b/tests/Mako.txt
@@ -1,15 +1,15 @@
 testTitle=MakoTest
     testName=Mako
-    testDuration=20.0
+    testDuration=10.0
     transactionsPerSecond=100000
     rows=1000000
     sampleSize=100
     valueBytes=16
     keyBytes=16
-    operations=u8
+    operations=g5gr5:10i5ir5:10grv5
     actorCountPerClient=256
     enableLogging=false
     commitGet=false
     populateData=true
     runBenchmark=true
-    preserveData=false
+    preserveData=true
diff --git a/tests/ReportConflictingKeys.txt b/tests/ReportConflictingKeys.txt
new file mode 100644
index 0000000000..8ca1763c44
--- /dev/null
+++ b/tests/ReportConflictingKeys.txt
@@ -0,0 +1,11 @@
+testTitle=ReportConflictingKeysTest
+    testName=ReportConflictingKeys
+    testDuration=10.0
+    transactionsPerSecond=100000
+    nodeCount=10000
+    actorsPerClient=16
+    keyPrefix=RCK
+    keyBytes=64
+    readConflictRangeCountPerTx=1
+    writeConflictRangeCountPerTx=1
+    reportConflictingKeys=true
\ No newline at end of file

From b376b947d5d3ea146772072a91454cb61cfe5578 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 4 Dec 2019 00:19:36 -0800
Subject: [PATCH 03/45] Encode conflicting keys using base64.

---
 fdbclient/NativeAPI.actor.cpp                 |  2 +-
 fdbclient/NativeAPI.actor.h                   |  2 +-
 fdbclient/ReadYourWrites.actor.cpp            | 20 ++++++++-----------
 fdbclient/libb64/decode.h                     | 12 +++++++++++
 .../workloads/ReportConflictingKeys.actor.cpp | 14 +++++++++----
 5 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 08ac380775..13c742a16d 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2674,7 +2674,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					return Void();
 				} else {
 					if (ci.conflictingKeyRanges.present()){
-						tr->info.conflictingKeyRanges.push_back_deep(tr->info.conflictingKeyRanges.arena(), ci.conflictingKeyRanges.get());
+						tr->info.conflictingKeyRanges = ci.conflictingKeyRanges.get();
 					}
 
 					if (info.debugID.present())
diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h
index 503ffc7f73..cc885031e8 100644
--- a/fdbclient/NativeAPI.actor.h
+++ b/fdbclient/NativeAPI.actor.h
@@ -144,7 +144,7 @@ struct TransactionInfo {
 	Optional<UID> debugID;
 	TaskPriority taskID;
 	bool useProvisionalProxies;
-	Standalone<VectorRef<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
+	Optional<Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
 
 	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false) {}
 };
diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index c30bea1e1a..808f29e22d 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -23,6 +23,7 @@
 #include "fdbclient/DatabaseContext.h"
 #include "fdbclient/StatusClient.h"
 #include "fdbclient/MonitorLeader.h"
+#include "fdbclient/libb64/encode.h"
 #include "flow/Util.h"
 #include "flow/actorcompiler.h"  // This must be the last #include.
 
@@ -1230,21 +1231,16 @@ Future< Optional<Value> > ReadYourWritesTransaction::get( const Key& key, bool s
 
 	// Add conflicting keys to special key space
 	if (key == LiteralStringRef("\xff\xff/conflicting_keys/json")){
-		if (!tr.info.conflictingKeyRanges.empty()){
+		if (tr.info.conflictingKeyRanges.present()){
 			json_spirit::mArray root;
-			json_spirit::mArray keyranges;
 			json_spirit::mObject keyrange;
-			for (int i = 0; i < tr.info.conflictingKeyRanges.size(); ++i) {
-				for (const auto & kr : tr.info.conflictingKeyRanges[i]) {
-					keyrange["begin"] = kr.begin.toString();
-					keyrange["end"] = kr.end.toString();
-					keyranges.push_back(keyrange);
-					keyrange.clear();
-				}
-				root.push_back(keyranges);
-				keyranges.clear();
+			for (const auto & kr : tr.info.conflictingKeyRanges.get()) {
+				keyrange["begin"] = base64::encoder::from_string(kr.begin.toString());
+				keyrange["end"] = base64::encoder::from_string(kr.end.toString());
+				root.push_back(keyrange);
+				keyrange.clear();
 			}
-			Optional<Value> output = StringRef(json_spirit::write_string(json_spirit::mValue(root), json_spirit::Output_options::raw_utf8).c_str());
+			Optional<Value> output = StringRef(json_spirit::write_string(json_spirit::mValue(root), json_spirit::Output_options::raw_utf8));
 			return output;
 		} else {
 			return Optional<Value>();
diff --git a/fdbclient/libb64/decode.h b/fdbclient/libb64/decode.h
index 368d00f3a0..fe71bb7946 100755
--- a/fdbclient/libb64/decode.h
+++ b/fdbclient/libb64/decode.h
@@ -9,6 +9,9 @@ For details, see http://sourceforge.net/projects/libb64
 #define BASE64_DECODE_H
 
 #include <iostream>
+#include <sstream>
+
+#define BUFFERSIZE 8192
 
 namespace base64
 {
@@ -60,6 +63,15 @@ namespace base64
 			delete [] code;
 			delete [] plaintext;
 		}
+
+		static std::string from_string(std::string s)
+		{
+			std::stringstream in(s);
+			std::stringstream out;
+			decoder dec;
+			dec.decode(in, out);
+			return out.str();
+		}
 	};
 
 } // namespace base64
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 78ca7e076b..a160c96e8a 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -23,6 +23,7 @@
 #include "fdbserver/workloads/workloads.actor.h"
 #include "fdbserver/workloads/BulkSetup.actor.h"
 #include "fdbclient/ReadYourWrites.h"
+#include "fdbclient/libb64/decode.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
 struct ReportConflictingKeysWorkload : TestWorkload {
@@ -161,9 +162,11 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 						ASSERT(root.size() > 0);
 						// Only use the last entry which contains the read_conflict_ranges corresponding to current
 						// conflicts
-						for (const auto& pair : root.back().get_array()) {
+						for (const auto& pair : root) {
 							json_spirit::mObject kr_obj = pair.get_obj();
-							KeyRange kr = KeyRangeRef(kr_obj["begin"].get_str(), kr_obj["end"].get_str());
+							std::string start_key = base64::decoder::from_string(kr_obj["begin"].get_str());
+							std::string end_key = base64::decoder::from_string(kr_obj["end"].get_str());
+							KeyRange kr = KeyRangeRef(start_key, end_key);
 							if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
 								    // Read_conflict_range remains same in the resolver.
 								    // Thus, the returned keyrange is either the original read_conflict_range or merged
@@ -175,7 +178,10 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 								TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
 							}
 						}
-					}
+					} else {
+                        ++self->invalidReports;
+                        TraceEvent(SevError, "TestFailure").detail("Reason", "FailedToParseJson");
+                    }
 				}
 				++self->retries;
 			}
@@ -185,4 +191,4 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	}
 };
 
-WorkloadFactory<ReportConflictingKeysWorkload> ReportConflictingKeysWorkload("ReportConflictingKeys");
\ No newline at end of file
+WorkloadFactory<ReportConflictingKeysWorkload> ReportConflictingKeysWorkload("ReportConflictingKeys");

From 9dc79488a8ce3616b70e951aeeaada7f9891ff3a Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 4 Dec 2019 11:55:46 -0800
Subject: [PATCH 04/45] narrow the returned conflicting ranges

---
 fdbserver/SkipList.cpp                        | 23 +++++++++++++------
 .../workloads/ReportConflictingKeys.actor.cpp |  8 +++----
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 184f154876..52deca50e0 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -772,10 +772,19 @@ private:
 		}
 
 		bool noConflict() { return true; }
-		bool conflict() { 
+		bool conflict(Node* start_node, Node* end_node) { 
 			*result = true;
 			if(conflictingKeyRange != nullptr){
-				conflictingKeyRange->push_back_deep(conflictingKeyRange->arena(), KeyRangeRef(start.value, end.value));
+				StringRef startKey(start.value);
+				StringRef endKey(end.value);
+				if (start_node != nullptr) {
+					startKey = StringRef(start_node->value(), start_node->length());
+				}
+				if (end_node != nullptr) {
+					endKey = StringRef(end_node->value(), end_node->length());
+				}
+				
+				conflictingKeyRange->push_back_deep(conflictingKeyRange->arena(), KeyRangeRef(startKey, endKey));
 			}
 			return true;
 		}
@@ -800,7 +809,7 @@ private:
 					if (start.finger[l]->getMaxVersion(l) <= version)
 						return noConflict();
 					if (l==0)
-						return conflict();
+						return conflict(nullptr, nullptr); // The whole readConflictRange is conflicting with other transactions with higher version number
 				}
 				state = 1;
 			case 1: 
@@ -809,12 +818,12 @@ private:
 					Node *e = end.finger[end.level];
 					while (e->getMaxVersion(end.level) > version) {
 						if (end.finished()) 
-							return conflict();
+							return conflict(e, nullptr);
 						end.nextLevel();
 						Node *f = end.finger[end.level];
 						while (e != f){
 							if (e->getMaxVersion(end.level) > version) 
-								return conflict();
+								return conflict(e, nullptr);
 							e = e->getNext(end.level);
 						}
 					}
@@ -826,7 +835,7 @@ private:
 						Node *p = nextS;
 						while (p != s){
 							if (p->getMaxVersion(start.level) > version) 
-								return conflict();
+								return conflict(p, s);
 							p = p->getNext(start.level);
 						}
 						if (start.finger[start.level]->getMaxVersion(start.level) <= version) 
@@ -836,7 +845,7 @@ private:
 							if (nextS->length() == start.value.size() && !memcmp(nextS->value(), start.value.begin(), start.value.size()))
 								return noConflict();
 							else
-								return conflict();
+								return conflict(nullptr, nextS);
 						}
 						start.nextLevel();
 					}
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index a160c96e8a..797adf65c9 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -168,11 +168,9 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 							std::string end_key = base64::decoder::from_string(kr_obj["end"].get_str());
 							KeyRange kr = KeyRangeRef(start_key, end_key);
 							if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
-								    // Read_conflict_range remains same in the resolver.
-								    // Thus, the returned keyrange is either the original read_conflict_range or merged
-								    // by several overlapped ones In either case, it contains at least one original
-								    // read_conflict_range
-								    return kr.contains(rCR);
+								    // Returned KeyRange is an intersection of Read_conflict_range and Write_conflict_range saved in the resolver.
+								    // Thus, it intersects with at least one original read_conflict_range
+								    return kr.intersects(rCR);
 							    })) {
 								++self->invalidReports;
 								TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");

From 10719200c391d39ac5e1766b86e0a3989b8f432c Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Tue, 10 Dec 2019 15:37:18 -0800
Subject: [PATCH 05/45] A hack way to call API through
 getRange("\xff\xff/conflicting_keys\<start_key>",
 "\xff\xff/conflicting_keys\<end_key>").

---
 fdbclient/MasterProxyInterface.h              |   5 +-
 fdbclient/NativeAPI.actor.cpp                 |  13 +-
 fdbclient/NativeAPI.actor.h                   |   3 +-
 fdbclient/ReadYourWrites.actor.cpp            |  83 ++++++++++---
 fdbclient/SystemData.cpp                      |   3 +
 fdbclient/SystemData.h                        |   2 +
 fdbclient/libb64/decode.h                     |  12 --
 fdbclient/vexillographer/fdb.options          |   2 +-
 fdbserver/ConflictSet.h                       |   2 +-
 fdbserver/MasterProxyServer.actor.cpp         |   6 +-
 fdbserver/Resolver.actor.cpp                  |   2 +-
 fdbserver/SkipList.cpp                        |  25 ++--
 .../workloads/ReportConflictingKeys.actor.cpp | 116 +++++++++++-------
 tests/CMakeLists.txt                          |   2 +-
 tests/{ => fast}/ReportConflictingKeys.txt    |   7 +-
 15 files changed, 180 insertions(+), 103 deletions(-)
 rename tests/{ => fast}/ReportConflictingKeys.txt (66%)

diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index 1c2143a66d..bd1d15e2f1 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -137,10 +137,7 @@ struct CommitTransactionRequest : TimedRequest {
 	void serialize(Ar& ar) { 
 		serializer(ar, transaction, reply, arena, flags, debugID);
 	}
-
-	void reportConflictingKeys(){
-		transaction.report_conflicting_keys = true;
-	}
+	
 };
 
 static inline int getBytes( CommitTransactionRequest const& r ) {
diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 13c742a16d..05eeb8a11f 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2613,7 +2613,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 				proxy_memory_limit_exceeded(),
 				commit_unknown_result()});
 	}
-	
+
 	try {
 		Version v = wait( readVersion );
 		req.transaction.read_snapshot = v;
@@ -2673,8 +2673,15 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					}
 					return Void();
 				} else {
+					// clear previous conflicting KeyRanges
+					tr->info.conflictingKeyRanges.reset();
 					if (ci.conflictingKeyRanges.present()){
-						tr->info.conflictingKeyRanges = ci.conflictingKeyRanges.get();
+						Standalone<VectorRef<KeyValueRef>> conflictingKeyRanges;
+						for (auto const & kr : ci.conflictingKeyRanges.get()) {
+							conflictingKeyRanges.push_back_deep(conflictingKeyRanges.arena(), KeyValueRef(kr.begin, conflictingKeysTrue));
+							conflictingKeyRanges.push_back_deep(conflictingKeyRanges.arena(), KeyValueRef(kr.end, conflictingKeysFalse));
+						}
+						tr->info.conflictingKeyRanges = conflictingKeyRanges;
 					}
 
 					if (info.debugID.present())
@@ -2789,7 +2796,7 @@ Future<Void> Transaction::commitMutations() {
 			tr.flags = tr.flags | CommitTransactionRequest::FLAG_FIRST_IN_BATCH;
 		}
 		if(options.reportConflictingKeys) {
-			tr.reportConflictingKeys();
+			tr.transaction.report_conflicting_keys = true;
 		}
 
 		Future<Void> commitResult = tryCommit( cx, trLogInfo, tr, readVersion, info, &this->committedVersion, this, options );
diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h
index cc885031e8..a84bfb44d8 100644
--- a/fdbclient/NativeAPI.actor.h
+++ b/fdbclient/NativeAPI.actor.h
@@ -144,7 +144,7 @@ struct TransactionInfo {
 	Optional<UID> debugID;
 	TaskPriority taskID;
 	bool useProvisionalProxies;
-	Optional<Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
+	Optional<Standalone<VectorRef<KeyValueRef>>> conflictingKeyRanges;
 
 	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false) {}
 };
@@ -273,7 +273,6 @@ public:
 	void reset();
 	void fullReset();
 	double getBackoff(int errCode);
-
 	void debugTransaction(UID dID) { info.debugID = dID; }
 
 	Future<Void> commitMutations();
diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 808f29e22d..aaab473285 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -23,7 +23,6 @@
 #include "fdbclient/DatabaseContext.h"
 #include "fdbclient/StatusClient.h"
 #include "fdbclient/MonitorLeader.h"
-#include "fdbclient/libb64/encode.h"
 #include "flow/Util.h"
 #include "flow/actorcompiler.h"  // This must be the last #include.
 
@@ -1190,6 +1189,43 @@ ACTOR Future<Standalone<RangeResultRef>> getWorkerInterfaces (Reference<ClusterC
 	}
 }
 
+ACTOR Future<Standalone<RangeResultRef>> getConflictingKeys(
+	VectorRef<KeyValueRef> conflictingKeys,
+	ReadYourWritesTransaction* ryw,
+	KeySelector begin, 
+	KeySelector end, 
+	GetRangeLimits limits,
+	bool snapshot,
+	bool reverse
+	) {
+		// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters it provides.
+		// It is kind of difficult to take care of each corner cases of what getRange does
+		// Thus, we use a hack way here to achieve it.
+		// We create an empty RYWTransaction and write all conflicting key/values to it.
+		// Since it is RYWTr, we can call getRange on it with same parameters given to the original getRange
+		state ReadYourWritesTransaction hackTr(ryw->getDatabase());
+		// One issue here is that, although the write itself is local. getRange will look through the database.
+		// To make sure there is no keys other than conflictings keys are read,
+		// We write it under system key prefix "\xff/conflicting_keys/", which should always be empty in the database.
+		// Thus, only the local written keys are under this prefix and will be looked through.
+		// One pain point is that it adds latency overhead here
+		state const KeyRef conflictingKeysPrefix = LiteralStringRef("\xff/conflicting_keys/");
+		hackTr.setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
+		for (const KeyValueRef kv : conflictingKeys) {
+			hackTr.set(kv.key.withPrefix(conflictingKeysPrefix), kv.value);
+		}
+		state KeySelector conflictingKeysBegin = KeySelectorRef(begin.getKey().withPrefix(conflictingKeysPrefix), begin.orEqual, begin.offset);
+		state KeySelector conflictingKeysEnd = KeySelectorRef(end.getKey().withPrefix(conflictingKeysPrefix), end.orEqual, end.offset);
+		Standalone<RangeResultRef> resultWithKeyPrefix = wait(hackTr.getRange(conflictingKeysBegin, conflictingKeysEnd, limits, snapshot, reverse));
+		// Remove the prefix we added before and then return the result
+		Standalone<RangeResultRef> result;
+		for (const KeyValueRef kv : resultWithKeyPrefix) {
+			result.push_back_deep(result.arena(), KeyValueRef(kv.key.removePrefix(conflictingKeysPrefix), kv.value));
+		}
+		hackTr.reset();
+		return result;
+	}
+
 Future< Optional<Value> > ReadYourWritesTransaction::get( const Key& key, bool snapshot ) {
 	TEST(true);
 	
@@ -1229,24 +1265,6 @@ Future< Optional<Value> > ReadYourWritesTransaction::get( const Key& key, bool s
 		return Optional<Value>();
 	}
 
-	// Add conflicting keys to special key space
-	if (key == LiteralStringRef("\xff\xff/conflicting_keys/json")){
-		if (tr.info.conflictingKeyRanges.present()){
-			json_spirit::mArray root;
-			json_spirit::mObject keyrange;
-			for (const auto & kr : tr.info.conflictingKeyRanges.get()) {
-				keyrange["begin"] = base64::encoder::from_string(kr.begin.toString());
-				keyrange["end"] = base64::encoder::from_string(kr.end.toString());
-				root.push_back(keyrange);
-				keyrange.clear();
-			}
-			Optional<Value> output = StringRef(json_spirit::write_string(json_spirit::mValue(root), json_spirit::Output_options::raw_utf8));
-			return output;
-		} else {
-			return Optional<Value>();
-		}
-	}
-
 	if(checkUsedDuringCommit()) {
 		return used_during_commit();
 	}
@@ -1298,6 +1316,33 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		}
 	}
 	
+	// Use special key prefix "\xff\xff/conflicting_keys/<some_key>",
+	// to retrieve keys which caused latest not_committed(conflicting with another transaction) error.
+	// The returned key value pairs are interpretted as :
+	// <key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
+	// <key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
+	// Due to the implementation of resolver, currently, 
+	// we can only give key ranges that contain at least one key is conflicting.
+	Key conflictingKeysPreifx = LiteralStringRef("\xff\xff/conflicting_keys/");
+	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {
+		// Remove the special key prefix "\xff\xff/conflicting_keys/"
+		Key beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPreifx);
+		Key endConflictingKey = end.getKey().removePrefix(conflictingKeysPreifx);
+
+		// Check if the conflicting key range to be read is valid
+		KeyRef maxKey = getMaxReadKey();
+		if(beginConflictingKey > maxKey || endConflictingKey > maxKey)
+			return key_outside_legal_range();
+		
+		begin.setKey(beginConflictingKey);
+		end.setKey(endConflictingKey);
+		if (tr.info.conflictingKeyRanges.present()) {
+			return getConflictingKeys(tr.info.conflictingKeyRanges.get(), this, begin, end, limits, snapshot, reverse);
+		} else {
+			return Standalone<RangeResultRef>();
+		}
+	}
+
 	if(checkUsedDuringCommit()) {
 		return used_during_commit();
 	}
diff --git a/fdbclient/SystemData.cpp b/fdbclient/SystemData.cpp
index 99bbed5429..ce0e831629 100644
--- a/fdbclient/SystemData.cpp
+++ b/fdbclient/SystemData.cpp
@@ -58,6 +58,9 @@ void decodeKeyServersValue( const ValueRef& value, vector<UID>& src, vector<UID>
 	}
 }
 
+const ValueRef conflictingKeysTrue = LiteralStringRef("1");
+const ValueRef conflictingKeysFalse = LiteralStringRef("0");
+
 //    "\xff/storageCache/[[begin]]" := "[[vector<uint16_t>]]"
 const KeyRangeRef storageCacheKeys( LiteralStringRef("\xff/storageCache/"), LiteralStringRef("\xff/storageCache0") );
 const KeyRef storageCachePrefix = storageCacheKeys.begin;
diff --git a/fdbclient/SystemData.h b/fdbclient/SystemData.h
index c3debfac3f..6d2609ccd6 100644
--- a/fdbclient/SystemData.h
+++ b/fdbclient/SystemData.h
@@ -64,6 +64,8 @@ const Key serverKeysPrefixFor( UID serverID );
 UID serverKeysDecodeServer( const KeyRef& key );
 bool serverHasKey( ValueRef storedValue );
 
+extern const ValueRef conflictingKeysTrue, conflictingKeysFalse;
+
 extern const KeyRef cacheKeysPrefix;
 
 const Key cacheKeysKey( uint16_t idx, const KeyRef& key );
diff --git a/fdbclient/libb64/decode.h b/fdbclient/libb64/decode.h
index fe71bb7946..368d00f3a0 100755
--- a/fdbclient/libb64/decode.h
+++ b/fdbclient/libb64/decode.h
@@ -9,9 +9,6 @@ For details, see http://sourceforge.net/projects/libb64
 #define BASE64_DECODE_H
 
 #include <iostream>
-#include <sstream>
-
-#define BUFFERSIZE 8192
 
 namespace base64
 {
@@ -63,15 +60,6 @@ namespace base64
 			delete [] code;
 			delete [] plaintext;
 		}
-
-		static std::string from_string(std::string s)
-		{
-			std::stringstream in(s);
-			std::stringstream out;
-			decoder dec;
-			dec.decode(in, out);
-			return out.str();
-		}
 	};
 
 } // namespace base64
diff --git a/fdbclient/vexillographer/fdb.options b/fdbclient/vexillographer/fdb.options
index 089bc4e1c2..e73f6074de 100644
--- a/fdbclient/vexillographer/fdb.options
+++ b/fdbclient/vexillographer/fdb.options
@@ -251,7 +251,7 @@ description is not currently required but encouraged.
     <Option name="use_provisional_proxies" code="711"
             description="This option should only be used by tools which change the database configuration." />
     <Option name="report_conflicting_keys" code="712"
-            description="The transaction will save conflicting keys if conflicts happen." />
+            description="The transaction can retrieve keys that are conflicting with other transactions." />
   </Scope>
 
   <!-- The enumeration values matter - do not change them without
diff --git a/fdbserver/ConflictSet.h b/fdbserver/ConflictSet.h
index 09e1fb5d14..8d6e4b787e 100644
--- a/fdbserver/ConflictSet.h
+++ b/fdbserver/ConflictSet.h
@@ -54,7 +54,7 @@ private:
 	std::vector< std::pair<StringRef,StringRef> > combinedWriteConflictRanges;
 	std::vector< struct ReadConflictRange > combinedReadConflictRanges;
 	bool* transactionConflictStatus;
-	std::map< int, Standalone<VectorRef<KeyRangeRef>> >* conflictingKeyRangeMap;
+	std::map< int, Standalone< VectorRef< KeyRangeRef > > >* conflictingKeyRangeMap;
 
 	void checkIntraBatchConflicts();
 	void combineWriteConflictRanges();
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 4eadeb8b1f..7bd1a26b1d 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1039,11 +1039,11 @@ ACTOR Future<Void> commitBatch(
 				for (int resolverInd : transactionResolverMap[t]) {
 					for (const KeyRangeRef & kr : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
 						unmergedConflictingKRs.emplace_back(kr);
-						// TraceEvent("ConflictingKeyRange").detail("ResolverIndex", resolverInd).detail("TrasactionIndex", t).detail("StartKey", kr.begin.toString()).detail("EndKey", kr.end.toString());
 					}
 				}
+				// At least one keyRange should be returned
 				ASSERT(unmergedConflictingKRs.size());
-				// Sort the keyranges by begin key, then union overlap ranges from left to right
+				// Sort the keyranges by begin key, then union overlapping ranges from left to right
 				std::sort(unmergedConflictingKRs.begin(), unmergedConflictingKRs.end(), [](KeyRange a, KeyRange b){
 					return a.begin < b.begin;
 				});
@@ -1066,7 +1066,7 @@ ACTOR Future<Void> commitBatch(
 			}
 		}
 
-		// Update corresponding transaction index on each resolver
+		// Update corresponding transaction indices on each resolver
 		for (int resolverInd : transactionResolverMap[t])
 			nextTr[resolverInd]++;
 
diff --git a/fdbserver/Resolver.actor.cpp b/fdbserver/Resolver.actor.cpp
index 7f341271a1..c5f5cd220a 100644
--- a/fdbserver/Resolver.actor.cpp
+++ b/fdbserver/Resolver.actor.cpp
@@ -132,7 +132,7 @@ ACTOR Future<Void> resolveBatch(
 		
 		vector<int> commitList;
 		vector<int> tooOldList;
-		std::map<int, Standalone<VectorRef<KeyRangeRef>> > conflictingKeyRangeMap;
+		std::map< int, Standalone< VectorRef< KeyRangeRef > > > conflictingKeyRangeMap;
 
 		// Detect conflicts
 		double expire = now() + SERVER_KNOBS->SAMPLE_EXPIRATION_TIME;
diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 52deca50e0..05d04d695e 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -123,7 +123,7 @@ struct ReadConflictRange {
 	StringRef begin, end;
 	Version version;
 	int transaction;
-	Standalone<VectorRef<KeyRangeRef>> * conflictingKeyRange;
+	Standalone< VectorRef< KeyRangeRef > >* conflictingKeyRange;
 	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, Standalone<VectorRef<KeyRangeRef>> * cKR = nullptr )
 		: begin(begin), end(end), version(version), transaction(transaction), conflictingKeyRange(cKR)
 	{
@@ -760,7 +760,7 @@ private:
 		Version version;
 		bool *result;
 		int state;
-		Standalone<VectorRef<KeyRangeRef>>* conflictingKeyRange;
+		Standalone<VectorRef<KeyRangeRef>>* conflictingKeyRange; // null if report_conflicting_keys is not enabled.
 
 		void init( const ReadConflictRange& r, Node* header, bool* tCS, Standalone<VectorRef<KeyRangeRef>>* cKR) {
 			this->start.init( r.begin, header );
@@ -783,7 +783,6 @@ private:
 				if (end_node != nullptr) {
 					endKey = StringRef(end_node->value(), end_node->length());
 				}
-				
 				conflictingKeyRange->push_back_deep(conflictingKeyRange->arena(), KeyRangeRef(startKey, endKey));
 			}
 			return true;
@@ -809,7 +808,7 @@ private:
 					if (start.finger[l]->getMaxVersion(l) <= version)
 						return noConflict();
 					if (l==0)
-						return conflict(nullptr, nullptr); // The whole readConflictRange is conflicting with other transactions with higher version number
+						return conflict(nullptr, nullptr); // The whole read_conflict_range is conflicting with other transactions with higher version number
 				}
 				state = 1;
 			case 1: 
@@ -1039,10 +1038,12 @@ public:
 		for(int i=begin; i<end; i++)
 			values[i] = true;
 	}
-	bool any( int begin, int end ) {
+	bool any( int begin, int end, int* conflictingIndex = nullptr ) {
 		for(int i=begin; i<end; i++)
-			if (values[i])
+			if (values[i]) {
+				if (conflictingIndex != nullptr) *conflictingIndex = i;
 				return true;
+			}
 		return false;
 	}
 };
@@ -1133,9 +1134,9 @@ public:
 		setBits(orValues, beginWord, lastWord+1, true);
 	}
 
-	bool any(int begin, int end) {
+	bool any(int begin, int end, int* conflictingIndex = nullptr) {
 		bool a = orImpl(begin,end);
-		bool b = debug.any(begin,end);
+		bool b = debug.any(begin,end, conflictingIndex);
 		ASSERT( a == b );
 		return b;
 	}
@@ -1164,8 +1165,14 @@ void ConflictBatch::checkIntraBatchConflicts() {
 		for(int i=0; i<tr.readRanges.size(); i++){
 			int startPointIndex = tr.readRanges[i].first;
 			int endPointIndex = tr.readRanges[i].second;
-			if ( mcs.any(startPointIndex , endPointIndex ) ) {
+			int conflictingIndex;
+			if ( mcs.any( startPointIndex , endPointIndex, tr.reportConflictingKeys ? &conflictingIndex : nullptr ) ) {
 				if (tr.reportConflictingKeys){
+					// The MiniConflictSet is difficult to change, use MiniConflictSet2 to hack here. (Future: use MiniConflictSet)
+					if (points[conflictingIndex].begin)
+						startPointIndex = conflictingIndex;
+					else
+						endPointIndex = conflictingIndex;
 					(*conflictingKeyRangeMap)[t].push_back_deep((*conflictingKeyRangeMap)[t].arena(), KeyRangeRef(points[startPointIndex].key, points[endPointIndex].key));
 				}
 				conflict = true;
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 797adf65c9..9ab592d29f 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -19,11 +19,11 @@
  */
 
 #include "fdbclient/NativeAPI.actor.h"
+#include "fdbclient/ReadYourWrites.h"
+#include "fdbclient/SystemData.h"
 #include "fdbserver/TesterInterface.actor.h"
 #include "fdbserver/workloads/workloads.actor.h"
 #include "fdbserver/workloads/BulkSetup.actor.h"
-#include "fdbclient/ReadYourWrites.h"
-#include "fdbclient/libb64/decode.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
 struct ReportConflictingKeysWorkload : TestWorkload {
@@ -31,8 +31,9 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	double testDuration, transactionsPerSecond, addReadConflictRangeProb, addWriteConflictRangeProb;
 	Key keyPrefix;
 
-	int nodeCount, actorCount, keyBytes, valueBytes, readConflictRangeCount, writeConflictRangeCount;
+	int nodeCountPerPrefix, actorCount, keyBytes, valueBytes, readConflictRangeCount, writeConflictRangeCount;
 	bool reportConflictingKeys, skipCorrectnessCheck;
+	uint64_t keyPrefixBytes, prefixCount;
 
 	PerfIntCounter invalidReports, commits, conflicts, retries, xacts;
 
@@ -42,12 +43,11 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		testDuration = getOption(options, LiteralStringRef("testDuration"), 10.0);
 		transactionsPerSecond = getOption(options, LiteralStringRef("transactionsPerSecond"), 5000.0) / clientCount;
 		actorCount = getOption(options, LiteralStringRef("actorsPerClient"), transactionsPerSecond / 5);
-		nodeCount = getOption(options, LiteralStringRef("nodeCount"), transactionsPerSecond * clientCount);
 		keyPrefix = unprintable(
 		    getOption(options, LiteralStringRef("keyPrefix"), LiteralStringRef("ReportConflictingKeysWorkload"))
 		        .toString());
 		keyBytes = getOption(options, LiteralStringRef("keyBytes"), 16);
-		ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
+		
 		readConflictRangeCount = getOption(options, LiteralStringRef("readConflictRangeCountPerTx"), 1);
 		writeConflictRangeCount = getOption(options, LiteralStringRef("writeConflictRangeCountPerTx"), 1);
 		// modeled by geometric distribution: (1 - prob) / prob = mean
@@ -56,6 +56,16 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		// If true, store key ranges conflicting with other txs
 		reportConflictingKeys = getOption(options, LiteralStringRef("reportConflictingKeys"), false);
 		skipCorrectnessCheck = getOption(options, LiteralStringRef("skipCorrectnessCheck"), false);
+		// used for generating keyPrefix
+		keyPrefixBytes = getOption(options, LiteralStringRef("keyPrefixBytes"), 0);
+		if (keyPrefixBytes) {
+			prefixCount = 255 * std::round(std::exp2(8*(keyPrefixBytes-1)));
+			ASSERT(keyPrefixBytes + 16 <= keyBytes);
+		}
+		else {
+			ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
+		}
+		nodeCountPerPrefix = getOption(options, LiteralStringRef("nodeCountPerPrefix"), 100);
 	}
 
 	std::string description() override { return "ReportConflictingKeysWorkload"; }
@@ -92,34 +102,51 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	double getCheckTimeout() override { return std::numeric_limits<double>::max(); }
 
 	// Copied from tester.actor.cpp, added parameter to determine the key's length
-	Key keyForIndex(int n) {
-		double p = (double)n / nodeCount;
-		int paddingLen = keyBytes - 16 - keyPrefix.size();
+	Key keyForIndex(int prefixIdx, int n) {
+		double p = (double)n / nodeCountPerPrefix;
+		int paddingLen = keyBytes - 16 - keyPrefixBytes;
 		// left padding by zero
-		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p)).withPrefix(keyPrefix);
+		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p)).withPrefix( prefixIdx >= 0 ? keyPrefixForIndex( prefixIdx) : keyPrefix);
+	}
+
+	Key keyPrefixForIndex(uint64_t n) {
+		Key prefix = makeString(keyPrefixBytes);
+		uint8_t * head = mutateString(prefix);
+		memset(head, 0, keyPrefixBytes);
+		int offset = keyPrefixBytes - 1;
+		while (n) {
+			*(head + offset) = static_cast<uint8_t>(n % 256);
+			n /= 256;
+			offset -= 1;
+		}
+		return prefix;
 	}
 
 	void addRandomReadConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>& readConflictRanges) {
-		int startIdx, endIdx;
+		int startIdx, endIdx, startPrefixIdx, endPrefixIdx;
 		Key startKey, endKey;
 		while (deterministicRandom()->random01() < addReadConflictRangeProb) {
-			startIdx = deterministicRandom()->randomInt(0, nodeCount);
-			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
-			startKey = keyForIndex(startIdx);
-			endKey = keyForIndex(endIdx);
+			startPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(0, prefixCount) : -1;
+			endPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(startPrefixIdx, prefixCount) : -1;
+			startIdx = deterministicRandom()->randomInt(0, nodeCountPerPrefix);
+			endIdx = deterministicRandom()->randomInt(startPrefixIdx < endPrefixIdx ? 0 : startIdx, nodeCountPerPrefix);
+			startKey = keyForIndex(startPrefixIdx, startIdx);
+			endKey = keyForIndex(endPrefixIdx, endIdx);
 			tr->addReadConflictRange(KeyRangeRef(startKey, endKey));
 			readConflictRanges.push_back(KeyRangeRef(startKey, endKey));
 		}
 	}
 
 	void addRandomWriteConflictRange(ReadYourWritesTransaction* tr) {
-		int startIdx, endIdx;
+		int startIdx, endIdx, startPrefixIdx, endPrefixIdx;
 		Key startKey, endKey;
 		while (deterministicRandom()->random01() < addWriteConflictRangeProb) {
-			startIdx = deterministicRandom()->randomInt(0, nodeCount);
-			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
-			startKey = keyForIndex(startIdx);
-			endKey = keyForIndex(endIdx);
+			startPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(0, prefixCount) : -1;
+			endPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(startPrefixIdx, prefixCount) : -1;
+			startIdx = deterministicRandom()->randomInt(0, nodeCountPerPrefix);
+			endIdx = deterministicRandom()->randomInt(startPrefixIdx < endPrefixIdx ? 0 : startIdx, nodeCountPerPrefix);
+			startKey = keyForIndex(startPrefixIdx, startIdx);
+			endKey = keyForIndex(endPrefixIdx, endIdx);
 			tr->addWriteConflictRange(KeyRangeRef(startKey, endKey));
 		}
 	}
@@ -153,33 +180,34 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				wait(tr.onError(e));
 				// check API correctness
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
-					Optional<Standalone<StringRef>> temp =
-					    wait(tr.get(LiteralStringRef("\xff\xff/conflicting_keys/json")));
-					auto jsonStr = temp.get().toString();
-					json_spirit::mValue val;
-					if (json_spirit::read_string(jsonStr, val)) {
-						auto root = val.get_array();
-						ASSERT(root.size() > 0);
-						// Only use the last entry which contains the read_conflict_ranges corresponding to current
-						// conflicts
-						for (const auto& pair : root) {
-							json_spirit::mObject kr_obj = pair.get_obj();
-							std::string start_key = base64::decoder::from_string(kr_obj["begin"].get_str());
-							std::string end_key = base64::decoder::from_string(kr_obj["end"].get_str());
-							KeyRange kr = KeyRangeRef(start_key, end_key);
-							if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
-								    // Returned KeyRange is an intersection of Read_conflict_range and Write_conflict_range saved in the resolver.
-								    // Thus, it intersects with at least one original read_conflict_range
-								    return kr.intersects(rCR);
-							    })) {
-								++self->invalidReports;
-								TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
+					state KeyRange ckr = KeyRangeRef(LiteralStringRef("\xff\xff/conflicting_keys/"), LiteralStringRef("\xff\xff/conflicting_keys/\xff"));
+					loop {
+						try {
+							Standalone<RangeResultRef> conflictingKeyRanges = wait(tr.getRange(ckr, readConflictRanges.size() * 2));
+							ASSERT( conflictingKeyRanges.size() && ( conflictingKeyRanges.size() % 2 == 0 ) );
+							for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
+								KeyValueRef startKey = conflictingKeyRanges[i];
+								ASSERT(startKey.value == conflictingKeysTrue);
+								KeyValueRef endKey = conflictingKeyRanges[i+1];
+								ASSERT(endKey.value ==  conflictingKeysFalse);
+								KeyRange kr = KeyRangeRef(startKey.key, endKey.key);
+								if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
+										// Read_conflict_range remains same in the resolver.
+										// Thus, the returned keyrange is either the original read_conflict_range or merged
+										// by several overlapped ones In either case, it contains at least one original
+										// read_conflict_range
+										return kr.intersects(rCR);
+									})) {
+									++self->invalidReports;
+									TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
+								}
 							}
+							break; // leave the loop once we finish readRange
+						} catch (Error& err) {
+							TraceEvent("FailedToGetConflictingKeys").error(err);
+							wait(tr.onError(err));
 						}
-					} else {
-                        ++self->invalidReports;
-                        TraceEvent(SevError, "TestFailure").detail("Reason", "FailedToParseJson");
-                    }
+					}
 				}
 				++self->retries;
 			}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 8c45f4f6e4..942db53a73 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -65,8 +65,8 @@ add_fdb_test(TEST_FILES ReadHalfAbsent.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessUnits.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessBTree.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessPager.txt IGNORE)
-add_fdb_test(TEST_FILES ReportConflictingKeys.txt IGNORE)
 add_fdb_test(TEST_FILES fast/RedwoodCorrectnessBTree.txt IGNORE)
+add_fdb_test(TEST_FILES fast/ReportConflictingKeys.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectness.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodPerfTests.txt IGNORE)
 add_fdb_test(TEST_FILES SampleNoSimAttrition.txt IGNORE)
diff --git a/tests/ReportConflictingKeys.txt b/tests/fast/ReportConflictingKeys.txt
similarity index 66%
rename from tests/ReportConflictingKeys.txt
rename to tests/fast/ReportConflictingKeys.txt
index 8ca1763c44..9ee68a57fd 100644
--- a/tests/ReportConflictingKeys.txt
+++ b/tests/fast/ReportConflictingKeys.txt
@@ -2,10 +2,11 @@ testTitle=ReportConflictingKeysTest
     testName=ReportConflictingKeys
     testDuration=10.0
     transactionsPerSecond=100000
-    nodeCount=10000
-    actorsPerClient=16
+    nodeCountPerPrefix=10000
+    actorsPerClient=256
     keyPrefix=RCK
     keyBytes=64
     readConflictRangeCountPerTx=1
     writeConflictRangeCountPerTx=1
-    reportConflictingKeys=true
\ No newline at end of file
+    reportConflictingKeys=true
+    skipCorrectnessCheck=false
\ No newline at end of file

From 57fb28af2c73370cfe5492844f23189986987121 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Mon, 6 Jan 2020 15:28:57 -0800
Subject: [PATCH 06/45] Explicitly set read-version and clear the whole
 keyspace to make sure the getRange happens locally

---
 fdbclient/ReadYourWrites.actor.cpp            | 61 +++++++------------
 .../workloads/ReportConflictingKeys.actor.cpp | 43 ++++++-------
 2 files changed, 41 insertions(+), 63 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index aaab473285..a78e8e17aa 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1189,43 +1189,6 @@ ACTOR Future<Standalone<RangeResultRef>> getWorkerInterfaces (Reference<ClusterC
 	}
 }
 
-ACTOR Future<Standalone<RangeResultRef>> getConflictingKeys(
-	VectorRef<KeyValueRef> conflictingKeys,
-	ReadYourWritesTransaction* ryw,
-	KeySelector begin, 
-	KeySelector end, 
-	GetRangeLimits limits,
-	bool snapshot,
-	bool reverse
-	) {
-		// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters it provides.
-		// It is kind of difficult to take care of each corner cases of what getRange does
-		// Thus, we use a hack way here to achieve it.
-		// We create an empty RYWTransaction and write all conflicting key/values to it.
-		// Since it is RYWTr, we can call getRange on it with same parameters given to the original getRange
-		state ReadYourWritesTransaction hackTr(ryw->getDatabase());
-		// One issue here is that, although the write itself is local. getRange will look through the database.
-		// To make sure there is no keys other than conflictings keys are read,
-		// We write it under system key prefix "\xff/conflicting_keys/", which should always be empty in the database.
-		// Thus, only the local written keys are under this prefix and will be looked through.
-		// One pain point is that it adds latency overhead here
-		state const KeyRef conflictingKeysPrefix = LiteralStringRef("\xff/conflicting_keys/");
-		hackTr.setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-		for (const KeyValueRef kv : conflictingKeys) {
-			hackTr.set(kv.key.withPrefix(conflictingKeysPrefix), kv.value);
-		}
-		state KeySelector conflictingKeysBegin = KeySelectorRef(begin.getKey().withPrefix(conflictingKeysPrefix), begin.orEqual, begin.offset);
-		state KeySelector conflictingKeysEnd = KeySelectorRef(end.getKey().withPrefix(conflictingKeysPrefix), end.orEqual, end.offset);
-		Standalone<RangeResultRef> resultWithKeyPrefix = wait(hackTr.getRange(conflictingKeysBegin, conflictingKeysEnd, limits, snapshot, reverse));
-		// Remove the prefix we added before and then return the result
-		Standalone<RangeResultRef> result;
-		for (const KeyValueRef kv : resultWithKeyPrefix) {
-			result.push_back_deep(result.arena(), KeyValueRef(kv.key.removePrefix(conflictingKeysPrefix), kv.value));
-		}
-		hackTr.reset();
-		return result;
-	}
-
 Future< Optional<Value> > ReadYourWritesTransaction::get( const Key& key, bool snapshot ) {
 	TEST(true);
 	
@@ -1337,7 +1300,29 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		begin.setKey(beginConflictingKey);
 		end.setKey(endConflictingKey);
 		if (tr.info.conflictingKeyRanges.present()) {
-			return getConflictingKeys(tr.info.conflictingKeyRanges.get(), this, begin, end, limits, snapshot, reverse);
+			// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
+			// It is difficult to take care of all corner cases of what getRange does.
+			// Consequently, we use a hack way here to achieve it.
+			// We create an empty RYWTransaction and write all conflicting key/values to it.
+			// Since it is RYWTr, we can call getRange on it with same parameters given to the original getRange.
+			ReadYourWritesTransaction hackTr(getDatabase());
+			// To make the getRange call local, we need to explicitly set the read version here.
+			// This version number 100 set here does nothing but prevent getting read version from the proxy
+			hackTr.setVersion(100);
+			// Clear the whole key space, thus, RYWTr knows to only read keys locally 
+			hackTr.clear(normalKeys);
+			// in case system keys are conflicting
+			hackTr.setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
+			hackTr.clear(systemKeys);
+			for (const KeyValueRef kv : tr.info.conflictingKeyRanges.get()) {
+				hackTr.set(kv.key, kv.value);
+			}
+			Future<Standalone<RangeResultRef>> resultWithKeyPrefixFuture = hackTr.getRange(begin, end, limits, snapshot, reverse);
+			// Make sure it happens locally
+			ASSERT(resultWithKeyPrefixFuture.isReady());
+			// Clear this RYWTr
+			hackTr.reset();
+			return resultWithKeyPrefixFuture.get();
 		} else {
 			return Standalone<RangeResultRef>();
 		}
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 9ab592d29f..0527bccd61 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -181,31 +181,24 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				// check API correctness
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
 					state KeyRange ckr = KeyRangeRef(LiteralStringRef("\xff\xff/conflicting_keys/"), LiteralStringRef("\xff\xff/conflicting_keys/\xff"));
-					loop {
-						try {
-							Standalone<RangeResultRef> conflictingKeyRanges = wait(tr.getRange(ckr, readConflictRanges.size() * 2));
-							ASSERT( conflictingKeyRanges.size() && ( conflictingKeyRanges.size() % 2 == 0 ) );
-							for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
-								KeyValueRef startKey = conflictingKeyRanges[i];
-								ASSERT(startKey.value == conflictingKeysTrue);
-								KeyValueRef endKey = conflictingKeyRanges[i+1];
-								ASSERT(endKey.value ==  conflictingKeysFalse);
-								KeyRange kr = KeyRangeRef(startKey.key, endKey.key);
-								if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
-										// Read_conflict_range remains same in the resolver.
-										// Thus, the returned keyrange is either the original read_conflict_range or merged
-										// by several overlapped ones In either case, it contains at least one original
-										// read_conflict_range
-										return kr.intersects(rCR);
-									})) {
-									++self->invalidReports;
-									TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
-								}
-							}
-							break; // leave the loop once we finish readRange
-						} catch (Error& err) {
-							TraceEvent("FailedToGetConflictingKeys").error(err);
-							wait(tr.onError(err));
+					// The getRange here using the special key prefix "\xff\xff/conflicting_keys/" happens locally, so error handling is not needed here
+					Standalone<RangeResultRef> conflictingKeyRanges = wait(tr.getRange(ckr, readConflictRanges.size() * 2));
+					ASSERT( conflictingKeyRanges.size() && ( conflictingKeyRanges.size() % 2 == 0 ) );
+					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
+						KeyValueRef startKey = conflictingKeyRanges[i];
+						ASSERT(startKey.value == conflictingKeysTrue);
+						KeyValueRef endKey = conflictingKeyRanges[i+1];
+						ASSERT(endKey.value ==  conflictingKeysFalse);
+						KeyRange kr = KeyRangeRef(startKey.key, endKey.key);
+						if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
+								// Read_conflict_range remains same in the resolver.
+								// Thus, the returned keyrange is either the original read_conflict_range or merged
+								// by several overlapped ones In either case, it contains at least one original
+								// read_conflict_range
+								return kr.intersects(rCR);
+							})) {
+							++self->invalidReports;
+							TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
 						}
 					}
 				}

From b8ffc72ccaf0d3ace3a3e6993a818b6f1adf402d Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Tue, 7 Jan 2020 13:54:10 -0800
Subject: [PATCH 07/45] Use a RYW in transactionInfo to track conflicting keys

---
 fdbclient/NativeAPI.actor.cpp      | 25 +++++++++++++++++++------
 fdbclient/NativeAPI.actor.h        |  5 +++--
 fdbclient/ReadYourWrites.actor.cpp | 23 ++---------------------
 3 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 05eeb8a11f..375b876808 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -33,6 +33,7 @@
 #include "fdbclient/MasterProxyInterface.h"
 #include "fdbclient/MonitorLeader.h"
 #include "fdbclient/MutationList.h"
+#include "fdbclient/ReadYourWrites.h"
 #include "fdbclient/StorageServerInterface.h"
 #include "fdbclient/SystemData.h"
 #include "fdbrpc/LoadBalance.h"
@@ -2673,15 +2674,27 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					}
 					return Void();
 				} else {
-					// clear previous conflicting KeyRanges
-					tr->info.conflictingKeyRanges.reset();
+					// clear the RYW transaction which contains previous conflicting keys
+					tr->info.conflictingKeysRYW.reset();
 					if (ci.conflictingKeyRanges.present()){
-						Standalone<VectorRef<KeyValueRef>> conflictingKeyRanges;
+						// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
+						// It is difficult to take care of all corner cases of what getRange does.
+						// Consequently, we use a hack way here to achieve it.
+						// We create an empty RYWTransaction and write all conflicting key/values to it.
+						// Since it is RYWTr, we can call getRange on it with same parameters given to the original getRange.
+						tr->info.conflictingKeysRYW = std::make_shared<ReadYourWritesTransaction>(tr->getDatabase());
+						// To make the getRange call local, we need to explicitly set the read version here.
+						// This version number 100 set here does nothing but prevent getting read version from the proxy
+						tr->info.conflictingKeysRYW.get()->setVersion(100);
+						// Clear the whole key space, thus, RYWTr knows to only read keys locally
+						tr->info.conflictingKeysRYW.get()->clear(normalKeys);
+						// in case system keys are conflicting
+						tr->info.conflictingKeysRYW.get()->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
+						tr->info.conflictingKeysRYW.get()->clear(systemKeys);
 						for (auto const & kr : ci.conflictingKeyRanges.get()) {
-							conflictingKeyRanges.push_back_deep(conflictingKeyRanges.arena(), KeyValueRef(kr.begin, conflictingKeysTrue));
-							conflictingKeyRanges.push_back_deep(conflictingKeyRanges.arena(), KeyValueRef(kr.end, conflictingKeysFalse));
+							tr->info.conflictingKeysRYW.get()->set(kr.begin, conflictingKeysTrue);
+							tr->info.conflictingKeysRYW.get()->set(kr.end, conflictingKeysFalse);
 						}
-						tr->info.conflictingKeyRanges = conflictingKeyRanges;
 					}
 
 					if (info.debugID.present())
diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h
index a84bfb44d8..fb0d8e851d 100644
--- a/fdbclient/NativeAPI.actor.h
+++ b/fdbclient/NativeAPI.actor.h
@@ -140,13 +140,14 @@ struct TransactionOptions {
 	void reset(Database const& cx);
 };
 
+class ReadYourWritesTransaction; // workaround cyclic dependency
 struct TransactionInfo {
 	Optional<UID> debugID;
 	TaskPriority taskID;
 	bool useProvisionalProxies;
-	Optional<Standalone<VectorRef<KeyValueRef>>> conflictingKeyRanges;
+	std::shared_ptr<ReadYourWritesTransaction> conflictingKeysRYW; // Used to save conflicting keys if FDBTransactionOptions::REPORT_CONFLICTING_KEYS is enabled
 
-	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false) {}
+	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false), conflictingKeysRYW(nullptr) {}
 };
 
 struct TransactionLogInfo : public ReferenceCounted<TransactionLogInfo>, NonCopyable {
diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index a78e8e17aa..4359b602a1 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1299,29 +1299,10 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		
 		begin.setKey(beginConflictingKey);
 		end.setKey(endConflictingKey);
-		if (tr.info.conflictingKeyRanges.present()) {
-			// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
-			// It is difficult to take care of all corner cases of what getRange does.
-			// Consequently, we use a hack way here to achieve it.
-			// We create an empty RYWTransaction and write all conflicting key/values to it.
-			// Since it is RYWTr, we can call getRange on it with same parameters given to the original getRange.
-			ReadYourWritesTransaction hackTr(getDatabase());
-			// To make the getRange call local, we need to explicitly set the read version here.
-			// This version number 100 set here does nothing but prevent getting read version from the proxy
-			hackTr.setVersion(100);
-			// Clear the whole key space, thus, RYWTr knows to only read keys locally 
-			hackTr.clear(normalKeys);
-			// in case system keys are conflicting
-			hackTr.setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-			hackTr.clear(systemKeys);
-			for (const KeyValueRef kv : tr.info.conflictingKeyRanges.get()) {
-				hackTr.set(kv.key, kv.value);
-			}
-			Future<Standalone<RangeResultRef>> resultWithKeyPrefixFuture = hackTr.getRange(begin, end, limits, snapshot, reverse);
+		if (tr.info.conflictingKeysRYW) {
+			Future<Standalone<RangeResultRef>> resultWithKeyPrefixFuture = tr.info.conflictingKeysRYW.get()->getRange(begin, end, limits, snapshot, reverse);
 			// Make sure it happens locally
 			ASSERT(resultWithKeyPrefixFuture.isReady());
-			// Clear this RYWTr
-			hackTr.reset();
 			return resultWithKeyPrefixFuture.get();
 		} else {
 			return Standalone<RangeResultRef>();

From 941ee6d9f9b9150abf30e0e32701df31d8a7ab5e Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Tue, 7 Jan 2020 15:39:50 -0800
Subject: [PATCH 08/45] update comments

---
 fdbclient/NativeAPI.actor.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h
index fb0d8e851d..68105af862 100644
--- a/fdbclient/NativeAPI.actor.h
+++ b/fdbclient/NativeAPI.actor.h
@@ -145,7 +145,9 @@ struct TransactionInfo {
 	Optional<UID> debugID;
 	TaskPriority taskID;
 	bool useProvisionalProxies;
-	std::shared_ptr<ReadYourWritesTransaction> conflictingKeysRYW; // Used to save conflicting keys if FDBTransactionOptions::REPORT_CONFLICTING_KEYS is enabled
+	// Used to save conflicting keys if FDBTransactionOptions::REPORT_CONFLICTING_KEYS is enabled
+	// shared_ptr used here since TransactionInfo is sometimes copied as function parameters.
+	std::shared_ptr<ReadYourWritesTransaction> conflictingKeysRYW;
 
 	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false), conflictingKeysRYW(nullptr) {}
 };

From 1cdb22c4a471fba9b8723ccb2e4f01f250d3cabb Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 9 Jan 2020 15:42:21 -0800
Subject: [PATCH 09/45] refine code according to comments

---
 fdbclient/NativeAPI.actor.cpp      | 12 ++++++------
 fdbclient/NativeAPI.actor.h        |  2 +-
 fdbclient/ReadYourWrites.actor.cpp |  3 ++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 375b876808..28a7e85289 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2685,15 +2685,15 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						tr->info.conflictingKeysRYW = std::make_shared<ReadYourWritesTransaction>(tr->getDatabase());
 						// To make the getRange call local, we need to explicitly set the read version here.
 						// This version number 100 set here does nothing but prevent getting read version from the proxy
-						tr->info.conflictingKeysRYW.get()->setVersion(100);
+						tr->info.conflictingKeysRYW->setVersion(100);
 						// Clear the whole key space, thus, RYWTr knows to only read keys locally
-						tr->info.conflictingKeysRYW.get()->clear(normalKeys);
+						tr->info.conflictingKeysRYW->clear(normalKeys);
 						// in case system keys are conflicting
-						tr->info.conflictingKeysRYW.get()->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-						tr->info.conflictingKeysRYW.get()->clear(systemKeys);
+						tr->info.conflictingKeysRYW->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
+						tr->info.conflictingKeysRYW->clear(systemKeys);
 						for (auto const & kr : ci.conflictingKeyRanges.get()) {
-							tr->info.conflictingKeysRYW.get()->set(kr.begin, conflictingKeysTrue);
-							tr->info.conflictingKeysRYW.get()->set(kr.end, conflictingKeysFalse);
+							tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
+							tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
 						}
 					}
 
diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h
index 68105af862..0cc4888f4f 100644
--- a/fdbclient/NativeAPI.actor.h
+++ b/fdbclient/NativeAPI.actor.h
@@ -149,7 +149,7 @@ struct TransactionInfo {
 	// shared_ptr used here since TransactionInfo is sometimes copied as function parameters.
 	std::shared_ptr<ReadYourWritesTransaction> conflictingKeysRYW;
 
-	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false), conflictingKeysRYW(nullptr) {}
+	explicit TransactionInfo( TaskPriority taskID ) : taskID(taskID), useProvisionalProxies(false) {}
 };
 
 struct TransactionLogInfo : public ReferenceCounted<TransactionLogInfo>, NonCopyable {
diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 4359b602a1..261f00a5a4 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1300,7 +1300,8 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		begin.setKey(beginConflictingKey);
 		end.setKey(endConflictingKey);
 		if (tr.info.conflictingKeysRYW) {
-			Future<Standalone<RangeResultRef>> resultWithKeyPrefixFuture = tr.info.conflictingKeysRYW.get()->getRange(begin, end, limits, snapshot, reverse);
+			Future<Standalone<RangeResultRef>> resultWithKeyPrefixFuture = 
+				tr.info.conflictingKeysRYW->getRange(begin, end, limits, snapshot, reverse);
 			// Make sure it happens locally
 			ASSERT(resultWithKeyPrefixFuture.isReady());
 			return resultWithKeyPrefixFuture.get();

From 1f633d3e5f3328145e01fb1416514a3ea01e4a7e Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 10 Jan 2020 15:53:06 -0800
Subject: [PATCH 10/45] change local var from Key to KeyRef

---
 fdbclient/ReadYourWrites.actor.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 261f00a5a4..67bb1028d6 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1286,11 +1286,11 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// <key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
 	// Due to the implementation of resolver, currently, 
 	// we can only give key ranges that contain at least one key is conflicting.
-	Key conflictingKeysPreifx = LiteralStringRef("\xff\xff/conflicting_keys/");
+	KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/conflicting_keys/");
 	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {
 		// Remove the special key prefix "\xff\xff/conflicting_keys/"
-		Key beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPreifx);
-		Key endConflictingKey = end.getKey().removePrefix(conflictingKeysPreifx);
+		KeyRef beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPreifx);
+		KeyRef endConflictingKey = end.getKey().removePrefix(conflictingKeysPreifx);
 
 		// Check if the conflicting key range to be read is valid
 		KeyRef maxKey = getMaxReadKey();

From 9a736dde8d026cd1aa76ca430816cb398bcb3b1a Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 10 Jan 2020 18:01:17 -0800
Subject: [PATCH 11/45] delete wait in workload when checking, add assert to
 make sure the getRange for conflicting keys happens locally

---
 fdbserver/workloads/ReportConflictingKeys.actor.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 0527bccd61..b548c7c657 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -182,7 +182,9 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
 					state KeyRange ckr = KeyRangeRef(LiteralStringRef("\xff\xff/conflicting_keys/"), LiteralStringRef("\xff\xff/conflicting_keys/\xff"));
 					// The getRange here using the special key prefix "\xff\xff/conflicting_keys/" happens locally, so error handling is not needed here
-					Standalone<RangeResultRef> conflictingKeyRanges = wait(tr.getRange(ckr, readConflictRanges.size() * 2));
+					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture = tr.getRange(ckr, readConflictRanges.size() * 2);
+					ASSERT(conflictingKeyRangesFuture.isReady());
+					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
 					ASSERT( conflictingKeyRanges.size() && ( conflictingKeyRanges.size() % 2 == 0 ) );
 					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
 						KeyValueRef startKey = conflictingKeyRanges[i];

From 1c2b116688b46aed8d00479db4a4bb0e12d41aee Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Mon, 13 Jan 2020 18:08:54 -0800
Subject: [PATCH 12/45] Push sorting and union of overlapped keyranges to
 clients

---
 fdbclient/NativeAPI.actor.cpp         | 25 ++++++++++++++++++++++-
 fdbserver/MasterProxyServer.actor.cpp | 29 ++++++++-------------------
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 28a7e85289..9ce4f3d308 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2677,6 +2677,29 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					// clear the RYW transaction which contains previous conflicting keys
 					tr->info.conflictingKeysRYW.reset();
 					if (ci.conflictingKeyRanges.present()){
+						// Since the performance of the proxy is important, we push the union of overlapped key range to client
+						// Union all overlapped key ranges here
+						std::vector<KeyRangeRef> unmergedConflictingKRs(ci.conflictingKeyRanges.get().begin(), ci.conflictingKeyRanges.get().end());
+						Standalone<VectorRef<KeyRangeRef>> mergedKeyRanges;
+						// At least one keyRange returned
+						ASSERT(unmergedConflictingKRs.size());
+						// Sort the keyranges by begin key, then union overlapping ranges from left to right
+						std::sort(unmergedConflictingKRs.begin(), unmergedConflictingKRs.end(), [](KeyRangeRef a, KeyRangeRef b){
+								return a.begin < b.begin;
+						});
+						auto next = unmergedConflictingKRs.begin();
+						// At least one conflicting keyrange should be returned, which means curr is valid
+						KeyRangeRef curr = *(next++);
+						while (next != unmergedConflictingKRs.end()) {
+								if (curr.end >= next->begin) {
+										curr = KeyRangeRef(curr.begin, std::max(curr.end, next->end));
+								} else {
+										mergedKeyRanges.push_back(mergedKeyRanges.arena(), curr);
+										curr = *next;
+								}
+								next++;
+						}
+						mergedKeyRanges.push_back(mergedKeyRanges.arena(), curr);
 						// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
 						// It is difficult to take care of all corner cases of what getRange does.
 						// Consequently, we use a hack way here to achieve it.
@@ -2691,7 +2714,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						// in case system keys are conflicting
 						tr->info.conflictingKeysRYW->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 						tr->info.conflictingKeysRYW->clear(systemKeys);
-						for (auto const & kr : ci.conflictingKeyRanges.get()) {
+						for (const KeyRangeRef & kr : mergedKeyRanges) {
 							tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
 							tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
 						}
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 7bd1a26b1d..9497582d2e 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1035,32 +1035,19 @@ ACTOR Future<Void> commitBatch(
 			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
 			if (trs[t].transaction.report_conflicting_keys) {
 				Standalone<VectorRef<KeyRangeRef>> conflictingEntries;
-				std::vector<KeyRange> unmergedConflictingKRs;
+				int conflictingKeyRangeNum = 0;
+				for (int resolverInd : transactionResolverMap[t]) {
+					conflictingKeyRangeNum += resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]].size();
+				}
+				conflictingEntries.reserve(conflictingEntries.arena(), conflictingKeyRangeNum);
 				for (int resolverInd : transactionResolverMap[t]) {
 					for (const KeyRangeRef & kr : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
-						unmergedConflictingKRs.emplace_back(kr);
+						conflictingEntries.push_back(conflictingEntries.arena(), kr);
 					}
 				}
 				// At least one keyRange should be returned
-				ASSERT(unmergedConflictingKRs.size());
-				// Sort the keyranges by begin key, then union overlapping ranges from left to right
-				std::sort(unmergedConflictingKRs.begin(), unmergedConflictingKRs.end(), [](KeyRange a, KeyRange b){
-					return a.begin < b.begin;
-				});
-				auto next = unmergedConflictingKRs.begin();
-				// At least one conflicting keyrange should be returned, which means curr is valid
-				KeyRangeRef curr = *(next++);
-				while (next != unmergedConflictingKRs.end()) {
-					if (curr.end >= next->begin) {
-						curr = KeyRangeRef(curr.begin, std::max(curr.end, next->end));
-					} else {
-						conflictingEntries.push_back_deep(conflictingEntries.arena(), curr);
-						curr = *next;
-					}
-					next++;
-				}
-				conflictingEntries.push_back_deep(conflictingEntries.arena(), curr);
-				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<KeyRangeRef>>>(conflictingEntries)));
+				ASSERT(conflictingEntries.size());
+				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<KeyRangeRef>>>(conflictingEntries.contents())));
 			} else {
 				trs[t].reply.sendError(not_committed());
 			}

From 1cf3b5b807d5d7d0e19e18f6ec5e766a139953d3 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Mon, 13 Jan 2020 20:54:16 -0800
Subject: [PATCH 13/45] Change to general prefix
 '\xff\xff/transaction/conflicting_keys/'

---
 fdbclient/ReadYourWrites.actor.cpp                  | 7 ++++---
 fdbserver/workloads/ReportConflictingKeys.actor.cpp | 5 +++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 67bb1028d6..522839623d 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1279,16 +1279,17 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		}
 	}
 	
-	// Use special key prefix "\xff\xff/conflicting_keys/<some_key>",
+	// Use special key prefix "\xff\xff/transaction/conflicting_keys/<some_key>",
 	// to retrieve keys which caused latest not_committed(conflicting with another transaction) error.
 	// The returned key value pairs are interpretted as :
 	// <key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
 	// <key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
 	// Due to the implementation of resolver, currently, 
 	// we can only give key ranges that contain at least one key is conflicting.
-	KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/conflicting_keys/");
+	KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
+	// TODO : This condition need to be changed in the future when we have more special keys under "\xff\xff/transaction/"
 	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {
-		// Remove the special key prefix "\xff\xff/conflicting_keys/"
+		// Remove the special key prefix "\xff\xff/transaction/conflicting_keys/"
 		KeyRef beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPreifx);
 		KeyRef endConflictingKey = end.getKey().removePrefix(conflictingKeysPreifx);
 
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index b548c7c657..cf9f572667 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -180,8 +180,9 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				wait(tr.onError(e));
 				// check API correctness
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
-					state KeyRange ckr = KeyRangeRef(LiteralStringRef("\xff\xff/conflicting_keys/"), LiteralStringRef("\xff\xff/conflicting_keys/\xff"));
-					// The getRange here using the special key prefix "\xff\xff/conflicting_keys/" happens locally, so error handling is not needed here
+					state KeyRange ckr = KeyRangeRef(LiteralStringRef("\xff\xff/transaction/conflicting_keys/"), LiteralStringRef("\xff\xff/transaction/conflicting_keys/\xff"));
+					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens locally
+					// Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture = tr.getRange(ckr, readConflictRanges.size() * 2);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();

From 72d39a31f1dc16a99318101653f09b92ec055d12 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 15 Jan 2020 10:21:51 -0800
Subject: [PATCH 14/45] Change to send back index of read_conflict_range not
 keys, which speeds up but fail to give narrowed keyrange

---
 fdbclient/MasterProxyInterface.h              |  4 +-
 fdbclient/NativeAPI.actor.cpp                 | 28 ++------
 fdbserver/ConflictSet.h                       |  4 +-
 fdbserver/MasterProxyServer.actor.cpp         | 15 ++---
 fdbserver/Resolver.actor.cpp                  |  2 +-
 fdbserver/ResolverInterface.h                 |  2 +-
 fdbserver/SkipList.cpp                        | 65 +++++++------------
 .../workloads/ReportConflictingKeys.actor.cpp |  4 +-
 8 files changed, 42 insertions(+), 82 deletions(-)

diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index bd1d15e2f1..81fb717b87 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -104,7 +104,7 @@ struct CommitID {
 	Version version; 			// returns invalidVersion if transaction conflicts
 	uint16_t txnBatchId;
 	Optional<Value> metadataVersion;
-	Optional<Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRanges;
+	Optional<Standalone<VectorRef<int>>> conflictingKeyRanges;
 
 	template <class Ar>
 	void serialize(Ar& ar) {
@@ -112,7 +112,7 @@ struct CommitID {
 	}
 
 	CommitID() : version(invalidVersion), txnBatchId(0) {}
-	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<Standalone<VectorRef<KeyRangeRef>>>& conflictingKeyRanges = Optional<Standalone<VectorRef<KeyRangeRef>>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKeyRanges(conflictingKeyRanges) {}
+	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<Standalone<VectorRef<int>>>& conflictingKeyRanges = Optional<Standalone<VectorRef<int>>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKeyRanges(conflictingKeyRanges) {}
 };
 
 struct CommitTransactionRequest : TimedRequest {
diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 9ce4f3d308..c95ad1bd3a 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2677,29 +2677,6 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					// clear the RYW transaction which contains previous conflicting keys
 					tr->info.conflictingKeysRYW.reset();
 					if (ci.conflictingKeyRanges.present()){
-						// Since the performance of the proxy is important, we push the union of overlapped key range to client
-						// Union all overlapped key ranges here
-						std::vector<KeyRangeRef> unmergedConflictingKRs(ci.conflictingKeyRanges.get().begin(), ci.conflictingKeyRanges.get().end());
-						Standalone<VectorRef<KeyRangeRef>> mergedKeyRanges;
-						// At least one keyRange returned
-						ASSERT(unmergedConflictingKRs.size());
-						// Sort the keyranges by begin key, then union overlapping ranges from left to right
-						std::sort(unmergedConflictingKRs.begin(), unmergedConflictingKRs.end(), [](KeyRangeRef a, KeyRangeRef b){
-								return a.begin < b.begin;
-						});
-						auto next = unmergedConflictingKRs.begin();
-						// At least one conflicting keyrange should be returned, which means curr is valid
-						KeyRangeRef curr = *(next++);
-						while (next != unmergedConflictingKRs.end()) {
-								if (curr.end >= next->begin) {
-										curr = KeyRangeRef(curr.begin, std::max(curr.end, next->end));
-								} else {
-										mergedKeyRanges.push_back(mergedKeyRanges.arena(), curr);
-										curr = *next;
-								}
-								next++;
-						}
-						mergedKeyRanges.push_back(mergedKeyRanges.arena(), curr);
 						// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
 						// It is difficult to take care of all corner cases of what getRange does.
 						// Consequently, we use a hack way here to achieve it.
@@ -2714,7 +2691,10 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						// in case system keys are conflicting
 						tr->info.conflictingKeysRYW->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 						tr->info.conflictingKeysRYW->clear(systemKeys);
-						for (const KeyRangeRef & kr : mergedKeyRanges) {
+						const auto cKRs = ci.conflictingKeyRanges.get();
+						std::set<int> mergedIds(cKRs.begin(), cKRs.end());
+						for (auto const & rCRIndex : mergedIds) {
+							const KeyRangeRef & kr = req.transaction.read_conflict_ranges[rCRIndex];
 							tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
 							tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
 						}
diff --git a/fdbserver/ConflictSet.h b/fdbserver/ConflictSet.h
index 8d6e4b787e..ffe80e0e6d 100644
--- a/fdbserver/ConflictSet.h
+++ b/fdbserver/ConflictSet.h
@@ -33,7 +33,7 @@ void clearConflictSet( ConflictSet*, Version );
 void destroyConflictSet(ConflictSet*);
 
 struct ConflictBatch {
-	explicit ConflictBatch( ConflictSet*,  std::map< int, Standalone<VectorRef<KeyRangeRef>>>* conflictingKeyRangeMap = nullptr);
+	explicit ConflictBatch( ConflictSet*,  std::map< int, std::vector< int > >* conflictingKeyRangeMap = nullptr);
 	~ConflictBatch();
 
 	enum TransactionCommitResult {
@@ -54,7 +54,7 @@ private:
 	std::vector< std::pair<StringRef,StringRef> > combinedWriteConflictRanges;
 	std::vector< struct ReadConflictRange > combinedReadConflictRanges;
 	bool* transactionConflictStatus;
-	std::map< int, Standalone< VectorRef< KeyRangeRef > > >* conflictingKeyRangeMap;
+	std::map< int, std::vector< int > >* conflictingKeyRangeMap;
 
 	void checkIntraBatchConflicts();
 	void combineWriteConflictRanges();
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 9497582d2e..adb1e36cf6 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1034,20 +1034,15 @@ ACTOR Future<Void> commitBatch(
 		else {
 			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
 			if (trs[t].transaction.report_conflicting_keys) {
-				Standalone<VectorRef<KeyRangeRef>> conflictingEntries;
-				int conflictingKeyRangeNum = 0;
+				Standalone<VectorRef<int>> conflictingEntryIds;
 				for (int resolverInd : transactionResolverMap[t]) {
-					conflictingKeyRangeNum += resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]].size();
-				}
-				conflictingEntries.reserve(conflictingEntries.arena(), conflictingKeyRangeNum);
-				for (int resolverInd : transactionResolverMap[t]) {
-					for (const KeyRangeRef & kr : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
-						conflictingEntries.push_back(conflictingEntries.arena(), kr);
+					for (auto const & rCRIndex : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
+						conflictingEntryIds.push_back(conflictingEntryIds.arena(), rCRIndex);
 					}
 				}
 				// At least one keyRange should be returned
-				ASSERT(conflictingEntries.size());
-				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<KeyRangeRef>>>(conflictingEntries.contents())));
+				ASSERT(conflictingEntryIds.size());
+				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<int>>>(conflictingEntryIds)));
 			} else {
 				trs[t].reply.sendError(not_committed());
 			}
diff --git a/fdbserver/Resolver.actor.cpp b/fdbserver/Resolver.actor.cpp
index c5f5cd220a..7789a216db 100644
--- a/fdbserver/Resolver.actor.cpp
+++ b/fdbserver/Resolver.actor.cpp
@@ -132,7 +132,7 @@ ACTOR Future<Void> resolveBatch(
 		
 		vector<int> commitList;
 		vector<int> tooOldList;
-		std::map< int, Standalone< VectorRef< KeyRangeRef > > > conflictingKeyRangeMap;
+		std::map< int, std::vector< int > > conflictingKeyRangeMap;
 
 		// Detect conflicts
 		double expire = now() + SERVER_KNOBS->SAMPLE_EXPIRATION_TIME;
diff --git a/fdbserver/ResolverInterface.h b/fdbserver/ResolverInterface.h
index 7d467cc143..829888d6ef 100644
--- a/fdbserver/ResolverInterface.h
+++ b/fdbserver/ResolverInterface.h
@@ -77,7 +77,7 @@ struct ResolveTransactionBatchReply {
 	VectorRef<uint8_t> committed;
 	Optional<UID> debugID;
 	VectorRef<VectorRef<StateTransactionRef>> stateMutations;  // [version][transaction#] -> (committed, [mutation#])
-	std::map<int, Standalone<VectorRef<KeyRangeRef>>> conflictingKeyRangeMap; // transaction index -> conflicting keyRanges given by the resolver
+	std::map<int, std::vector<int>> conflictingKeyRangeMap; // transaction index -> conflicting read_conflict_range ids given by the resolver
 
 	template <class Archive>
 	void serialize(Archive& ar) {
diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 05d04d695e..038214cd60 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -123,9 +123,10 @@ struct ReadConflictRange {
 	StringRef begin, end;
 	Version version;
 	int transaction;
-	Standalone< VectorRef< KeyRangeRef > >* conflictingKeyRange;
-	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, Standalone<VectorRef<KeyRangeRef>> * cKR = nullptr )
-		: begin(begin), end(end), version(version), transaction(transaction), conflictingKeyRange(cKR)
+	std::vector<int>* conflictingKeyRange;
+	int indexInTx;
+	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, int indexInTx, std::vector<int> * cKR = nullptr )
+		: begin(begin), end(end), version(version), transaction(transaction), indexInTx(indexInTx), conflictingKeyRange(cKR)
 	{
 	}
 	bool operator<(const ReadConflictRange& rhs) const { return compare(begin, rhs.begin)<0; }
@@ -530,7 +531,7 @@ public:
 
 		int started = min(M,count);
 		for(int i=0; i<started; i++){
-			inProgress[i].init( ranges[i], header, transactionConflictStatus, ranges[i].conflictingKeyRange );
+			inProgress[i].init( ranges[i], header, transactionConflictStatus, ranges[i].indexInTx, ranges[i].conflictingKeyRange );
 			nextJob[i] = i+1;
 		}
 		nextJob[started-1] = 0;
@@ -547,7 +548,7 @@ public:
 				}
 				else {
 					int temp = started++;
-					inProgress[job].init( ranges[temp], header, transactionConflictStatus, ranges[temp].conflictingKeyRange );
+					inProgress[job].init( ranges[temp], header, transactionConflictStatus, ranges[temp].indexInTx, ranges[temp].conflictingKeyRange );
 				}
 			}
 			prevJob = job;
@@ -760,31 +761,24 @@ private:
 		Version version;
 		bool *result;
 		int state;
-		Standalone<VectorRef<KeyRangeRef>>* conflictingKeyRange; // null if report_conflicting_keys is not enabled.
+		int indexInTx;
+		std::vector<int>* conflictingKeyRange; // null if report_conflicting_keys is not enabled.
 
-		void init( const ReadConflictRange& r, Node* header, bool* tCS, Standalone<VectorRef<KeyRangeRef>>* cKR) {
+		void init( const ReadConflictRange& r, Node* header, bool* tCS, int indexInTx, std::vector<int>* cKR) {
 			this->start.init( r.begin, header );
 			this->end.init( r.end, header );
 			this->version = r.version;
+			this->indexInTx = indexInTx;
 			result = &tCS[ r.transaction ];
 			conflictingKeyRange = cKR;
 			this->state = 0;
 		}
 
 		bool noConflict() { return true; }
-		bool conflict(Node* start_node, Node* end_node) { 
+		bool conflict() { 
 			*result = true;
-			if(conflictingKeyRange != nullptr){
-				StringRef startKey(start.value);
-				StringRef endKey(end.value);
-				if (start_node != nullptr) {
-					startKey = StringRef(start_node->value(), start_node->length());
-				}
-				if (end_node != nullptr) {
-					endKey = StringRef(end_node->value(), end_node->length());
-				}
-				conflictingKeyRange->push_back_deep(conflictingKeyRange->arena(), KeyRangeRef(startKey, endKey));
-			}
+			if(conflictingKeyRange != nullptr)
+				conflictingKeyRange->push_back(indexInTx);
 			return true;
 		}
 
@@ -808,7 +802,7 @@ private:
 					if (start.finger[l]->getMaxVersion(l) <= version)
 						return noConflict();
 					if (l==0)
-						return conflict(nullptr, nullptr); // The whole read_conflict_range is conflicting with other transactions with higher version number
+						return conflict(); // The whole read_conflict_range is conflicting with other transactions with higher version number
 				}
 				state = 1;
 			case 1: 
@@ -817,12 +811,12 @@ private:
 					Node *e = end.finger[end.level];
 					while (e->getMaxVersion(end.level) > version) {
 						if (end.finished()) 
-							return conflict(e, nullptr);
+							return conflict();
 						end.nextLevel();
 						Node *f = end.finger[end.level];
 						while (e != f){
 							if (e->getMaxVersion(end.level) > version) 
-								return conflict(e, nullptr);
+								return conflict();
 							e = e->getNext(end.level);
 						}
 					}
@@ -834,7 +828,7 @@ private:
 						Node *p = nextS;
 						while (p != s){
 							if (p->getMaxVersion(start.level) > version) 
-								return conflict(p, s);
+								return conflict();
 							p = p->getNext(start.level);
 						}
 						if (start.finger[start.level]->getMaxVersion(start.level) <= version) 
@@ -844,7 +838,7 @@ private:
 							if (nextS->length() == start.value.size() && !memcmp(nextS->value(), start.value.begin(), start.value.size()))
 								return noConflict();
 							else
-								return conflict(nullptr, nextS);
+								return conflict();
 						}
 						start.nextLevel();
 					}
@@ -980,7 +974,7 @@ void destroyConflictSet(ConflictSet* cs) {
 	delete cs;
 }
 
-ConflictBatch::ConflictBatch( ConflictSet* cs, std::map< int, Standalone<VectorRef<KeyRangeRef>> >* conflictingKeyRangeMap )
+ConflictBatch::ConflictBatch( ConflictSet* cs, std::map< int, std::vector< int > >* conflictingKeyRangeMap )
 	: cs(cs), transactionCount(0), conflictingKeyRangeMap(conflictingKeyRangeMap)
 {
 }
@@ -1016,7 +1010,7 @@ void ConflictBatch::addTransaction( const CommitTransactionRef& tr ) {
 			points.emplace_back(range.begin, false, true, false, t, &info->readRanges[r].first);
 			//points.back().keyEnd = StringRef(buf,range.second);
 			points.emplace_back(range.end, false, false, false, t, &info->readRanges[r].second);
-			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t, tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t] : nullptr);
+			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t, r, tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t] : nullptr);
 		}
 		for(int r=0; r<tr.write_conflict_ranges.size(); r++) {
 			const KeyRangeRef& range = tr.write_conflict_ranges[r];
@@ -1038,10 +1032,9 @@ public:
 		for(int i=begin; i<end; i++)
 			values[i] = true;
 	}
-	bool any( int begin, int end, int* conflictingIndex = nullptr ) {
+	bool any( int begin, int end ) {
 		for(int i=begin; i<end; i++)
 			if (values[i]) {
-				if (conflictingIndex != nullptr) *conflictingIndex = i;
 				return true;
 			}
 		return false;
@@ -1134,9 +1127,9 @@ public:
 		setBits(orValues, beginWord, lastWord+1, true);
 	}
 
-	bool any(int begin, int end, int* conflictingIndex = nullptr) {
+	bool any(int begin, int end) {
 		bool a = orImpl(begin,end);
-		bool b = debug.any(begin,end, conflictingIndex);
+		bool b = debug.any(begin,end);
 		ASSERT( a == b );
 		return b;
 	}
@@ -1163,17 +1156,9 @@ void ConflictBatch::checkIntraBatchConflicts() {
 		if (transactionConflictStatus[t]) continue;
 		bool conflict = tr.tooOld;
 		for(int i=0; i<tr.readRanges.size(); i++){
-			int startPointIndex = tr.readRanges[i].first;
-			int endPointIndex = tr.readRanges[i].second;
-			int conflictingIndex;
-			if ( mcs.any( startPointIndex , endPointIndex, tr.reportConflictingKeys ? &conflictingIndex : nullptr ) ) {
+			if ( mcs.any( tr.readRanges[i].first , tr.readRanges[i].second ) ) {
 				if (tr.reportConflictingKeys){
-					// The MiniConflictSet is difficult to change, use MiniConflictSet2 to hack here. (Future: use MiniConflictSet)
-					if (points[conflictingIndex].begin)
-						startPointIndex = conflictingIndex;
-					else
-						endPointIndex = conflictingIndex;
-					(*conflictingKeyRangeMap)[t].push_back_deep((*conflictingKeyRangeMap)[t].arena(), KeyRangeRef(points[startPointIndex].key, points[endPointIndex].key));
+					(*conflictingKeyRangeMap)[t].push_back(i);
 				}
 				conflict = true;
 				break;
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index cf9f572667..39f9a203ef 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -192,13 +192,13 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 						ASSERT(startKey.value == conflictingKeysTrue);
 						KeyValueRef endKey = conflictingKeyRanges[i+1];
 						ASSERT(endKey.value ==  conflictingKeysFalse);
-						KeyRange kr = KeyRangeRef(startKey.key, endKey.key);
+						KeyRangeRef kr = KeyRangeRef(startKey.key, endKey.key);
 						if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
 								// Read_conflict_range remains same in the resolver.
 								// Thus, the returned keyrange is either the original read_conflict_range or merged
 								// by several overlapped ones In either case, it contains at least one original
 								// read_conflict_range
-								return kr.intersects(rCR);
+								return kr.contains(rCR);
 							})) {
 							++self->invalidReports;
 							TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");

From 0749a5a1ab0a4fd2254eccdb77723dc1e9154565 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 15 Jan 2020 11:06:40 -0800
Subject: [PATCH 15/45] update comment

---
 fdbserver/MasterProxyServer.actor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index adb1e36cf6..22e5ee7913 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1040,7 +1040,7 @@ ACTOR Future<Void> commitBatch(
 						conflictingEntryIds.push_back(conflictingEntryIds.arena(), rCRIndex);
 					}
 				}
-				// At least one keyRange should be returned
+				// At least one keyRange index should be returned
 				ASSERT(conflictingEntryIds.size());
 				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<int>>>(conflictingEntryIds)));
 			} else {

From 30b9858cf55757546b0e1b5a0d5012d7a10c9751 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 16 Jan 2020 13:06:36 -0800
Subject: [PATCH 16/45] using std::vector instead of VectorRef to avoid unknown
 arena change behavior

---
 fdbclient/MasterProxyInterface.h      | 4 ++--
 fdbclient/NativeAPI.actor.cpp         | 1 +
 fdbserver/MasterProxyServer.actor.cpp | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index 81fb717b87..ae9a1db049 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -104,7 +104,7 @@ struct CommitID {
 	Version version; 			// returns invalidVersion if transaction conflicts
 	uint16_t txnBatchId;
 	Optional<Value> metadataVersion;
-	Optional<Standalone<VectorRef<int>>> conflictingKeyRanges;
+	Optional<std::vector<int>> conflictingKeyRanges;
 
 	template <class Ar>
 	void serialize(Ar& ar) {
@@ -112,7 +112,7 @@ struct CommitID {
 	}
 
 	CommitID() : version(invalidVersion), txnBatchId(0) {}
-	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<Standalone<VectorRef<int>>>& conflictingKeyRanges = Optional<Standalone<VectorRef<int>>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKeyRanges(conflictingKeyRanges) {}
+	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<std::vector<int>>& conflictingKeyRanges = Optional<std::vector<int>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKeyRanges(conflictingKeyRanges) {}
 };
 
 struct CommitTransactionRequest : TimedRequest {
diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index c95ad1bd3a..d02ff9cd48 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2691,6 +2691,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						// in case system keys are conflicting
 						tr->info.conflictingKeysRYW->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 						tr->info.conflictingKeysRYW->clear(systemKeys);
+						// merge duplicate indices
 						const auto cKRs = ci.conflictingKeyRanges.get();
 						std::set<int> mergedIds(cKRs.begin(), cKRs.end());
 						for (auto const & rCRIndex : mergedIds) {
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 22e5ee7913..3a4fe6a20b 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1034,15 +1034,15 @@ ACTOR Future<Void> commitBatch(
 		else {
 			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
 			if (trs[t].transaction.report_conflicting_keys) {
-				Standalone<VectorRef<int>> conflictingEntryIds;
+				std::vector<int> conflictingEntryIds;
 				for (int resolverInd : transactionResolverMap[t]) {
 					for (auto const & rCRIndex : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
-						conflictingEntryIds.push_back(conflictingEntryIds.arena(), rCRIndex);
+						conflictingEntryIds.emplace_back(rCRIndex);
 					}
 				}
 				// At least one keyRange index should be returned
 				ASSERT(conflictingEntryIds.size());
-				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<int>>>(conflictingEntryIds)));
+				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<std::vector<int>>(conflictingEntryIds)));
 			} else {
 				trs[t].reply.sendError(not_committed());
 			}

From 1fa934837da550d357eca9b161bb64509c98755e Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 16 Jan 2020 13:14:13 -0800
Subject: [PATCH 17/45] Update some var names

---
 fdbclient/MasterProxyInterface.h      | 6 +++---
 fdbserver/MasterProxyServer.actor.cpp | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index ae9a1db049..784331384b 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -104,15 +104,15 @@ struct CommitID {
 	Version version; 			// returns invalidVersion if transaction conflicts
 	uint16_t txnBatchId;
 	Optional<Value> metadataVersion;
-	Optional<std::vector<int>> conflictingKeyRanges;
+	Optional<std::vector<int>> conflictingKRIndices;
 
 	template <class Ar>
 	void serialize(Ar& ar) {
-		serializer(ar, version, txnBatchId, metadataVersion, conflictingKeyRanges);
+		serializer(ar, version, txnBatchId, metadataVersion, conflictingKRIndices);
 	}
 
 	CommitID() : version(invalidVersion), txnBatchId(0) {}
-	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<std::vector<int>>& conflictingKeyRanges = Optional<std::vector<int>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKeyRanges(conflictingKeyRanges) {}
+	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<std::vector<int>>& conflictingKRIndices = Optional<std::vector<int>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKRIndices(conflictingKRIndices) {}
 };
 
 struct CommitTransactionRequest : TimedRequest {
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 3a4fe6a20b..52655e957c 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1034,15 +1034,15 @@ ACTOR Future<Void> commitBatch(
 		else {
 			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
 			if (trs[t].transaction.report_conflicting_keys) {
-				std::vector<int> conflictingEntryIds;
+				std::vector<int> conflictingKRIndices;
 				for (int resolverInd : transactionResolverMap[t]) {
 					for (auto const & rCRIndex : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
-						conflictingEntryIds.emplace_back(rCRIndex);
+						conflictingKRIndices.emplace_back(rCRIndex);
 					}
 				}
 				// At least one keyRange index should be returned
-				ASSERT(conflictingEntryIds.size());
-				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<std::vector<int>>(conflictingEntryIds)));
+				ASSERT(conflictingKRIndices.size());
+				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<std::vector<int>>(conflictingKRIndices)));
 			} else {
 				trs[t].reply.sendError(not_committed());
 			}

From db901d626a0ab11e94b8f51f2ac42356819d9a32 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 16 Jan 2020 14:41:06 -0800
Subject: [PATCH 18/45] Update some var names and make returned list in
 ResolveTransactionBatchReply as Optional

---
 fdbclient/NativeAPI.actor.cpp         | 4 ++--
 fdbserver/MasterProxyServer.actor.cpp | 4 ++--
 fdbserver/Resolver.actor.cpp          | 3 ++-
 fdbserver/ResolverInterface.h         | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index d02ff9cd48..ccdba29164 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2676,7 +2676,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 				} else {
 					// clear the RYW transaction which contains previous conflicting keys
 					tr->info.conflictingKeysRYW.reset();
-					if (ci.conflictingKeyRanges.present()){
+					if (ci.conflictingKRIndices.present()){
 						// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
 						// It is difficult to take care of all corner cases of what getRange does.
 						// Consequently, we use a hack way here to achieve it.
@@ -2692,7 +2692,7 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						tr->info.conflictingKeysRYW->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 						tr->info.conflictingKeysRYW->clear(systemKeys);
 						// merge duplicate indices
-						const auto cKRs = ci.conflictingKeyRanges.get();
+						const auto cKRs = ci.conflictingKRIndices.get();
 						std::set<int> mergedIds(cKRs.begin(), cKRs.end());
 						for (auto const & rCRIndex : mergedIds) {
 							const KeyRangeRef & kr = req.transaction.read_conflict_ranges[rCRIndex];
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 52655e957c..7b4aaab6bc 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1036,9 +1036,9 @@ ACTOR Future<Void> commitBatch(
 			if (trs[t].transaction.report_conflicting_keys) {
 				std::vector<int> conflictingKRIndices;
 				for (int resolverInd : transactionResolverMap[t]) {
-					for (auto const & rCRIndex : resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]]){
+					const std::vector<int>& cKRs = resolution[resolverInd].conflictingKeyRangeMap.get()[nextTr[resolverInd]];
+					for (auto const & rCRIndex : cKRs)
 						conflictingKRIndices.emplace_back(rCRIndex);
-					}
 				}
 				// At least one keyRange index should be returned
 				ASSERT(conflictingKRIndices.size());
diff --git a/fdbserver/Resolver.actor.cpp b/fdbserver/Resolver.actor.cpp
index 7789a216db..2f0933d5c8 100644
--- a/fdbserver/Resolver.actor.cpp
+++ b/fdbserver/Resolver.actor.cpp
@@ -165,7 +165,8 @@ ACTOR Future<Void> resolveBatch(
 		for (int c = 0; c<tooOldList.size(); c++)
 			reply.committed[tooOldList[c]] = ConflictBatch::TransactionTooOld;
 		
-		reply.conflictingKeyRangeMap = std::move(conflictingKeyRangeMap);
+		if (!conflictingKeyRangeMap.empty())
+			reply.conflictingKeyRangeMap = std::move(conflictingKeyRangeMap);
 
 		ASSERT(req.prevVersion >= 0 || req.txnStateTransactions.size() == 0); // The master's request should not have any state transactions
 
diff --git a/fdbserver/ResolverInterface.h b/fdbserver/ResolverInterface.h
index 829888d6ef..d53779a5c7 100644
--- a/fdbserver/ResolverInterface.h
+++ b/fdbserver/ResolverInterface.h
@@ -77,7 +77,7 @@ struct ResolveTransactionBatchReply {
 	VectorRef<uint8_t> committed;
 	Optional<UID> debugID;
 	VectorRef<VectorRef<StateTransactionRef>> stateMutations;  // [version][transaction#] -> (committed, [mutation#])
-	std::map<int, std::vector<int>> conflictingKeyRangeMap; // transaction index -> conflicting read_conflict_range ids given by the resolver
+	Optional<std::map<int, std::vector<int>>> conflictingKeyRangeMap; // transaction index -> conflicting read_conflict_range ids given by the resolver
 
 	template <class Archive>
 	void serialize(Archive& ar) {

From 4de8f2fdfa29731aff09c9e8b9653407e6a10546 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 16 Jan 2020 14:42:14 -0800
Subject: [PATCH 19/45] Move the test to basic correctness test

---
 tests/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 942db53a73..74c3eabefe 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -66,7 +66,6 @@ add_fdb_test(TEST_FILES RedwoodCorrectnessUnits.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessBTree.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectnessPager.txt IGNORE)
 add_fdb_test(TEST_FILES fast/RedwoodCorrectnessBTree.txt IGNORE)
-add_fdb_test(TEST_FILES fast/ReportConflictingKeys.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodCorrectness.txt IGNORE)
 add_fdb_test(TEST_FILES RedwoodPerfTests.txt IGNORE)
 add_fdb_test(TEST_FILES SampleNoSimAttrition.txt IGNORE)
@@ -114,6 +113,7 @@ add_fdb_test(TEST_FILES fast/MemoryLifetime.txt)
 add_fdb_test(TEST_FILES fast/MoveKeysCycle.txt)
 add_fdb_test(TEST_FILES fast/RandomSelector.txt)
 add_fdb_test(TEST_FILES fast/RandomUnitTests.txt)
+add_fdb_test(TEST_FILES fast/ReportConflictingKeys.txt)
 add_fdb_test(TEST_FILES fast/SelectorCorrectness.txt)
 add_fdb_test(TEST_FILES fast/Sideband.txt)
 add_fdb_test(TEST_FILES fast/SidebandWithStatus.txt)

From d17d1c88cdf77db4d97a729310b34b678d242d0d Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 17 Jan 2020 11:31:43 -0800
Subject: [PATCH 20/45] add const to local KeyRef

---
 fdbclient/ReadYourWrites.actor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 522839623d..3585e65942 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1286,7 +1286,7 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// <key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
 	// Due to the implementation of resolver, currently, 
 	// we can only give key ranges that contain at least one key is conflicting.
-	KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
+	const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
 	// TODO : This condition need to be changed in the future when we have more special keys under "\xff\xff/transaction/"
 	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {
 		// Remove the special key prefix "\xff\xff/transaction/conflicting_keys/"

From 670c3e629f1667a575de9f8b0e148575643bfe39 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 17 Jan 2020 14:24:13 -0800
Subject: [PATCH 21/45] Fix arena issue and change std::vector back to
 VectorRef

---
 fdbclient/MasterProxyInterface.h      |  4 ++--
 fdbserver/ConflictSet.h               |  5 +++--
 fdbserver/MasterProxyServer.actor.cpp |  8 +++----
 fdbserver/Resolver.actor.cpp          |  9 +++-----
 fdbserver/ResolverInterface.h         |  4 ++--
 fdbserver/SkipList.cpp                | 30 ++++++++++++++++-----------
 6 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/fdbclient/MasterProxyInterface.h b/fdbclient/MasterProxyInterface.h
index 784331384b..89eb9d5eca 100644
--- a/fdbclient/MasterProxyInterface.h
+++ b/fdbclient/MasterProxyInterface.h
@@ -104,7 +104,7 @@ struct CommitID {
 	Version version; 			// returns invalidVersion if transaction conflicts
 	uint16_t txnBatchId;
 	Optional<Value> metadataVersion;
-	Optional<std::vector<int>> conflictingKRIndices;
+	Optional<Standalone<VectorRef<int>>> conflictingKRIndices;
 
 	template <class Ar>
 	void serialize(Ar& ar) {
@@ -112,7 +112,7 @@ struct CommitID {
 	}
 
 	CommitID() : version(invalidVersion), txnBatchId(0) {}
-	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<std::vector<int>>& conflictingKRIndices = Optional<std::vector<int>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKRIndices(conflictingKRIndices) {}
+	CommitID( Version version, uint16_t txnBatchId, const Optional<Value>& metadataVersion, const Optional<Standalone<VectorRef<int>>>& conflictingKRIndices = Optional<Standalone<VectorRef<int>>>() ) : version(version), txnBatchId(txnBatchId), metadataVersion(metadataVersion), conflictingKRIndices(conflictingKRIndices) {}
 };
 
 struct CommitTransactionRequest : TimedRequest {
diff --git a/fdbserver/ConflictSet.h b/fdbserver/ConflictSet.h
index ffe80e0e6d..3284f9f4ab 100644
--- a/fdbserver/ConflictSet.h
+++ b/fdbserver/ConflictSet.h
@@ -33,7 +33,7 @@ void clearConflictSet( ConflictSet*, Version );
 void destroyConflictSet(ConflictSet*);
 
 struct ConflictBatch {
-	explicit ConflictBatch( ConflictSet*,  std::map< int, std::vector< int > >* conflictingKeyRangeMap = nullptr);
+	explicit ConflictBatch( ConflictSet*,  std::map< int, VectorRef< int > >* conflictingKeyRangeMap = nullptr, Arena* resolveBatchReplyArena = nullptr);
 	~ConflictBatch();
 
 	enum TransactionCommitResult {
@@ -54,7 +54,8 @@ private:
 	std::vector< std::pair<StringRef,StringRef> > combinedWriteConflictRanges;
 	std::vector< struct ReadConflictRange > combinedReadConflictRanges;
 	bool* transactionConflictStatus;
-	std::map< int, std::vector< int > >* conflictingKeyRangeMap;
+	std::map< int, VectorRef< int > >* conflictingKeyRangeMap;
+	Arena* resolveBatchReplyArena;
 
 	void checkIntraBatchConflicts();
 	void combineWriteConflictRanges();
diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 7b4aaab6bc..01be822575 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1034,15 +1034,15 @@ ACTOR Future<Void> commitBatch(
 		else {
 			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
 			if (trs[t].transaction.report_conflicting_keys) {
-				std::vector<int> conflictingKRIndices;
+				Standalone<VectorRef<int>> conflictingKRIndices;
 				for (int resolverInd : transactionResolverMap[t]) {
-					const std::vector<int>& cKRs = resolution[resolverInd].conflictingKeyRangeMap.get()[nextTr[resolverInd]];
+					auto const & cKRs = resolution[resolverInd].conflictingKeyRangeMap[nextTr[resolverInd]];
 					for (auto const & rCRIndex : cKRs)
-						conflictingKRIndices.emplace_back(rCRIndex);
+						conflictingKRIndices.push_back(conflictingKRIndices.arena(), rCRIndex);
 				}
 				// At least one keyRange index should be returned
 				ASSERT(conflictingKRIndices.size());
-				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<std::vector<int>>(conflictingKRIndices)));
+				trs[t].reply.send(CommitID(invalidVersion, t, Optional<Value>(), Optional<Standalone<VectorRef<int>>>(conflictingKRIndices)));
 			} else {
 				trs[t].reply.sendError(not_committed());
 			}
diff --git a/fdbserver/Resolver.actor.cpp b/fdbserver/Resolver.actor.cpp
index 2f0933d5c8..57700e4105 100644
--- a/fdbserver/Resolver.actor.cpp
+++ b/fdbserver/Resolver.actor.cpp
@@ -130,14 +130,15 @@ ACTOR Future<Void> resolveBatch(
 		if(req.debugID.present())
 			g_traceBatch.addEvent("CommitDebug", debugID.get().first(), "Resolver.resolveBatch.AfterOrderer");
 		
+		ResolveTransactionBatchReply &reply = proxyInfo.outstandingBatches[req.version];
+
 		vector<int> commitList;
 		vector<int> tooOldList;
-		std::map< int, std::vector< int > > conflictingKeyRangeMap;
 
 		// Detect conflicts
 		double expire = now() + SERVER_KNOBS->SAMPLE_EXPIRATION_TIME;
 		double tstart = timer();
-		ConflictBatch conflictBatch(self->conflictSet, &conflictingKeyRangeMap);
+		ConflictBatch conflictBatch(self->conflictSet, &reply.conflictingKeyRangeMap, &reply.arena);
 		int keys = 0;
 		for(int t=0; t<req.transactions.size(); t++) {
 			conflictBatch.addTransaction( req.transactions[t] );
@@ -156,7 +157,6 @@ ACTOR Future<Void> resolveBatch(
 		g_counters.conflictTransactions += req.transactions.size();
 		g_counters.conflictKeys += keys;
 
-		ResolveTransactionBatchReply &reply = proxyInfo.outstandingBatches[req.version];
 		reply.debugID = req.debugID;
 		reply.committed.resize( reply.arena, req.transactions.size() );
 		for(int c=0; c<commitList.size(); c++)
@@ -164,9 +164,6 @@ ACTOR Future<Void> resolveBatch(
 
 		for (int c = 0; c<tooOldList.size(); c++)
 			reply.committed[tooOldList[c]] = ConflictBatch::TransactionTooOld;
-		
-		if (!conflictingKeyRangeMap.empty())
-			reply.conflictingKeyRangeMap = std::move(conflictingKeyRangeMap);
 
 		ASSERT(req.prevVersion >= 0 || req.txnStateTransactions.size() == 0); // The master's request should not have any state transactions
 
diff --git a/fdbserver/ResolverInterface.h b/fdbserver/ResolverInterface.h
index d53779a5c7..e1f74707b9 100644
--- a/fdbserver/ResolverInterface.h
+++ b/fdbserver/ResolverInterface.h
@@ -77,11 +77,11 @@ struct ResolveTransactionBatchReply {
 	VectorRef<uint8_t> committed;
 	Optional<UID> debugID;
 	VectorRef<VectorRef<StateTransactionRef>> stateMutations;  // [version][transaction#] -> (committed, [mutation#])
-	Optional<std::map<int, std::vector<int>>> conflictingKeyRangeMap; // transaction index -> conflicting read_conflict_range ids given by the resolver
+	std::map<int, VectorRef<int>> conflictingKeyRangeMap; // transaction index -> conflicting read_conflict_range ids given by the resolver
 
 	template <class Archive>
 	void serialize(Archive& ar) {
-		serializer(ar, committed, stateMutations, arena, debugID, conflictingKeyRangeMap);
+		serializer(ar, committed, stateMutations, debugID, conflictingKeyRangeMap, arena);
 	}
 
 };
diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 038214cd60..3f1b6af859 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -123,10 +123,12 @@ struct ReadConflictRange {
 	StringRef begin, end;
 	Version version;
 	int transaction;
-	std::vector<int>* conflictingKeyRange;
 	int indexInTx;
-	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, int indexInTx, std::vector<int> * cKR = nullptr )
-		: begin(begin), end(end), version(version), transaction(transaction), indexInTx(indexInTx), conflictingKeyRange(cKR)
+	VectorRef<int>* conflictingKeyRange;
+	Arena* cKRArena;
+	
+	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, int indexInTx, VectorRef<int> * cKR = nullptr, Arena* cKRArena = nullptr)
+		: begin(begin), end(end), version(version), transaction(transaction), indexInTx(indexInTx), conflictingKeyRange(cKR), cKRArena(cKRArena)
 	{
 	}
 	bool operator<(const ReadConflictRange& rhs) const { return compare(begin, rhs.begin)<0; }
@@ -531,7 +533,7 @@ public:
 
 		int started = min(M,count);
 		for(int i=0; i<started; i++){
-			inProgress[i].init( ranges[i], header, transactionConflictStatus, ranges[i].indexInTx, ranges[i].conflictingKeyRange );
+			inProgress[i].init( ranges[i], header, transactionConflictStatus, ranges[i].indexInTx, ranges[i].conflictingKeyRange, ranges[i].cKRArena );
 			nextJob[i] = i+1;
 		}
 		nextJob[started-1] = 0;
@@ -548,7 +550,7 @@ public:
 				}
 				else {
 					int temp = started++;
-					inProgress[job].init( ranges[temp], header, transactionConflictStatus, ranges[temp].indexInTx, ranges[temp].conflictingKeyRange );
+					inProgress[job].init( ranges[temp], header, transactionConflictStatus, ranges[temp].indexInTx, ranges[temp].conflictingKeyRange, ranges[temp].cKRArena );
 				}
 			}
 			prevJob = job;
@@ -762,13 +764,15 @@ private:
 		bool *result;
 		int state;
 		int indexInTx;
-		std::vector<int>* conflictingKeyRange; // null if report_conflicting_keys is not enabled.
+		VectorRef<int>* conflictingKeyRange; // null if report_conflicting_keys is not enabled.
+		Arena* cKRArena; // null if report_conflicting_keys is not enabled.
 
-		void init( const ReadConflictRange& r, Node* header, bool* tCS, int indexInTx, std::vector<int>* cKR) {
+		void init( const ReadConflictRange& r, Node* header, bool* tCS, int indexInTx, VectorRef<int>* cKR, Arena* cKRArena) {
 			this->start.init( r.begin, header );
 			this->end.init( r.end, header );
 			this->version = r.version;
 			this->indexInTx = indexInTx;
+			this->cKRArena = cKRArena;
 			result = &tCS[ r.transaction ];
 			conflictingKeyRange = cKR;
 			this->state = 0;
@@ -778,7 +782,7 @@ private:
 		bool conflict() { 
 			*result = true;
 			if(conflictingKeyRange != nullptr)
-				conflictingKeyRange->push_back(indexInTx);
+				conflictingKeyRange->push_back(*cKRArena, indexInTx);
 			return true;
 		}
 
@@ -974,8 +978,8 @@ void destroyConflictSet(ConflictSet* cs) {
 	delete cs;
 }
 
-ConflictBatch::ConflictBatch( ConflictSet* cs, std::map< int, std::vector< int > >* conflictingKeyRangeMap )
-	: cs(cs), transactionCount(0), conflictingKeyRangeMap(conflictingKeyRangeMap)
+ConflictBatch::ConflictBatch( ConflictSet* cs, std::map< int, VectorRef< int > >* conflictingKeyRangeMap, Arena* resolveBatchReplyArena )
+	: cs(cs), transactionCount(0), conflictingKeyRangeMap(conflictingKeyRangeMap),resolveBatchReplyArena(resolveBatchReplyArena)
 {
 }
 
@@ -1010,7 +1014,9 @@ void ConflictBatch::addTransaction( const CommitTransactionRef& tr ) {
 			points.emplace_back(range.begin, false, true, false, t, &info->readRanges[r].first);
 			//points.back().keyEnd = StringRef(buf,range.second);
 			points.emplace_back(range.end, false, false, false, t, &info->readRanges[r].second);
-			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t, r, tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t] : nullptr);
+			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t, r,
+				tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t] : nullptr,
+				tr.report_conflicting_keys ? resolveBatchReplyArena : nullptr);
 		}
 		for(int r=0; r<tr.write_conflict_ranges.size(); r++) {
 			const KeyRangeRef& range = tr.write_conflict_ranges[r];
@@ -1158,7 +1164,7 @@ void ConflictBatch::checkIntraBatchConflicts() {
 		for(int i=0; i<tr.readRanges.size(); i++){
 			if ( mcs.any( tr.readRanges[i].first , tr.readRanges[i].second ) ) {
 				if (tr.reportConflictingKeys){
-					(*conflictingKeyRangeMap)[t].push_back(i);
+					(*conflictingKeyRangeMap)[t].push_back(*resolveBatchReplyArena, i);
 				}
 				conflict = true;
 				break;

From 17721a00f1fd363ba9931ad7cd247d1ccbd651bc Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 17 Jan 2020 14:26:45 -0800
Subject: [PATCH 22/45] change vars names

---
 fdbclient/ReadYourWrites.actor.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 3585e65942..70d1475d53 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1301,11 +1301,11 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		begin.setKey(beginConflictingKey);
 		end.setKey(endConflictingKey);
 		if (tr.info.conflictingKeysRYW) {
-			Future<Standalone<RangeResultRef>> resultWithKeyPrefixFuture = 
+			Future<Standalone<RangeResultRef>> resultFuture = 
 				tr.info.conflictingKeysRYW->getRange(begin, end, limits, snapshot, reverse);
 			// Make sure it happens locally
-			ASSERT(resultWithKeyPrefixFuture.isReady());
-			return resultWithKeyPrefixFuture.get();
+			ASSERT(resultFuture.isReady());
+			return resultFuture.get();
 		} else {
 			return Standalone<RangeResultRef>();
 		}

From 5b62d672dc9a43c576131984b6ff51ac2fbab2d7 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 17 Jan 2020 15:18:07 -0800
Subject: [PATCH 23/45] update comments

---
 fdbclient/ReadYourWrites.actor.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 70d1475d53..05d0eed503 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1284,8 +1284,7 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// The returned key value pairs are interpretted as :
 	// <key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
 	// <key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
-	// Due to the implementation of resolver, currently, 
-	// we can only give key ranges that contain at least one key is conflicting.
+	// Currently, the conflicting keyranges returned are original read_conflict_ranges.
 	const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
 	// TODO : This condition need to be changed in the future when we have more special keys under "\xff\xff/transaction/"
 	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {

From 5386b4ecdfa8da13f866c2b0244ea3bf71f91da2 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Tue, 21 Jan 2020 15:18:16 -0800
Subject: [PATCH 24/45] Adding prefix to returned keys, keeping consistency
 with query range

---
 fdbclient/ReadYourWrites.actor.cpp            | 20 +++++++++++++------
 .../workloads/ReportConflictingKeys.actor.cpp | 17 ++++++++++------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 05d0eed503..21378bc516 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1282,11 +1282,11 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// Use special key prefix "\xff\xff/transaction/conflicting_keys/<some_key>",
 	// to retrieve keys which caused latest not_committed(conflicting with another transaction) error.
 	// The returned key value pairs are interpretted as :
-	// <key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
-	// <key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
+	// prefix/<key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
+	// prefix/<key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
 	// Currently, the conflicting keyranges returned are original read_conflict_ranges.
 	const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
-	// TODO : This condition need to be changed in the future when we have more special keys under "\xff\xff/transaction/"
+	// TODO : This condition needs to be changed in the future when we have more special keys under "\xff\xff/transaction/"
 	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {
 		// Remove the special key prefix "\xff\xff/transaction/conflicting_keys/"
 		KeyRef beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPreifx);
@@ -1300,11 +1300,19 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 		begin.setKey(beginConflictingKey);
 		end.setKey(endConflictingKey);
 		if (tr.info.conflictingKeysRYW) {
-			Future<Standalone<RangeResultRef>> resultFuture = 
+			Future<Standalone<RangeResultRef>> resultWithoutPrefixFuture = 
 				tr.info.conflictingKeysRYW->getRange(begin, end, limits, snapshot, reverse);
 			// Make sure it happens locally
-			ASSERT(resultFuture.isReady());
-			return resultFuture.get();
+			ASSERT(resultWithoutPrefixFuture.isReady());
+			Standalone<RangeResultRef> resultWithoutPrefix = resultWithoutPrefixFuture.get();
+			// Add prefix to results, making keys consistent with the getRange query
+			Standalone<RangeResultRef> resultWithPrefix;
+			resultWithPrefix.reserve(resultWithPrefix.arena(), resultWithoutPrefix.size());
+			for (auto const & kv : resultWithoutPrefix) {
+				KeyValueRef kvWithPrefix(kv.key.withPrefix(conflictingKeysPreifx), kv.value);
+				resultWithPrefix.push_back_deep(resultWithPrefix.arena(), kvWithPrefix);
+			}
+			return resultWithPrefix;
 		} else {
 			return Standalone<RangeResultRef>();
 		}
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 39f9a203ef..4891debdb2 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -180,7 +180,9 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				wait(tr.onError(e));
 				// check API correctness
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
-					state KeyRange ckr = KeyRangeRef(LiteralStringRef("\xff\xff/transaction/conflicting_keys/"), LiteralStringRef("\xff\xff/transaction/conflicting_keys/\xff"));
+					const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
+					state KeyRange ckr = KeyRangeRef(LiteralStringRef("").withPrefix(conflictingKeysPreifx),
+						LiteralStringRef("\xff").withPrefix(conflictingKeysPreifx));
 					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens locally
 					// Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture = tr.getRange(ckr, readConflictRanges.size() * 2);
@@ -188,11 +190,14 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
 					ASSERT( conflictingKeyRanges.size() && ( conflictingKeyRanges.size() % 2 == 0 ) );
 					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
-						KeyValueRef startKey = conflictingKeyRanges[i];
-						ASSERT(startKey.value == conflictingKeysTrue);
-						KeyValueRef endKey = conflictingKeyRanges[i+1];
-						ASSERT(endKey.value ==  conflictingKeysFalse);
-						KeyRangeRef kr = KeyRangeRef(startKey.key, endKey.key);
+						KeyValueRef startKeyWithPreifx = conflictingKeyRanges[i];
+						ASSERT(startKeyWithPreifx.value == conflictingKeysTrue);
+						KeyValueRef endKeyWithPrefix = conflictingKeyRanges[i+1];
+						ASSERT(endKeyWithPrefix.value ==  conflictingKeysFalse);
+						// Remove the prefix of returning keys
+						Key startKey = startKeyWithPreifx.key.removePrefix(conflictingKeysPreifx);
+						Key endKey = endKeyWithPrefix.key.removePrefix(conflictingKeysPreifx);
+						KeyRangeRef kr = KeyRangeRef(startKey, endKey);
 						if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
 								// Read_conflict_range remains same in the resolver.
 								// Thus, the returned keyrange is either the original read_conflict_range or merged

From 7224807d1d822fab7cf9e29a73f11638b6a40073 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Tue, 21 Jan 2020 15:25:43 -0800
Subject: [PATCH 25/45] fix typo in var name

---
 fdbclient/ReadYourWrites.actor.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 21378bc516..42dd7bf16a 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1285,12 +1285,12 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// prefix/<key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
 	// prefix/<key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
 	// Currently, the conflicting keyranges returned are original read_conflict_ranges.
-	const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
+	const KeyRef conflictingKeysPrefix = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
 	// TODO : This condition needs to be changed in the future when we have more special keys under "\xff\xff/transaction/"
-	if (begin.getKey().startsWith(conflictingKeysPreifx) && end.getKey().startsWith(conflictingKeysPreifx)) {
+	if (begin.getKey().startsWith(conflictingKeysPrefix) && end.getKey().startsWith(conflictingKeysPrefix)) {
 		// Remove the special key prefix "\xff\xff/transaction/conflicting_keys/"
-		KeyRef beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPreifx);
-		KeyRef endConflictingKey = end.getKey().removePrefix(conflictingKeysPreifx);
+		KeyRef beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPrefix);
+		KeyRef endConflictingKey = end.getKey().removePrefix(conflictingKeysPrefix);
 
 		// Check if the conflicting key range to be read is valid
 		KeyRef maxKey = getMaxReadKey();
@@ -1309,8 +1309,8 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 			Standalone<RangeResultRef> resultWithPrefix;
 			resultWithPrefix.reserve(resultWithPrefix.arena(), resultWithoutPrefix.size());
 			for (auto const & kv : resultWithoutPrefix) {
-				KeyValueRef kvWithPrefix(kv.key.withPrefix(conflictingKeysPreifx), kv.value);
-				resultWithPrefix.push_back_deep(resultWithPrefix.arena(), kvWithPrefix);
+				KeyValueRef kvWithPrefix(kv.key.withPrefix(conflictingKeysPrefix, resultWithPrefix.arena()), kv.value);
+				resultWithPrefix.push_back(resultWithPrefix.arena(), kvWithPrefix);
 			}
 			return resultWithPrefix;
 		} else {

From 3a98c691b68bd3704fcc082910b1393a1cb29c66 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 4 Mar 2020 11:42:19 -0800
Subject: [PATCH 26/45] Update comments

---
 fdbserver/SkipList.cpp                        |  4 +-
 .../workloads/ReportConflictingKeys.actor.cpp | 39 ++++++++++---------
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 9d949a4c84..33fe40e696 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -608,8 +608,8 @@ private:
 		bool* result;
 		int state;
 		int indexInTx;
-		VectorRef<int>* conflictingKeyRange; // null if report_conflicting_keys is not enabled.
-		Arena* cKRArena; // null if report_conflicting_keys is not enabled.
+		VectorRef<int>* conflictingKeyRange; // nullptr if report_conflicting_keys is not enabled.
+		Arena* cKRArena; // nullptr if report_conflicting_keys is not enabled.
 
 		void init( const ReadConflictRange& r, Node* header, bool* tCS, int indexInTx, VectorRef<int>* cKR, Arena* cKRArena) {
 			this->start.init( r.begin, header );
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 4891debdb2..1d978ca8da 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -47,7 +47,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		    getOption(options, LiteralStringRef("keyPrefix"), LiteralStringRef("ReportConflictingKeysWorkload"))
 		        .toString());
 		keyBytes = getOption(options, LiteralStringRef("keyBytes"), 16);
-		
+
 		readConflictRangeCount = getOption(options, LiteralStringRef("readConflictRangeCountPerTx"), 1);
 		writeConflictRangeCount = getOption(options, LiteralStringRef("writeConflictRangeCountPerTx"), 1);
 		// modeled by geometric distribution: (1 - prob) / prob = mean
@@ -59,10 +59,9 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		// used for generating keyPrefix
 		keyPrefixBytes = getOption(options, LiteralStringRef("keyPrefixBytes"), 0);
 		if (keyPrefixBytes) {
-			prefixCount = 255 * std::round(std::exp2(8*(keyPrefixBytes-1)));
+			prefixCount = 255 * std::round(std::exp2(8 * (keyPrefixBytes - 1)));
 			ASSERT(keyPrefixBytes + 16 <= keyBytes);
-		}
-		else {
+		} else {
 			ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
 		}
 		nodeCountPerPrefix = getOption(options, LiteralStringRef("nodeCountPerPrefix"), 100);
@@ -106,12 +105,13 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		double p = (double)n / nodeCountPerPrefix;
 		int paddingLen = keyBytes - 16 - keyPrefixBytes;
 		// left padding by zero
-		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p)).withPrefix( prefixIdx >= 0 ? keyPrefixForIndex( prefixIdx) : keyPrefix);
+		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p))
+		    .withPrefix(prefixIdx >= 0 ? keyPrefixForIndex(prefixIdx) : keyPrefix);
 	}
 
 	Key keyPrefixForIndex(uint64_t n) {
 		Key prefix = makeString(keyPrefixBytes);
-		uint8_t * head = mutateString(prefix);
+		uint8_t* head = mutateString(prefix);
 		memset(head, 0, keyPrefixBytes);
 		int offset = keyPrefixBytes - 1;
 		while (n) {
@@ -182,29 +182,30 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
 					const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
 					state KeyRange ckr = KeyRangeRef(LiteralStringRef("").withPrefix(conflictingKeysPreifx),
-						LiteralStringRef("\xff").withPrefix(conflictingKeysPreifx));
-					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens locally
-					// Thus, the error handling is not needed here
-					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture = tr.getRange(ckr, readConflictRanges.size() * 2);
+					                                 LiteralStringRef("\xff").withPrefix(conflictingKeysPreifx));
+					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens
+					// locally Thus, the error handling is not needed here
+					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture =
+					    tr.getRange(ckr, readConflictRanges.size() * 2);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
-					ASSERT( conflictingKeyRanges.size() && ( conflictingKeyRanges.size() % 2 == 0 ) );
+					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 0));
 					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
 						KeyValueRef startKeyWithPreifx = conflictingKeyRanges[i];
 						ASSERT(startKeyWithPreifx.value == conflictingKeysTrue);
-						KeyValueRef endKeyWithPrefix = conflictingKeyRanges[i+1];
-						ASSERT(endKeyWithPrefix.value ==  conflictingKeysFalse);
+						KeyValueRef endKeyWithPrefix = conflictingKeyRanges[i + 1];
+						ASSERT(endKeyWithPrefix.value == conflictingKeysFalse);
 						// Remove the prefix of returning keys
 						Key startKey = startKeyWithPreifx.key.removePrefix(conflictingKeysPreifx);
 						Key endKey = endKeyWithPrefix.key.removePrefix(conflictingKeysPreifx);
 						KeyRangeRef kr = KeyRangeRef(startKey, endKey);
 						if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
-								// Read_conflict_range remains same in the resolver.
-								// Thus, the returned keyrange is either the original read_conflict_range or merged
-								// by several overlapped ones In either case, it contains at least one original
-								// read_conflict_range
-								return kr.contains(rCR);
-							})) {
+							    // Read_conflict_range remains same in the resolver.
+							    // Thus, the returned keyrange is either the original read_conflict_range or merged
+							    // by several overlapped ones In either case, it contains at least one original
+							    // read_conflict_range
+							    return kr.contains(rCR);
+						    })) {
 							++self->invalidReports;
 							TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
 						}

From c63909c18ccc686141a69ddac69ba3dc1791b8e9 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 4 Mar 2020 11:44:14 -0800
Subject: [PATCH 27/45] clang-format

---
 fdbserver/ConflictSet.h |  5 +--
 fdbserver/SkipList.cpp  | 68 +++++++++++++++++++++--------------------
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/fdbserver/ConflictSet.h b/fdbserver/ConflictSet.h
index 6b2fffaf08..369ad01701 100644
--- a/fdbserver/ConflictSet.h
+++ b/fdbserver/ConflictSet.h
@@ -33,7 +33,8 @@ void clearConflictSet(ConflictSet*, Version);
 void destroyConflictSet(ConflictSet*);
 
 struct ConflictBatch {
-	explicit ConflictBatch( ConflictSet*,  std::map< int, VectorRef< int > >* conflictingKeyRangeMap = nullptr, Arena* resolveBatchReplyArena = nullptr);
+	explicit ConflictBatch(ConflictSet*, std::map<int, VectorRef<int>>* conflictingKeyRangeMap = nullptr,
+	                       Arena* resolveBatchReplyArena = nullptr);
 	~ConflictBatch();
 
 	enum TransactionCommitResult {
@@ -55,7 +56,7 @@ private:
 	std::vector<std::pair<StringRef, StringRef>> combinedWriteConflictRanges;
 	std::vector<struct ReadConflictRange> combinedReadConflictRanges;
 	bool* transactionConflictStatus;
-	std::map< int, VectorRef< int > >* conflictingKeyRangeMap;
+	std::map<int, VectorRef<int>>* conflictingKeyRangeMap;
 	Arena* resolveBatchReplyArena;
 
 	void checkIntraBatchConflicts();
diff --git a/fdbserver/SkipList.cpp b/fdbserver/SkipList.cpp
index 33fe40e696..5f6520f1f4 100644
--- a/fdbserver/SkipList.cpp
+++ b/fdbserver/SkipList.cpp
@@ -73,12 +73,12 @@ struct ReadConflictRange {
 	int indexInTx;
 	VectorRef<int>* conflictingKeyRange;
 	Arena* cKRArena;
-	
-	ReadConflictRange( StringRef begin, StringRef end, Version version, int transaction, int indexInTx, VectorRef<int> * cKR = nullptr, Arena* cKRArena = nullptr)
-		: begin(begin), end(end), version(version), transaction(transaction), indexInTx(indexInTx), conflictingKeyRange(cKR), cKRArena(cKRArena)
-	{
-	}
-	bool operator<(const ReadConflictRange& rhs) const { return compare(begin, rhs.begin)<0; }
+
+	ReadConflictRange(StringRef begin, StringRef end, Version version, int transaction, int indexInTx,
+	                  VectorRef<int>* cKR = nullptr, Arena* cKRArena = nullptr)
+	  : begin(begin), end(end), version(version), transaction(transaction), indexInTx(indexInTx),
+	    conflictingKeyRange(cKR), cKRArena(cKRArena) {}
+	bool operator<(const ReadConflictRange& rhs) const { return compare(begin, rhs.begin) < 0; }
 };
 
 struct KeyInfo {
@@ -293,10 +293,10 @@ private:
 		int nPointers, valueLength;
 	};
 
-	static force_inline bool less( const uint8_t* a, int aLen, const uint8_t* b, int bLen ) {
-		int c = memcmp(a,b,min(aLen,bLen));
-		if (c<0) return true;
-		if (c>0) return false;
+	static force_inline bool less(const uint8_t* a, int aLen, const uint8_t* b, int bLen) {
+		int c = memcmp(a, b, min(aLen, bLen));
+		if (c < 0) return true;
+		if (c > 0) return false;
 		return aLen < bLen;
 	}
 
@@ -419,10 +419,11 @@ public:
 		CheckMax inProgress[M];
 		if (!count) return;
 
-		int started = min(M,count);
-		for(int i=0; i<started; i++){
-			inProgress[i].init( ranges[i], header, transactionConflictStatus, ranges[i].indexInTx, ranges[i].conflictingKeyRange, ranges[i].cKRArena );
-			nextJob[i] = i+1;
+		int started = min(M, count);
+		for (int i = 0; i < started; i++) {
+			inProgress[i].init(ranges[i], header, transactionConflictStatus, ranges[i].indexInTx,
+			                   ranges[i].conflictingKeyRange, ranges[i].cKRArena);
+			nextJob[i] = i + 1;
 		}
 		nextJob[started - 1] = 0;
 
@@ -435,10 +436,10 @@ public:
 					if (prevJob == job) break;
 					nextJob[prevJob] = nextJob[job];
 					job = prevJob;
-				}
-				else {
+				} else {
 					int temp = started++;
-					inProgress[job].init( ranges[temp], header, transactionConflictStatus, ranges[temp].indexInTx, ranges[temp].conflictingKeyRange, ranges[temp].cKRArena );
+					inProgress[job].init(ranges[temp], header, transactionConflictStatus, ranges[temp].indexInTx,
+					                     ranges[temp].conflictingKeyRange, ranges[temp].cKRArena);
 				}
 			}
 			prevJob = job;
@@ -611,22 +612,22 @@ private:
 		VectorRef<int>* conflictingKeyRange; // nullptr if report_conflicting_keys is not enabled.
 		Arena* cKRArena; // nullptr if report_conflicting_keys is not enabled.
 
-		void init( const ReadConflictRange& r, Node* header, bool* tCS, int indexInTx, VectorRef<int>* cKR, Arena* cKRArena) {
-			this->start.init( r.begin, header );
-			this->end.init( r.end, header );
+		void init(const ReadConflictRange& r, Node* header, bool* tCS, int indexInTx, VectorRef<int>* cKR,
+		          Arena* cKRArena) {
+			this->start.init(r.begin, header);
+			this->end.init(r.end, header);
 			this->version = r.version;
 			this->indexInTx = indexInTx;
 			this->cKRArena = cKRArena;
-			result = &tCS[ r.transaction ];
+			result = &tCS[r.transaction];
 			conflictingKeyRange = cKR;
 			this->state = 0;
 		}
 
 		bool noConflict() { return true; }
-		bool conflict() { 
+		bool conflict() {
 			*result = true;
-			if(conflictingKeyRange != nullptr)
-				conflictingKeyRange->push_back(*cKRArena, indexInTx);
+			if (conflictingKeyRange != nullptr) conflictingKeyRange->push_back(*cKRArena, indexInTx);
 			return true;
 		}
 
@@ -745,10 +746,10 @@ void destroyConflictSet(ConflictSet* cs) {
 	delete cs;
 }
 
-ConflictBatch::ConflictBatch( ConflictSet* cs, std::map< int, VectorRef< int > >* conflictingKeyRangeMap, Arena* resolveBatchReplyArena )
-	: cs(cs), transactionCount(0), conflictingKeyRangeMap(conflictingKeyRangeMap),resolveBatchReplyArena(resolveBatchReplyArena)
-{
-}
+ConflictBatch::ConflictBatch(ConflictSet* cs, std::map<int, VectorRef<int>>* conflictingKeyRangeMap,
+                             Arena* resolveBatchReplyArena)
+  : cs(cs), transactionCount(0), conflictingKeyRangeMap(conflictingKeyRangeMap),
+    resolveBatchReplyArena(resolveBatchReplyArena) {}
 
 ConflictBatch::~ConflictBatch() {}
 
@@ -779,8 +780,9 @@ void ConflictBatch::addTransaction(const CommitTransactionRef& tr) {
 			points.emplace_back(range.begin, true, false, t, &info->readRanges[r].first);
 			points.emplace_back(range.end, false, false, t, &info->readRanges[r].second);
 			combinedReadConflictRanges.emplace_back(range.begin, range.end, tr.read_snapshot, t, r,
-				tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t] : nullptr,
-				tr.report_conflicting_keys ? resolveBatchReplyArena : nullptr);
+			                                        tr.report_conflicting_keys ? &(*conflictingKeyRangeMap)[t]
+			                                                                   : nullptr,
+			                                        tr.report_conflicting_keys ? resolveBatchReplyArena : nullptr);
 		}
 		for (int r = 0; r < tr.write_conflict_ranges.size(); r++) {
 			const KeyRangeRef& range = tr.write_conflict_ranges[r];
@@ -817,9 +819,9 @@ void ConflictBatch::checkIntraBatchConflicts() {
 		const TransactionInfo& tr = *transactionInfo[t];
 		if (transactionConflictStatus[t]) continue;
 		bool conflict = tr.tooOld;
-		for(int i=0; i<tr.readRanges.size(); i++){
-			if ( mcs.any( tr.readRanges[i].first , tr.readRanges[i].second ) ) {
-				if (tr.reportConflictingKeys){
+		for (int i = 0; i < tr.readRanges.size(); i++) {
+			if (mcs.any(tr.readRanges[i].first, tr.readRanges[i].second)) {
+				if (tr.reportConflictingKeys) {
 					(*conflictingKeyRangeMap)[t].push_back(*resolveBatchReplyArena, i);
 				}
 				conflict = true;

From 1a5b41157e362a953accf7db930589981a3efbf5 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 11:17:34 -0700
Subject: [PATCH 28/45] add test for native transaction object

---
 fdbserver/workloads/ReportConflictingKeys.actor.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 1d978ca8da..b5934b74d3 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -163,6 +163,10 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				// used for throttling
 				wait(poisson(&lastTime, delay));
 				if (self->reportConflictingKeys) tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
+				// If READ_YOUR_WRITES_DISABLE set, it behaves like native transaction object
+				// where overlapped conflict ranges are not merged.
+				if (deterministicRandom()->random01() < 0.5)
+					tr.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
 				self->addRandomReadConflictRange(&tr, readConflictRanges);
 				self->addRandomWriteConflictRange(&tr);
 				++self->commits;

From d1c56d3b57dcfeda2e35299b9d74728d8c8584e1 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 12:25:50 -0700
Subject: [PATCH 29/45] add constant KeyRefs in SystemData

---
 fdbclient/SystemData.cpp | 3 +++
 fdbclient/SystemData.h   | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/fdbclient/SystemData.cpp b/fdbclient/SystemData.cpp
index f7bf8112fc..5dcac85466 100644
--- a/fdbclient/SystemData.cpp
+++ b/fdbclient/SystemData.cpp
@@ -28,6 +28,7 @@ const KeyRangeRef systemKeys(systemKeysPrefix, LiteralStringRef("\xff\xff") );
 const KeyRangeRef nonMetadataSystemKeys(LiteralStringRef("\xff\x02"), LiteralStringRef("\xff\x03"));
 const KeyRangeRef allKeys = KeyRangeRef(normalKeys.begin, systemKeys.end);
 const KeyRef afterAllKeys = LiteralStringRef("\xff\xff\x00");
+const KeyRangeRef specialKeys = KeyRangeRef(LiteralStringRef("\xff\xff"), LiteralStringRef("\xff\xff\xff\xff"));
 
 // keyServersKeys.contains(k) iff k.startsWith(keyServersPrefix)
 const KeyRangeRef keyServersKeys( LiteralStringRef("\xff/keyServers/"), LiteralStringRef("\xff/keyServers0") );
@@ -58,6 +59,8 @@ void decodeKeyServersValue( const ValueRef& value, vector<UID>& src, vector<UID>
 	}
 }
 
+const KeyRef conflictingKeysPrefix = LiteralStringRef("/transaction/conflicting_keys/");
+const KeyRef conflictingKeysAbsolutePrefix = conflictingKeysPrefix.withPrefix(specialKeys.begin);
 const ValueRef conflictingKeysTrue = LiteralStringRef("1");
 const ValueRef conflictingKeysFalse = LiteralStringRef("0");
 
diff --git a/fdbclient/SystemData.h b/fdbclient/SystemData.h
index edb8c08aa5..0af5f8fac3 100644
--- a/fdbclient/SystemData.h
+++ b/fdbclient/SystemData.h
@@ -36,6 +36,7 @@ extern const KeyRangeRef normalKeys; // '' to systemKeys.begin
 extern const KeyRangeRef systemKeys;  // [FF] to [FF][FF]
 extern const KeyRangeRef nonMetadataSystemKeys; // [FF][00] to [FF][01]
 extern const KeyRangeRef allKeys; // '' to systemKeys.end
+extern const KeyRangeRef specialKeys; // [FF][FF] to [FF][FF][FF][FF]
 extern const KeyRef afterAllKeys;
 
 //    "\xff/keyServers/[[begin]]" := "[[vector<serverID>, vector<serverID>]]"
@@ -64,6 +65,7 @@ const Key serverKeysPrefixFor( UID serverID );
 UID serverKeysDecodeServer( const KeyRef& key );
 bool serverHasKey( ValueRef storedValue );
 
+extern const KeyRef conflictingKeysPrefix, conflictingKeysAbsolutePrefix;
 extern const ValueRef conflictingKeysTrue, conflictingKeysFalse;
 
 extern const KeyRef cacheKeysPrefix;

From bdabb8638e8d139b57a3181d286456a892471364 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 12:40:40 -0700
Subject: [PATCH 30/45] Change prefix

---
 fdbclient/ReadYourWrites.actor.cpp | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 97f20dcfb9..769a70cc7b 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1285,12 +1285,11 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// prefix/<key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
 	// prefix/<key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
 	// Currently, the conflicting keyranges returned are original read_conflict_ranges.
-	const KeyRef conflictingKeysPrefix = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
-	// TODO : This condition needs to be changed in the future when we have more special keys under "\xff\xff/transaction/"
-	if (begin.getKey().startsWith(conflictingKeysPrefix) && end.getKey().startsWith(conflictingKeysPrefix)) {
-		// Remove the special key prefix "\xff\xff/transaction/conflicting_keys/"
-		KeyRef beginConflictingKey = begin.getKey().removePrefix(conflictingKeysPrefix);
-		KeyRef endConflictingKey = end.getKey().removePrefix(conflictingKeysPrefix);
+	// TODO : This interface needs to be integrated into the framework that handles special keys' calls in the future
+	if (begin.getKey().startsWith(conflictingKeysAbsolutePrefix) && end.getKey().startsWith(conflictingKeysAbsolutePrefix)) {
+		// Remove the special key prefix "\xff\xff"
+		KeyRef beginConflictingKey = begin.getKey().removePrefix(specialKeys.begin);
+		KeyRef endConflictingKey = end.getKey().removePrefix(specialKeys.begin);
 
 		// Check if the conflicting key range to be read is valid
 		KeyRef maxKey = getMaxReadKey();
@@ -1309,7 +1308,7 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 			Standalone<RangeResultRef> resultWithPrefix;
 			resultWithPrefix.reserve(resultWithPrefix.arena(), resultWithoutPrefix.size());
 			for (auto const & kv : resultWithoutPrefix) {
-				KeyValueRef kvWithPrefix(kv.key.withPrefix(conflictingKeysPrefix, resultWithPrefix.arena()), kv.value);
+				KeyValueRef kvWithPrefix(kv.key.withPrefix(specialKeys.begin, resultWithPrefix.arena()), kv.value);
 				resultWithPrefix.push_back(resultWithPrefix.arena(), kvWithPrefix);
 			}
 			return resultWithPrefix;

From 6ae60870fcabf50363d8c7c6e42d9b95d6637bb4 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 13:20:40 -0700
Subject: [PATCH 31/45] use krmSetRange

---
 fdbclient/NativeAPI.actor.cpp | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 415bbadf2e..b9782de43a 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2752,22 +2752,26 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						// We create an empty RYWTransaction and write all conflicting key/values to it.
 						// Since it is RYWTr, we can call getRange on it with same parameters given to the original getRange.
 						tr->info.conflictingKeysRYW = std::make_shared<ReadYourWritesTransaction>(tr->getDatabase());
+						state Reference<ReadYourWritesTransaction> hackTr =
+							Reference<ReadYourWritesTransaction>(tr->info.conflictingKeysRYW.get());
+						state Standalone<VectorRef<int>> conflictingKRIndices = ci.conflictingKRIndices.get();
 						// To make the getRange call local, we need to explicitly set the read version here.
 						// This version number 100 set here does nothing but prevent getting read version from the proxy
 						tr->info.conflictingKeysRYW->setVersion(100);
 						// Clear the whole key space, thus, RYWTr knows to only read keys locally
 						tr->info.conflictingKeysRYW->clear(normalKeys);
-						// in case system keys are conflicting
-						tr->info.conflictingKeysRYW->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-						tr->info.conflictingKeysRYW->clear(systemKeys);
-						// merge duplicate indices
-						const auto cKRs = ci.conflictingKRIndices.get();
-						std::set<int> mergedIds(cKRs.begin(), cKRs.end());
+						// initialize value
+						wait(krmSetRange(hackTr, conflictingKeysPrefix, normalKeys, conflictingKeysFalse));
+						// drop duplicate indices and merge overlapped ranges
+						// Note: addReadConflictRange in native transaction object does not merge overlapped ranges
+						state std::set<int> mergedIds(conflictingKRIndices.begin(), conflictingKRIndices.end());
 						for (auto const & rCRIndex : mergedIds) {
-							const KeyRangeRef & kr = req.transaction.read_conflict_ranges[rCRIndex];
-							tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
-							tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
+							const KeyRange kr = req.transaction.read_conflict_ranges[rCRIndex];
+							// tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
+							// tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
+							wait(krmSetRange(hackTr, conflictingKeysPrefix, kr, conflictingKeysTrue));
 						}
+						hackTr.extractPtr(); // Avoid the Reference to destroy the RYW object
 					}
 
 					if (info.debugID.present())

From 02ee4f4c466deb79cee3a323111cd6548da1bf12 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 22:22:51 -0700
Subject: [PATCH 32/45] Update comments

---
 fdbserver/MasterProxyServer.actor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fdbserver/MasterProxyServer.actor.cpp b/fdbserver/MasterProxyServer.actor.cpp
index 782f6e0503..9948332f99 100644
--- a/fdbserver/MasterProxyServer.actor.cpp
+++ b/fdbserver/MasterProxyServer.actor.cpp
@@ -1143,7 +1143,7 @@ ACTOR Future<Void> commitBatch(
 			trs[t].reply.sendError(transaction_too_old());
 		}
 		else {
-			// If enable the option to report conflicting keys from resolvers, we union all conflicting key ranges here and send back through CommitID
+			// If enable the option to report conflicting keys from resolvers, we send back all keyranges' indices through CommitID
 			if (trs[t].transaction.report_conflicting_keys) {
 				Standalone<VectorRef<int>> conflictingKRIndices;
 				for (int resolverInd : transactionResolverMap[t]) {

From 0094293d50b839fb891df4f782c10dfb7d1bb6e9 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 23:11:49 -0700
Subject: [PATCH 33/45] add const vars

---
 fdbclient/SystemData.cpp | 2 +-
 fdbclient/SystemData.h   | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fdbclient/SystemData.cpp b/fdbclient/SystemData.cpp
index 5dcac85466..065358082f 100644
--- a/fdbclient/SystemData.cpp
+++ b/fdbclient/SystemData.cpp
@@ -60,7 +60,7 @@ void decodeKeyServersValue( const ValueRef& value, vector<UID>& src, vector<UID>
 }
 
 const KeyRef conflictingKeysPrefix = LiteralStringRef("/transaction/conflicting_keys/");
-const KeyRef conflictingKeysAbsolutePrefix = conflictingKeysPrefix.withPrefix(specialKeys.begin);
+const Key conflictingKeysAbsolutePrefix = conflictingKeysPrefix.withPrefix(specialKeys.begin);
 const ValueRef conflictingKeysTrue = LiteralStringRef("1");
 const ValueRef conflictingKeysFalse = LiteralStringRef("0");
 
diff --git a/fdbclient/SystemData.h b/fdbclient/SystemData.h
index 0af5f8fac3..6b9de0fbf1 100644
--- a/fdbclient/SystemData.h
+++ b/fdbclient/SystemData.h
@@ -65,7 +65,8 @@ const Key serverKeysPrefixFor( UID serverID );
 UID serverKeysDecodeServer( const KeyRef& key );
 bool serverHasKey( ValueRef storedValue );
 
-extern const KeyRef conflictingKeysPrefix, conflictingKeysAbsolutePrefix;
+extern const KeyRef conflictingKeysPrefix;
+extern const Key conflictingKeysAbsolutePrefix;
 extern const ValueRef conflictingKeysTrue, conflictingKeysFalse;
 
 extern const KeyRef cacheKeysPrefix;

From c2f0c41c52562774f36ee5c1094aa84b3cacbe47 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Wed, 11 Mar 2020 23:12:38 -0700
Subject: [PATCH 34/45] use krmSetRange

---
 fdbclient/NativeAPI.actor.cpp                 |  3 ++-
 .../workloads/ReportConflictingKeys.actor.cpp | 20 +++++++++----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index b9782de43a..a869fc3636 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2761,7 +2761,8 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						// Clear the whole key space, thus, RYWTr knows to only read keys locally
 						tr->info.conflictingKeysRYW->clear(normalKeys);
 						// initialize value
-						wait(krmSetRange(hackTr, conflictingKeysPrefix, normalKeys, conflictingKeysFalse));
+						// wait(krmSetRange(hackTr, conflictingKeysPrefix, normalKeys, conflictingKeysFalse));
+						tr->info.conflictingKeysRYW->set(conflictingKeysPrefix, conflictingKeysFalse);
 						// drop duplicate indices and merge overlapped ranges
 						// Note: addReadConflictRange in native transaction object does not merge overlapped ranges
 						state std::set<int> mergedIds(conflictingKRIndices.begin(), conflictingKRIndices.end());
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index b5934b74d3..ceee5e2fe0 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -184,24 +184,24 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				wait(tr.onError(e));
 				// check API correctness
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
-					const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
-					state KeyRange ckr = KeyRangeRef(LiteralStringRef("").withPrefix(conflictingKeysPreifx),
-					                                 LiteralStringRef("\xff").withPrefix(conflictingKeysPreifx));
+					// const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
+					state KeyRange ckr = KeyRangeRef(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix),
+					                                 LiteralStringRef("\xff\xff").withPrefix(conflictingKeysAbsolutePrefix));
 					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens
 					// locally Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture =
-					    tr.getRange(ckr, readConflictRanges.size() * 2);
+					    tr.getRange(ckr, readConflictRanges.size() * 2 + 1);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
-					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 0));
-					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
-						KeyValueRef startKeyWithPreifx = conflictingKeyRanges[i];
-						ASSERT(startKeyWithPreifx.value == conflictingKeysTrue);
+					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 1));
+					for (int i = 1; i < conflictingKeyRanges.size(); i += 2) {
+						KeyValueRef startKeyWithPrefix = conflictingKeyRanges[i];
+						ASSERT(startKeyWithPrefix.value == conflictingKeysTrue);
 						KeyValueRef endKeyWithPrefix = conflictingKeyRanges[i + 1];
 						ASSERT(endKeyWithPrefix.value == conflictingKeysFalse);
 						// Remove the prefix of returning keys
-						Key startKey = startKeyWithPreifx.key.removePrefix(conflictingKeysPreifx);
-						Key endKey = endKeyWithPrefix.key.removePrefix(conflictingKeysPreifx);
+						Key startKey = startKeyWithPrefix.key.removePrefix(conflictingKeysAbsolutePrefix);
+						Key endKey = endKeyWithPrefix.key.removePrefix(conflictingKeysAbsolutePrefix);
 						KeyRangeRef kr = KeyRangeRef(startKey, endKey);
 						if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
 							    // Read_conflict_range remains same in the resolver.

From 4e8cb0cb9653bc7e37caaabe68916bbb562b42bc Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 12 Mar 2020 09:53:00 -0700
Subject: [PATCH 35/45] add krmSetRangeCoalescing for RYWTr

---
 fdbclient/KeyRangeMap.actor.cpp | 67 +++++++++++++++++++++++++++++++++
 fdbclient/KeyRangeMap.h         |  1 +
 2 files changed, 68 insertions(+)

diff --git a/fdbclient/KeyRangeMap.actor.cpp b/fdbclient/KeyRangeMap.actor.cpp
index d3cb36f833..5dc785b2cf 100644
--- a/fdbclient/KeyRangeMap.actor.cpp
+++ b/fdbclient/KeyRangeMap.actor.cpp
@@ -216,3 +216,70 @@ ACTOR Future<Void> krmSetRangeCoalescing( Transaction *tr, Key mapPrefix, KeyRan
 
 	return Void();
 }
+
+ACTOR Future<Void> krmSetRangeCoalescing( Reference<ReadYourWritesTransaction> tr, Key mapPrefix, KeyRange range, KeyRange maxRange, Value value ) {
+	ASSERT(maxRange.contains(range));
+
+	state KeyRange withPrefix = KeyRangeRef( mapPrefix.toString() + range.begin.toString(), mapPrefix.toString() + range.end.toString() );
+	state KeyRange maxWithPrefix = KeyRangeRef( mapPrefix.toString() + maxRange.begin.toString(), mapPrefix.toString() + maxRange.end.toString() );
+
+	state vector<Future<Standalone<RangeResultRef>>> keys;
+	keys.push_back(tr->getRange(lastLessThan(withPrefix.begin), firstGreaterOrEqual(withPrefix.begin), 1, true));
+	keys.push_back(tr->getRange(lastLessOrEqual(withPrefix.end), firstGreaterThan(withPrefix.end) + 1, 2, true));
+	wait(waitForAll(keys));
+
+	//Determine how far to extend this range at the beginning
+	auto beginRange = keys[0].get();
+	bool hasBegin = beginRange.size() > 0 && beginRange[0].key.startsWith(mapPrefix);
+	Value beginValue = hasBegin ? beginRange[0].value : LiteralStringRef("");
+
+	state Key beginKey = withPrefix.begin;
+	if(beginValue == value) {
+		bool outsideRange = !hasBegin || beginRange[0].key < maxWithPrefix.begin;
+		beginKey = outsideRange ? maxWithPrefix.begin : beginRange[0].key;
+	}
+
+	//Determine how far to extend this range at the end
+	auto endRange = keys[1].get();
+	bool hasEnd = endRange.size() >= 1 && endRange[0].key.startsWith(mapPrefix) && endRange[0].key <= withPrefix.end;
+	bool hasNext = (endRange.size() == 2 && endRange[1].key.startsWith(mapPrefix)) || (endRange.size() == 1 && withPrefix.end < endRange[0].key && endRange[0].key.startsWith(mapPrefix));
+	Value existingValue = hasEnd ? endRange[0].value : LiteralStringRef("");
+	bool valueMatches = value == existingValue;
+
+	KeyRange conflictRange = KeyRangeRef( hasBegin ? beginRange[0].key : mapPrefix, withPrefix.begin );
+	if( !conflictRange.empty() )
+		tr->addReadConflictRange( conflictRange );
+
+	conflictRange = KeyRangeRef( hasEnd ? endRange[0].key : mapPrefix, hasNext ? keyAfter(endRange.end()[-1].key) : strinc( mapPrefix ) );
+	if( !conflictRange.empty() )
+		tr->addReadConflictRange( conflictRange );
+
+	state Key endKey;
+	state Value endValue;
+
+	//Case 1: Coalesce completely with the following range
+	if(hasNext && endRange.end()[-1].key <= maxWithPrefix.end && valueMatches) {
+		endKey = endRange.end()[-1].key;
+		endValue = endRange.end()[-1].value;
+	}
+
+	//Case 2: Coalesce with the following range only up to the end of maxRange
+	else if(valueMatches) {
+		endKey = maxWithPrefix.end;
+		endValue = existingValue;
+	}
+
+	//Case 3: Don't coalesce
+	else {
+		endKey = withPrefix.end;
+		endValue = existingValue;
+	}
+
+	tr->clear(KeyRangeRef(beginKey, endKey));
+
+	ASSERT(value != endValue || endKey == maxWithPrefix.end);
+	tr->set(beginKey, value);
+	tr->set(endKey, endValue);
+
+	return Void();
+}
diff --git a/fdbclient/KeyRangeMap.h b/fdbclient/KeyRangeMap.h
index aafd92cb69..f34badb988 100644
--- a/fdbclient/KeyRangeMap.h
+++ b/fdbclient/KeyRangeMap.h
@@ -103,6 +103,7 @@ void krmSetPreviouslyEmptyRange( struct CommitTransactionRef& tr, Arena& trArena
 Future<Void> krmSetRange( Transaction* const& tr, Key const& mapPrefix, KeyRange const& range, Value const& value );
 Future<Void> krmSetRange( Reference<ReadYourWritesTransaction> const& tr, Key const& mapPrefix, KeyRange const& range, Value const& value );
 Future<Void> krmSetRangeCoalescing( Transaction* const& tr, Key const& mapPrefix, KeyRange const& range, KeyRange const& maxRange, Value const& value );
+Future<Void> krmSetRangeCoalescing( Reference<ReadYourWritesTransaction> const& tr, Key const& mapPrefix, KeyRange const& range, KeyRange const& maxRange, Value const& value );
 Standalone<RangeResultRef> krmDecodeRanges( KeyRef mapPrefix, KeyRange keys, Standalone<RangeResultRef> kv );
 
 template <class Val, class Metric, class MetricFunc>

From 6f90228a0be252bb590679fe0e971212355299a6 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Thu, 12 Mar 2020 11:31:36 -0700
Subject: [PATCH 36/45] change to krmSetRangeCoalescing

---
 fdbclient/NativeAPI.actor.cpp                       |  3 ++-
 fdbserver/workloads/ReportConflictingKeys.actor.cpp | 10 +++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index a869fc3636..13c3825bef 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2770,7 +2770,8 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 							const KeyRange kr = req.transaction.read_conflict_ranges[rCRIndex];
 							// tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
 							// tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
-							wait(krmSetRange(hackTr, conflictingKeysPrefix, kr, conflictingKeysTrue));
+							// wait(krmSetRange(hackTr, conflictingKeysPrefix, kr, conflictingKeysTrue));
+							wait(krmSetRangeCoalescing(hackTr, conflictingKeysPrefix, kr, allKeys, conflictingKeysTrue));
 						}
 						hackTr.extractPtr(); // Avoid the Reference to destroy the RYW object
 					}
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index ceee5e2fe0..06aa509063 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -184,17 +184,17 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				wait(tr.onError(e));
 				// check API correctness
 				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
-					// const KeyRef conflictingKeysPreifx = LiteralStringRef("\xff\xff/transaction/conflicting_keys/");
-					state KeyRange ckr = KeyRangeRef(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix),
+					// \xff\xff/transaction/conflicting_keys is always false, we skip it here for simplicity
+					state KeyRange ckr = KeyRangeRef(keyAfter(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix)),
 					                                 LiteralStringRef("\xff\xff").withPrefix(conflictingKeysAbsolutePrefix));
 					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens
 					// locally Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture =
-					    tr.getRange(ckr, readConflictRanges.size() * 2 + 1);
+					    tr.getRange(ckr, readConflictRanges.size() * 2);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
-					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 1));
-					for (int i = 1; i < conflictingKeyRanges.size(); i += 2) {
+					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 0));
+					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
 						KeyValueRef startKeyWithPrefix = conflictingKeyRanges[i];
 						ASSERT(startKeyWithPrefix.value == conflictingKeysTrue);
 						KeyValueRef endKeyWithPrefix = conflictingKeyRanges[i + 1];

From 7118759dfa9e00cd86b538603253384103d18579 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 01:48:55 -0700
Subject: [PATCH 37/45] Delete code to test resolvers' performance, simplify
 the workload to only test correctness

---
 .../workloads/ReportConflictingKeys.actor.cpp | 86 ++++++-------------
 tests/fast/ReportConflictingKeys.txt          |  8 +-
 2 files changed, 27 insertions(+), 67 deletions(-)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 06aa509063..9afee15f54 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -31,9 +31,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	double testDuration, transactionsPerSecond, addReadConflictRangeProb, addWriteConflictRangeProb;
 	Key keyPrefix;
 
-	int nodeCountPerPrefix, actorCount, keyBytes, valueBytes, readConflictRangeCount, writeConflictRangeCount;
-	bool reportConflictingKeys, skipCorrectnessCheck;
-	uint64_t keyPrefixBytes, prefixCount;
+	int nodeCount, actorCount, keyBytes, valueBytes, readConflictRangeCount, writeConflictRangeCount;
 
 	PerfIntCounter invalidReports, commits, conflicts, retries, xacts;
 
@@ -41,8 +39,8 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	  : TestWorkload(wcx), invalidReports("InvalidReports"), conflicts("Conflicts"), retries("Retries"),
 	    commits("Commits"), xacts("Transactions") {
 		testDuration = getOption(options, LiteralStringRef("testDuration"), 10.0);
-		transactionsPerSecond = getOption(options, LiteralStringRef("transactionsPerSecond"), 5000.0) / clientCount;
-		actorCount = getOption(options, LiteralStringRef("actorsPerClient"), transactionsPerSecond / 5);
+		// transactionsPerSecond = getOption(options, LiteralStringRef("transactionsPerSecond"), 5000.0) / clientCount;
+		actorCount = getOption(options, LiteralStringRef("actorsPerClient"), 1);
 		keyPrefix = unprintable(
 		    getOption(options, LiteralStringRef("keyPrefix"), LiteralStringRef("ReportConflictingKeysWorkload"))
 		        .toString());
@@ -53,18 +51,8 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		// modeled by geometric distribution: (1 - prob) / prob = mean
 		addReadConflictRangeProb = readConflictRangeCount / (readConflictRangeCount + 1.0);
 		addWriteConflictRangeProb = writeConflictRangeCount / (writeConflictRangeCount + 1.0);
-		// If true, store key ranges conflicting with other txs
-		reportConflictingKeys = getOption(options, LiteralStringRef("reportConflictingKeys"), false);
-		skipCorrectnessCheck = getOption(options, LiteralStringRef("skipCorrectnessCheck"), false);
-		// used for generating keyPrefix
-		keyPrefixBytes = getOption(options, LiteralStringRef("keyPrefixBytes"), 0);
-		if (keyPrefixBytes) {
-			prefixCount = 255 * std::round(std::exp2(8 * (keyPrefixBytes - 1)));
-			ASSERT(keyPrefixBytes + 16 <= keyBytes);
-		} else {
-			ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
-		}
-		nodeCountPerPrefix = getOption(options, LiteralStringRef("nodeCountPerPrefix"), 100);
+		ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
+		nodeCount = getOption(options, LiteralStringRef("nodeCount"), 100);
 	}
 
 	std::string description() override { return "ReportConflictingKeysWorkload"; }
@@ -74,12 +62,8 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	Future<Void> start(const Database& cx) override { return _start(cx->clone(), this); }
 
 	ACTOR Future<Void> _start(Database cx, ReportConflictingKeysWorkload* self) {
-		std::vector<Future<Void>> clients;
-		for (int c = 0; c < self->actorCount; ++c) {
-			clients.push_back(self->conflictingClient(cx, self, self->actorCount / self->transactionsPerSecond, c));
-		}
-
-		wait(timeout(waitForAll(clients), self->testDuration, Void()));
+		if (self->clientId == 0)
+			wait(timeout(self->conflictingClient(cx, self), self->testDuration, Void()));
 		return Void();
 	}
 
@@ -101,68 +85,48 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	double getCheckTimeout() override { return std::numeric_limits<double>::max(); }
 
 	// Copied from tester.actor.cpp, added parameter to determine the key's length
-	Key keyForIndex(int prefixIdx, int n) {
-		double p = (double)n / nodeCountPerPrefix;
-		int paddingLen = keyBytes - 16 - keyPrefixBytes;
+	Key keyForIndex(int n) {
+		double p = (double)n / nodeCount;
+		int paddingLen = keyBytes - 16 - keyPrefix.size();
 		// left padding by zero
 		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p))
-		    .withPrefix(prefixIdx >= 0 ? keyPrefixForIndex(prefixIdx) : keyPrefix);
-	}
-
-	Key keyPrefixForIndex(uint64_t n) {
-		Key prefix = makeString(keyPrefixBytes);
-		uint8_t* head = mutateString(prefix);
-		memset(head, 0, keyPrefixBytes);
-		int offset = keyPrefixBytes - 1;
-		while (n) {
-			*(head + offset) = static_cast<uint8_t>(n % 256);
-			n /= 256;
-			offset -= 1;
-		}
-		return prefix;
+		    .withPrefix(keyPrefix);
 	}
 
 	void addRandomReadConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>& readConflictRanges) {
-		int startIdx, endIdx, startPrefixIdx, endPrefixIdx;
+		int startIdx, endIdx;
 		Key startKey, endKey;
 		while (deterministicRandom()->random01() < addReadConflictRangeProb) {
-			startPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(0, prefixCount) : -1;
-			endPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(startPrefixIdx, prefixCount) : -1;
-			startIdx = deterministicRandom()->randomInt(0, nodeCountPerPrefix);
-			endIdx = deterministicRandom()->randomInt(startPrefixIdx < endPrefixIdx ? 0 : startIdx, nodeCountPerPrefix);
-			startKey = keyForIndex(startPrefixIdx, startIdx);
-			endKey = keyForIndex(endPrefixIdx, endIdx);
+			startIdx = deterministicRandom()->randomInt(0, nodeCount);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
+			startKey = keyForIndex(startIdx);
+			endKey = keyForIndex(endIdx);
 			tr->addReadConflictRange(KeyRangeRef(startKey, endKey));
 			readConflictRanges.push_back(KeyRangeRef(startKey, endKey));
 		}
 	}
 
 	void addRandomWriteConflictRange(ReadYourWritesTransaction* tr) {
-		int startIdx, endIdx, startPrefixIdx, endPrefixIdx;
+		int startIdx, endIdx;
 		Key startKey, endKey;
 		while (deterministicRandom()->random01() < addWriteConflictRangeProb) {
-			startPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(0, prefixCount) : -1;
-			endPrefixIdx = keyPrefixBytes ? deterministicRandom()->randomInt(startPrefixIdx, prefixCount) : -1;
-			startIdx = deterministicRandom()->randomInt(0, nodeCountPerPrefix);
-			endIdx = deterministicRandom()->randomInt(startPrefixIdx < endPrefixIdx ? 0 : startIdx, nodeCountPerPrefix);
-			startKey = keyForIndex(startPrefixIdx, startIdx);
-			endKey = keyForIndex(endPrefixIdx, endIdx);
+			startIdx = deterministicRandom()->randomInt(0, nodeCount);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
+			startKey = keyForIndex(startIdx);
+			endKey = keyForIndex(endIdx);
 			tr->addWriteConflictRange(KeyRangeRef(startKey, endKey));
 		}
 	}
 
-	ACTOR Future<Void> conflictingClient(Database cx, ReportConflictingKeysWorkload* self, double delay,
-	                                     int actorIndex) {
+	ACTOR Future<Void> conflictingClient(Database cx, ReportConflictingKeysWorkload* self) {
 
 		state ReadYourWritesTransaction tr(cx);
-		state double lastTime = now();
+		state ReadYourWritesTransaction tr2(cx);
 		state std::vector<KeyRange> readConflictRanges;
 
 		loop {
 			try {
-				// used for throttling
-				wait(poisson(&lastTime, delay));
-				if (self->reportConflictingKeys) tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
+				tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
 				// If READ_YOUR_WRITES_DISABLE set, it behaves like native transaction object
 				// where overlapped conflict ranges are not merged.
 				if (deterministicRandom()->random01() < 0.5)
@@ -183,7 +147,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				}
 				wait(tr.onError(e));
 				// check API correctness
-				if (!self->skipCorrectnessCheck && self->reportConflictingKeys && isConflict) {
+				if (isConflict) {
 					// \xff\xff/transaction/conflicting_keys is always false, we skip it here for simplicity
 					state KeyRange ckr = KeyRangeRef(keyAfter(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix)),
 					                                 LiteralStringRef("\xff\xff").withPrefix(conflictingKeysAbsolutePrefix));
diff --git a/tests/fast/ReportConflictingKeys.txt b/tests/fast/ReportConflictingKeys.txt
index 9ee68a57fd..1010e2493a 100644
--- a/tests/fast/ReportConflictingKeys.txt
+++ b/tests/fast/ReportConflictingKeys.txt
@@ -1,12 +1,8 @@
 testTitle=ReportConflictingKeysTest
     testName=ReportConflictingKeys
     testDuration=10.0
-    transactionsPerSecond=100000
-    nodeCountPerPrefix=10000
-    actorsPerClient=256
+    nodeCount=10000
     keyPrefix=RCK
     keyBytes=64
     readConflictRangeCountPerTx=1
-    writeConflictRangeCountPerTx=1
-    reportConflictingKeys=true
-    skipCorrectnessCheck=false
\ No newline at end of file
+    writeConflictRangeCountPerTx=1
\ No newline at end of file

From 6e92716be749c572864828d666b972c00d99008b Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 12:41:48 -0700
Subject: [PATCH 38/45] update comments

---
 fdbclient/ReadYourWrites.actor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp
index 769a70cc7b..df039e83b3 100644
--- a/fdbclient/ReadYourWrites.actor.cpp
+++ b/fdbclient/ReadYourWrites.actor.cpp
@@ -1284,7 +1284,7 @@ Future< Standalone<RangeResultRef> > ReadYourWritesTransaction::getRange(
 	// The returned key value pairs are interpretted as :
 	// prefix/<key1> : '1' - any keys equal or larger than this key are (probably) conflicting keys
 	// prefix/<key2> : '0' - any keys equal or larger than this key are (definitely) not conflicting keys
-	// Currently, the conflicting keyranges returned are original read_conflict_ranges.
+	// Currently, the conflicting keyranges returned are original read_conflict_ranges or union of them.
 	// TODO : This interface needs to be integrated into the framework that handles special keys' calls in the future
 	if (begin.getKey().startsWith(conflictingKeysAbsolutePrefix) && end.getKey().startsWith(conflictingKeysAbsolutePrefix)) {
 		// Remove the special key prefix "\xff\xff"

From aef9b515de44f1c0ad12f3d5a2426fe94ef2bd5c Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 12:42:28 -0700
Subject: [PATCH 39/45] Change the workload to a more controlled test like
 ConflictRange test

---
 .../workloads/ReportConflictingKeys.actor.cpp | 90 +++++++++++++------
 tests/fast/ReportConflictingKeys.txt          |  8 +-
 2 files changed, 69 insertions(+), 29 deletions(-)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 9afee15f54..9d38b0a16a 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -48,9 +48,10 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 
 		readConflictRangeCount = getOption(options, LiteralStringRef("readConflictRangeCountPerTx"), 1);
 		writeConflictRangeCount = getOption(options, LiteralStringRef("writeConflictRangeCountPerTx"), 1);
-		// modeled by geometric distribution: (1 - prob) / prob = mean
-		addReadConflictRangeProb = readConflictRangeCount / (readConflictRangeCount + 1.0);
-		addWriteConflictRangeProb = writeConflictRangeCount / (writeConflictRangeCount + 1.0);
+		ASSERT(readConflictRangeCount >= 1 && writeConflictRangeCount >= 1);
+		// modeled by geometric distribution: (1 - prob) / prob = mean - 1, since we add at least one conflictRange to each tx
+		addReadConflictRangeProb = (readConflictRangeCount - 1.0) / readConflictRangeCount;
+		addWriteConflictRangeProb = (writeConflictRangeCount - 1.0) / writeConflictRangeCount;
 		ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
 		nodeCount = getOption(options, LiteralStringRef("nodeCount"), 100);
 	}
@@ -93,29 +94,32 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		    .withPrefix(keyPrefix);
 	}
 
-	void addRandomReadConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>& readConflictRanges) {
+	void addRandomReadConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>* readConflictRanges) {
 		int startIdx, endIdx;
 		Key startKey, endKey;
-		while (deterministicRandom()->random01() < addReadConflictRangeProb) {
+		do { // add at least one
 			startIdx = deterministicRandom()->randomInt(0, nodeCount);
-			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount+1);
 			startKey = keyForIndex(startIdx);
 			endKey = keyForIndex(endIdx);
 			tr->addReadConflictRange(KeyRangeRef(startKey, endKey));
-			readConflictRanges.push_back(KeyRangeRef(startKey, endKey));
-		}
+			if (readConflictRanges)
+				readConflictRanges->push_back(KeyRangeRef(startKey, endKey));
+		} while (deterministicRandom()->random01() < addReadConflictRangeProb);
 	}
 
-	void addRandomWriteConflictRange(ReadYourWritesTransaction* tr) {
+	void addRandomWriteConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>* writeConflictRanges) {
 		int startIdx, endIdx;
 		Key startKey, endKey;
-		while (deterministicRandom()->random01() < addWriteConflictRangeProb) {
+		do { // add at least one
 			startIdx = deterministicRandom()->randomInt(0, nodeCount);
-			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount+1);
 			startKey = keyForIndex(startIdx);
 			endKey = keyForIndex(endIdx);
 			tr->addWriteConflictRange(KeyRangeRef(startKey, endKey));
-		}
+			if (writeConflictRanges)
+				writeConflictRanges->push_back(KeyRangeRef(startKey, endKey));
+		} while (deterministicRandom()->random01() < addWriteConflictRangeProb);
 	}
 
 	ACTOR Future<Void> conflictingClient(Database cx, ReportConflictingKeysWorkload* self) {
@@ -123,38 +127,47 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		state ReadYourWritesTransaction tr(cx);
 		state ReadYourWritesTransaction tr2(cx);
 		state std::vector<KeyRange> readConflictRanges;
+		state std::vector<KeyRange> writeConflictRanges;
 
 		loop {
 			try {
-				tr.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
+				tr2.setOption(FDBTransactionOptions::REPORT_CONFLICTING_KEYS);
 				// If READ_YOUR_WRITES_DISABLE set, it behaves like native transaction object
 				// where overlapped conflict ranges are not merged.
 				if (deterministicRandom()->random01() < 0.5)
 					tr.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
-				self->addRandomReadConflictRange(&tr, readConflictRanges);
-				self->addRandomWriteConflictRange(&tr);
+				if (deterministicRandom()->random01() < 0.5)
+					tr2.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
+				Version readVersion = wait( tr.getReadVersion() );
+				tr2.setVersion(readVersion);
+				self->addRandomReadConflictRange(&tr, nullptr);
+				self->addRandomWriteConflictRange(&tr, &writeConflictRanges);
 				++self->commits;
 				wait(tr.commit());
 				++self->xacts;
-			} catch (Error& e) {
-				TraceEvent("FailedToCommitTx").error(e);
-				state bool isConflict = false;
-				if (e.code() == error_code_operation_cancelled)
-					throw;
-				else if (e.code() == error_code_not_committed) {
+
+				state bool foundConflict = false;
+				try {
+					self->addRandomReadConflictRange(&tr2, &readConflictRanges);
+					self->addRandomWriteConflictRange(&tr2, nullptr);
+					++self->commits;
+					wait(tr2.commit());
+					++self->xacts;
+				} catch (Error& e) {
+					if( e.code() != error_code_not_committed )
+						throw e;
+					foundConflict = true;
 					++self->conflicts;
-					isConflict = true;
 				}
-				wait(tr.onError(e));
 				// check API correctness
-				if (isConflict) {
+				if (foundConflict) {
 					// \xff\xff/transaction/conflicting_keys is always false, we skip it here for simplicity
 					state KeyRange ckr = KeyRangeRef(keyAfter(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix)),
 					                                 LiteralStringRef("\xff\xff").withPrefix(conflictingKeysAbsolutePrefix));
 					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens
 					// locally Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture =
-					    tr.getRange(ckr, readConflictRanges.size() * 2);
+					    tr2.getRange(ckr, readConflictRanges.size() * 2);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
 					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 0));
@@ -177,12 +190,37 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 							++self->invalidReports;
 							TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
 						}
+						else if (!std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(), [&kr](KeyRange wCR) {
+							    // Returned key range should be conflicting with at least one writeConflictRange
+							    return kr.intersects(wCR);
+						    })) {
+							++self->invalidReports;
+							TraceEvent(SevError, "TestFailure").detail("Reason", "Returned keyranges are not conflicting with any write ranges");
+						}
+					}
+				} else {
+					// make sure no conflicts between readConflictRange and writeConflictRange
+					for (const KeyRange& rCR : readConflictRanges) {
+						if (std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(), [&rCR](KeyRange wCR){
+							bool result = wCR.intersects(rCR);
+							if (result)
+								TraceEvent(SevError, "TestFailure").detail("WriteRange", wCR.toString()).detail("ReadRange", rCR.toString());
+							return result;
+						})) {
+							TraceEvent(SevError, "TestFailure").detail("Reason", "No conflicts but should be");
+							break;
+						}
 					}
 				}
-				++self->retries;
+			} catch (Error& e) {
+				state Error e2 = e;
+				wait(tr.onError(e2));
+				wait(tr2.onError(e2));
 			}
 			readConflictRanges.clear();
+			writeConflictRanges.clear();
 			tr.reset();
+			tr2.reset();
 		}
 	}
 };
diff --git a/tests/fast/ReportConflictingKeys.txt b/tests/fast/ReportConflictingKeys.txt
index 1010e2493a..36599059c7 100644
--- a/tests/fast/ReportConflictingKeys.txt
+++ b/tests/fast/ReportConflictingKeys.txt
@@ -1,8 +1,10 @@
 testTitle=ReportConflictingKeysTest
     testName=ReportConflictingKeys
-    testDuration=10.0
+    testDuration=20.0
     nodeCount=10000
     keyPrefix=RCK
     keyBytes=64
-    readConflictRangeCountPerTx=1
-    writeConflictRangeCountPerTx=1
\ No newline at end of file
+    readConflictRangeCountPerTx=10
+    writeConflictRangeCountPerTx=10
+    connectionFailuresDisableDuration=100000
+    buggify=off
\ No newline at end of file

From 8ee4fea3d36eb2d8af8566577ea372975d16f793 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 12:54:12 -0700
Subject: [PATCH 40/45] clang-format

---
 .../workloads/ReportConflictingKeys.actor.cpp | 67 ++++++++++---------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 9d38b0a16a..f2ee0aaf2b 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -49,7 +49,8 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		readConflictRangeCount = getOption(options, LiteralStringRef("readConflictRangeCountPerTx"), 1);
 		writeConflictRangeCount = getOption(options, LiteralStringRef("writeConflictRangeCountPerTx"), 1);
 		ASSERT(readConflictRangeCount >= 1 && writeConflictRangeCount >= 1);
-		// modeled by geometric distribution: (1 - prob) / prob = mean - 1, since we add at least one conflictRange to each tx
+		// modeled by geometric distribution: (1 - prob) / prob = mean - 1, since we add at least one conflictRange to
+		// each tx
 		addReadConflictRangeProb = (readConflictRangeCount - 1.0) / readConflictRangeCount;
 		addWriteConflictRangeProb = (writeConflictRangeCount - 1.0) / writeConflictRangeCount;
 		ASSERT(keyPrefix.size() + 16 <= keyBytes); // make sure the string format is valid
@@ -63,8 +64,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	Future<Void> start(const Database& cx) override { return _start(cx->clone(), this); }
 
 	ACTOR Future<Void> _start(Database cx, ReportConflictingKeysWorkload* self) {
-		if (self->clientId == 0)
-			wait(timeout(self->conflictingClient(cx, self), self->testDuration, Void()));
+		if (self->clientId == 0) wait(timeout(self->conflictingClient(cx, self), self->testDuration, Void()));
 		return Void();
 	}
 
@@ -90,8 +90,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		double p = (double)n / nodeCount;
 		int paddingLen = keyBytes - 16 - keyPrefix.size();
 		// left padding by zero
-		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p))
-		    .withPrefix(keyPrefix);
+		return StringRef(format("%0*llx", paddingLen, *(uint64_t*)&p)).withPrefix(keyPrefix);
 	}
 
 	void addRandomReadConflictRange(ReadYourWritesTransaction* tr, std::vector<KeyRange>* readConflictRanges) {
@@ -99,12 +98,11 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		Key startKey, endKey;
 		do { // add at least one
 			startIdx = deterministicRandom()->randomInt(0, nodeCount);
-			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount+1);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount + 1);
 			startKey = keyForIndex(startIdx);
 			endKey = keyForIndex(endIdx);
 			tr->addReadConflictRange(KeyRangeRef(startKey, endKey));
-			if (readConflictRanges)
-				readConflictRanges->push_back(KeyRangeRef(startKey, endKey));
+			if (readConflictRanges) readConflictRanges->push_back(KeyRangeRef(startKey, endKey));
 		} while (deterministicRandom()->random01() < addReadConflictRangeProb);
 	}
 
@@ -113,12 +111,11 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		Key startKey, endKey;
 		do { // add at least one
 			startIdx = deterministicRandom()->randomInt(0, nodeCount);
-			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount+1);
+			endIdx = deterministicRandom()->randomInt(startIdx, nodeCount + 1);
 			startKey = keyForIndex(startIdx);
 			endKey = keyForIndex(endIdx);
 			tr->addWriteConflictRange(KeyRangeRef(startKey, endKey));
-			if (writeConflictRanges)
-				writeConflictRanges->push_back(KeyRangeRef(startKey, endKey));
+			if (writeConflictRanges) writeConflictRanges->push_back(KeyRangeRef(startKey, endKey));
 		} while (deterministicRandom()->random01() < addWriteConflictRangeProb);
 	}
 
@@ -138,7 +135,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 					tr.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
 				if (deterministicRandom()->random01() < 0.5)
 					tr2.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
-				Version readVersion = wait( tr.getReadVersion() );
+				Version readVersion = wait(tr.getReadVersion());
 				tr2.setVersion(readVersion);
 				self->addRandomReadConflictRange(&tr, nullptr);
 				self->addRandomWriteConflictRange(&tr, &writeConflictRanges);
@@ -154,16 +151,16 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 					wait(tr2.commit());
 					++self->xacts;
 				} catch (Error& e) {
-					if( e.code() != error_code_not_committed )
-						throw e;
+					if (e.code() != error_code_not_committed) throw e;
 					foundConflict = true;
 					++self->conflicts;
 				}
 				// check API correctness
 				if (foundConflict) {
-					// \xff\xff/transaction/conflicting_keys is always false, we skip it here for simplicity
-					state KeyRange ckr = KeyRangeRef(keyAfter(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix)),
-					                                 LiteralStringRef("\xff\xff").withPrefix(conflictingKeysAbsolutePrefix));
+					// \xff\xff/transaction/conflicting_keys is always initialized to false, skip it here
+					state KeyRange ckr =
+					    KeyRangeRef(keyAfter(LiteralStringRef("").withPrefix(conflictingKeysAbsolutePrefix)),
+					                LiteralStringRef("\xff\xff").withPrefix(conflictingKeysAbsolutePrefix));
 					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens
 					// locally Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture =
@@ -188,26 +185,34 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 							    return kr.contains(rCR);
 						    })) {
 							++self->invalidReports;
-							TraceEvent(SevError, "TestFailure").detail("Reason", "InvalidKeyRangeReturned");
-						}
-						else if (!std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(), [&kr](KeyRange wCR) {
-							    // Returned key range should be conflicting with at least one writeConflictRange
-							    return kr.intersects(wCR);
-						    })) {
+							TraceEvent(SevError, "TestFailure")
+							    .detail(
+							        "Reason",
+							        "Returned conflicting keys are not original readConflictRanges or union of them");
+						} else if (!std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(),
+						                        [&kr](KeyRange wCR) {
+							                        // Returned key range should be conflicting with at least one
+							                        // writeConflictRange
+							                        return kr.intersects(wCR);
+						                        })) {
 							++self->invalidReports;
-							TraceEvent(SevError, "TestFailure").detail("Reason", "Returned keyranges are not conflicting with any write ranges");
+							TraceEvent(SevError, "TestFailure")
+							    .detail("Reason", "Returned keyranges are not conflicting with any write ranges");
 						}
 					}
 				} else {
 					// make sure no conflicts between readConflictRange and writeConflictRange
 					for (const KeyRange& rCR : readConflictRanges) {
-						if (std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(), [&rCR](KeyRange wCR){
-							bool result = wCR.intersects(rCR);
-							if (result)
-								TraceEvent(SevError, "TestFailure").detail("WriteRange", wCR.toString()).detail("ReadRange", rCR.toString());
-							return result;
-						})) {
-							TraceEvent(SevError, "TestFailure").detail("Reason", "No conflicts but should be");
+						if (std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(), [&rCR](KeyRange wCR) {
+							    bool result = wCR.intersects(rCR);
+							    if (result)
+								    TraceEvent(SevError, "TestFailure")
+								        .detail("WriteConflictRange", wCR.toString())
+								        .detail("ReadConflictRange", rCR.toString());
+							    return result;
+						    })) {
+							++self->invalidReports;
+							TraceEvent(SevError, "TestFailure").detail("Reason", "No conflicts returned but it should");
 							break;
 						}
 					}

From c4c38c5eca7a45ba27baf74fe2db11b658b30ce9 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 12:58:12 -0700
Subject: [PATCH 41/45] Delete commented code

---
 fdbclient/NativeAPI.actor.cpp | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 13c3825bef..2ac668ffcc 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -2746,7 +2746,8 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 					// clear the RYW transaction which contains previous conflicting keys
 					tr->info.conflictingKeysRYW.reset();
 					if (ci.conflictingKRIndices.present()){
-						// In general, if we want to use getRange to expose conflicting keys, we need to support all the parameters getRange provides.
+						// In general, if we want to use getRange to expose conflicting keys,
+						// we need to support all the parameters getRange provides.
 						// It is difficult to take care of all corner cases of what getRange does.
 						// Consequently, we use a hack way here to achieve it.
 						// We create an empty RYWTransaction and write all conflicting key/values to it.
@@ -2761,16 +2762,12 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						// Clear the whole key space, thus, RYWTr knows to only read keys locally
 						tr->info.conflictingKeysRYW->clear(normalKeys);
 						// initialize value
-						// wait(krmSetRange(hackTr, conflictingKeysPrefix, normalKeys, conflictingKeysFalse));
 						tr->info.conflictingKeysRYW->set(conflictingKeysPrefix, conflictingKeysFalse);
 						// drop duplicate indices and merge overlapped ranges
 						// Note: addReadConflictRange in native transaction object does not merge overlapped ranges
 						state std::set<int> mergedIds(conflictingKRIndices.begin(), conflictingKRIndices.end());
 						for (auto const & rCRIndex : mergedIds) {
 							const KeyRange kr = req.transaction.read_conflict_ranges[rCRIndex];
-							// tr->info.conflictingKeysRYW->set(kr.begin, conflictingKeysTrue);
-							// tr->info.conflictingKeysRYW->set(kr.end, conflictingKeysFalse);
-							// wait(krmSetRange(hackTr, conflictingKeysPrefix, kr, conflictingKeysTrue));
 							wait(krmSetRangeCoalescing(hackTr, conflictingKeysPrefix, kr, allKeys, conflictingKeysTrue));
 						}
 						hackTr.extractPtr(); // Avoid the Reference to destroy the RYW object

From a3b0dce3cd577a1ed90bbc34986f131200df6fef Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 13:12:22 -0700
Subject: [PATCH 42/45] Rename vars, update comments

---
 .../workloads/ReportConflictingKeys.actor.cpp | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index f2ee0aaf2b..b7ed1db500 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -26,6 +26,7 @@
 #include "fdbserver/workloads/BulkSetup.actor.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
+//For this test to report properly buggify must be disabled (flow.h) , and failConnection must be disabled in (sim2.actor.cpp)
 struct ReportConflictingKeysWorkload : TestWorkload {
 
 	double testDuration, transactionsPerSecond, addReadConflictRangeProb, addWriteConflictRangeProb;
@@ -49,7 +50,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		readConflictRangeCount = getOption(options, LiteralStringRef("readConflictRangeCountPerTx"), 1);
 		writeConflictRangeCount = getOption(options, LiteralStringRef("writeConflictRangeCountPerTx"), 1);
 		ASSERT(readConflictRangeCount >= 1 && writeConflictRangeCount >= 1);
-		// modeled by geometric distribution: (1 - prob) / prob = mean - 1, since we add at least one conflictRange to
+		// modeled by geometric distribution: (1 - prob) / prob = mean - 1, where we add at least one conflictRange to
 		// each tx
 		addReadConflictRangeProb = (readConflictRangeCount - 1.0) / readConflictRangeCount;
 		addWriteConflictRangeProb = (writeConflictRangeCount - 1.0) / writeConflictRangeCount;
@@ -121,7 +122,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 
 	ACTOR Future<Void> conflictingClient(Database cx, ReportConflictingKeysWorkload* self) {
 
-		state ReadYourWritesTransaction tr(cx);
+		state ReadYourWritesTransaction tr1(cx);
 		state ReadYourWritesTransaction tr2(cx);
 		state std::vector<KeyRange> readConflictRanges;
 		state std::vector<KeyRange> writeConflictRanges;
@@ -132,15 +133,18 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				// If READ_YOUR_WRITES_DISABLE set, it behaves like native transaction object
 				// where overlapped conflict ranges are not merged.
 				if (deterministicRandom()->random01() < 0.5)
-					tr.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
+					tr1.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
 				if (deterministicRandom()->random01() < 0.5)
 					tr2.setOption(FDBTransactionOptions::READ_YOUR_WRITES_DISABLE);
-				Version readVersion = wait(tr.getReadVersion());
+				// We have the two tx with same grv, then commit the first
+				// If the second one is not able to commit due to conflicts, verify the returned conflicting keys
+				// Otherwise, there is no conflicts between tr1's writeConflictRange and tr2's readConflictRange
+				Version readVersion = wait(tr1.getReadVersion());
 				tr2.setVersion(readVersion);
-				self->addRandomReadConflictRange(&tr, nullptr);
-				self->addRandomWriteConflictRange(&tr, &writeConflictRanges);
+				self->addRandomReadConflictRange(&tr1, nullptr);
+				self->addRandomWriteConflictRange(&tr1, &writeConflictRanges);
 				++self->commits;
-				wait(tr.commit());
+				wait(tr1.commit());
 				++self->xacts;
 
 				state bool foundConflict = false;
@@ -201,7 +205,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 						}
 					}
 				} else {
-					// make sure no conflicts between readConflictRange and writeConflictRange
+					// make sure no conflicts between tr2's readConflictRange and tr1's writeConflictRange
 					for (const KeyRange& rCR : readConflictRanges) {
 						if (std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(), [&rCR](KeyRange wCR) {
 							    bool result = wCR.intersects(rCR);
@@ -219,12 +223,12 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 				}
 			} catch (Error& e) {
 				state Error e2 = e;
-				wait(tr.onError(e2));
+				wait(tr1.onError(e2));
 				wait(tr2.onError(e2));
 			}
 			readConflictRanges.clear();
 			writeConflictRanges.clear();
-			tr.reset();
+			tr1.reset();
 			tr2.reset();
 		}
 	}

From c246f79d726ebb720309a8bcee3c5400233ca2bd Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 13:18:18 -0700
Subject: [PATCH 43/45] Update comments

---
 fdbserver/workloads/ReportConflictingKeys.actor.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index b7ed1db500..121b97c403 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -26,7 +26,8 @@
 #include "fdbserver/workloads/BulkSetup.actor.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
-//For this test to report properly buggify must be disabled (flow.h) , and failConnection must be disabled in (sim2.actor.cpp)
+// For this test to report properly buggify must be disabled (flow.h) , and failConnection must be disabled in
+// (sim2.actor.cpp)
 struct ReportConflictingKeysWorkload : TestWorkload {
 
 	double testDuration, transactionsPerSecond, addReadConflictRangeProb, addWriteConflictRangeProb;
@@ -184,15 +185,14 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 						if (!std::any_of(readConflictRanges.begin(), readConflictRanges.end(), [&kr](KeyRange rCR) {
 							    // Read_conflict_range remains same in the resolver.
 							    // Thus, the returned keyrange is either the original read_conflict_range or merged
-							    // by several overlapped ones In either case, it contains at least one original
+							    // by several overlapped ones in either cases, it contains at least one original
 							    // read_conflict_range
 							    return kr.contains(rCR);
 						    })) {
 							++self->invalidReports;
 							TraceEvent(SevError, "TestFailure")
-							    .detail(
-							        "Reason",
-							        "Returned conflicting keys are not original readConflictRanges or union of them");
+							    .detail("Reason",
+							            "Returned conflicting keys are not original or merged readConflictRanges");
 						} else if (!std::any_of(writeConflictRanges.begin(), writeConflictRanges.end(),
 						                        [&kr](KeyRange wCR) {
 							                        // Returned key range should be conflicting with at least one
@@ -201,7 +201,7 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 						                        })) {
 							++self->invalidReports;
 							TraceEvent(SevError, "TestFailure")
-							    .detail("Reason", "Returned keyranges are not conflicting with any write ranges");
+							    .detail("Reason", "Returned keyrange is not conflicting with any writeConflictRange");
 						}
 					}
 				} else {

From 39a37531db7bec7311d9ea181fbfe1ac2740464b Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 15:42:15 -0700
Subject: [PATCH 44/45] Fix issues according to Andrew's comments

---
 fdbclient/KeyRangeMap.actor.cpp               | 76 +++----------------
 fdbclient/NativeAPI.actor.cpp                 | 34 +++++----
 .../workloads/ReportConflictingKeys.actor.cpp | 12 +--
 3 files changed, 35 insertions(+), 87 deletions(-)

diff --git a/fdbclient/KeyRangeMap.actor.cpp b/fdbclient/KeyRangeMap.actor.cpp
index 5dc785b2cf..98ef568cd7 100644
--- a/fdbclient/KeyRangeMap.actor.cpp
+++ b/fdbclient/KeyRangeMap.actor.cpp
@@ -150,7 +150,8 @@ ACTOR Future<Void> krmSetRange( Reference<ReadYourWritesTransaction> tr, Key map
 //Sets a range of keys in a key range map, coalescing with adjacent regions if the values match
 //Ranges outside of maxRange will not be coalesced
 //CAUTION: use care when attempting to coalesce multiple ranges in the same prefix in a single transaction
-ACTOR Future<Void> krmSetRangeCoalescing( Transaction *tr, Key mapPrefix, KeyRange range, KeyRange maxRange, Value value ) {
+ACTOR template <class Transaction>
+static Future<Void> krmSetRangeCoalescing_( Transaction *tr, Key mapPrefix, KeyRange range, KeyRange maxRange, Value value ) {
 	ASSERT(maxRange.contains(range));
 
 	state KeyRange withPrefix = KeyRangeRef( mapPrefix.toString() + range.begin.toString(), mapPrefix.toString() + range.end.toString() );
@@ -216,70 +217,11 @@ ACTOR Future<Void> krmSetRangeCoalescing( Transaction *tr, Key mapPrefix, KeyRan
 
 	return Void();
 }
-
-ACTOR Future<Void> krmSetRangeCoalescing( Reference<ReadYourWritesTransaction> tr, Key mapPrefix, KeyRange range, KeyRange maxRange, Value value ) {
-	ASSERT(maxRange.contains(range));
-
-	state KeyRange withPrefix = KeyRangeRef( mapPrefix.toString() + range.begin.toString(), mapPrefix.toString() + range.end.toString() );
-	state KeyRange maxWithPrefix = KeyRangeRef( mapPrefix.toString() + maxRange.begin.toString(), mapPrefix.toString() + maxRange.end.toString() );
-
-	state vector<Future<Standalone<RangeResultRef>>> keys;
-	keys.push_back(tr->getRange(lastLessThan(withPrefix.begin), firstGreaterOrEqual(withPrefix.begin), 1, true));
-	keys.push_back(tr->getRange(lastLessOrEqual(withPrefix.end), firstGreaterThan(withPrefix.end) + 1, 2, true));
-	wait(waitForAll(keys));
-
-	//Determine how far to extend this range at the beginning
-	auto beginRange = keys[0].get();
-	bool hasBegin = beginRange.size() > 0 && beginRange[0].key.startsWith(mapPrefix);
-	Value beginValue = hasBegin ? beginRange[0].value : LiteralStringRef("");
-
-	state Key beginKey = withPrefix.begin;
-	if(beginValue == value) {
-		bool outsideRange = !hasBegin || beginRange[0].key < maxWithPrefix.begin;
-		beginKey = outsideRange ? maxWithPrefix.begin : beginRange[0].key;
-	}
-
-	//Determine how far to extend this range at the end
-	auto endRange = keys[1].get();
-	bool hasEnd = endRange.size() >= 1 && endRange[0].key.startsWith(mapPrefix) && endRange[0].key <= withPrefix.end;
-	bool hasNext = (endRange.size() == 2 && endRange[1].key.startsWith(mapPrefix)) || (endRange.size() == 1 && withPrefix.end < endRange[0].key && endRange[0].key.startsWith(mapPrefix));
-	Value existingValue = hasEnd ? endRange[0].value : LiteralStringRef("");
-	bool valueMatches = value == existingValue;
-
-	KeyRange conflictRange = KeyRangeRef( hasBegin ? beginRange[0].key : mapPrefix, withPrefix.begin );
-	if( !conflictRange.empty() )
-		tr->addReadConflictRange( conflictRange );
-
-	conflictRange = KeyRangeRef( hasEnd ? endRange[0].key : mapPrefix, hasNext ? keyAfter(endRange.end()[-1].key) : strinc( mapPrefix ) );
-	if( !conflictRange.empty() )
-		tr->addReadConflictRange( conflictRange );
-
-	state Key endKey;
-	state Value endValue;
-
-	//Case 1: Coalesce completely with the following range
-	if(hasNext && endRange.end()[-1].key <= maxWithPrefix.end && valueMatches) {
-		endKey = endRange.end()[-1].key;
-		endValue = endRange.end()[-1].value;
-	}
-
-	//Case 2: Coalesce with the following range only up to the end of maxRange
-	else if(valueMatches) {
-		endKey = maxWithPrefix.end;
-		endValue = existingValue;
-	}
-
-	//Case 3: Don't coalesce
-	else {
-		endKey = withPrefix.end;
-		endValue = existingValue;
-	}
-
-	tr->clear(KeyRangeRef(beginKey, endKey));
-
-	ASSERT(value != endValue || endKey == maxWithPrefix.end);
-	tr->set(beginKey, value);
-	tr->set(endKey, endValue);
-
-	return Void();
+Future<Void> krmSetRangeCoalescing(Transaction* const& tr, Key const& mapPrefix, KeyRange const& range,
+                                   KeyRange const& maxRange, Value const& value) {
+   return krmSetRangeCoalescing_(tr, mapPrefix, range, maxRange, value);
+}
+Future<Void> krmSetRangeCoalescing(Reference<ReadYourWritesTransaction> const& tr, Key const& mapPrefix,
+                                   KeyRange const& range, KeyRange const& maxRange, Value const& value) {
+   return holdWhile(tr, krmSetRangeCoalescing_(tr.getPtr(), mapPrefix, range, maxRange, value));
 }
diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp
index 2ac668ffcc..9a9026be8c 100644
--- a/fdbclient/NativeAPI.actor.cpp
+++ b/fdbclient/NativeAPI.actor.cpp
@@ -21,6 +21,7 @@
 #include "fdbclient/NativeAPI.actor.h"
 
 #include <iterator>
+#include <unordered_set>
 
 #include "fdbclient/Atomic.h"
 #include "fdbclient/ClusterInterface.h"
@@ -2755,20 +2756,25 @@ ACTOR static Future<Void> tryCommit( Database cx, Reference<TransactionLogInfo>
 						tr->info.conflictingKeysRYW = std::make_shared<ReadYourWritesTransaction>(tr->getDatabase());
 						state Reference<ReadYourWritesTransaction> hackTr =
 							Reference<ReadYourWritesTransaction>(tr->info.conflictingKeysRYW.get());
-						state Standalone<VectorRef<int>> conflictingKRIndices = ci.conflictingKRIndices.get();
-						// To make the getRange call local, we need to explicitly set the read version here.
-						// This version number 100 set here does nothing but prevent getting read version from the proxy
-						tr->info.conflictingKeysRYW->setVersion(100);
-						// Clear the whole key space, thus, RYWTr knows to only read keys locally
-						tr->info.conflictingKeysRYW->clear(normalKeys);
-						// initialize value
-						tr->info.conflictingKeysRYW->set(conflictingKeysPrefix, conflictingKeysFalse);
-						// drop duplicate indices and merge overlapped ranges
-						// Note: addReadConflictRange in native transaction object does not merge overlapped ranges
-						state std::set<int> mergedIds(conflictingKRIndices.begin(), conflictingKRIndices.end());
-						for (auto const & rCRIndex : mergedIds) {
-							const KeyRange kr = req.transaction.read_conflict_ranges[rCRIndex];
-							wait(krmSetRangeCoalescing(hackTr, conflictingKeysPrefix, kr, allKeys, conflictingKeysTrue));
+						try {
+							state Standalone<VectorRef<int>> conflictingKRIndices = ci.conflictingKRIndices.get();
+							// To make the getRange call local, we need to explicitly set the read version here.
+							// This version number 100 set here does nothing but prevent getting read version from the proxy
+							tr->info.conflictingKeysRYW->setVersion(100);
+							// Clear the whole key space, thus, RYWTr knows to only read keys locally
+							tr->info.conflictingKeysRYW->clear(normalKeys);
+							// initialize value
+							tr->info.conflictingKeysRYW->set(conflictingKeysPrefix, conflictingKeysFalse);
+							// drop duplicate indices and merge overlapped ranges
+							// Note: addReadConflictRange in native transaction object does not merge overlapped ranges
+							state std::unordered_set<int> mergedIds(conflictingKRIndices.begin(), conflictingKRIndices.end());
+							for (auto const & rCRIndex : mergedIds) {
+								const KeyRange kr = req.transaction.read_conflict_ranges[rCRIndex];
+								wait(krmSetRangeCoalescing(hackTr, conflictingKeysPrefix, kr, allKeys, conflictingKeysTrue));
+							}
+						} catch (Error& e) {
+							hackTr.extractPtr(); // Make sure the RYW is not freed twice in case exception thrown
+							throw;
 						}
 						hackTr.extractPtr(); // Avoid the Reference to destroy the RYW object
 					}
diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 121b97c403..4955017342 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -35,10 +35,10 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 
 	int nodeCount, actorCount, keyBytes, valueBytes, readConflictRangeCount, writeConflictRangeCount;
 
-	PerfIntCounter invalidReports, commits, conflicts, retries, xacts;
+	PerfIntCounter invalidReports, commits, conflicts, xacts;
 
 	ReportConflictingKeysWorkload(WorkloadContext const& wcx)
-	  : TestWorkload(wcx), invalidReports("InvalidReports"), conflicts("Conflicts"), retries("Retries"),
+	  : TestWorkload(wcx), invalidReports("InvalidReports"), conflicts("Conflicts"),
 	    commits("Commits"), xacts("Transactions") {
 		testDuration = getOption(options, LiteralStringRef("testDuration"), 10.0);
 		// transactionsPerSecond = getOption(options, LiteralStringRef("transactionsPerSecond"), 5000.0) / clientCount;
@@ -80,8 +80,6 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 		m.push_back(PerfMetric("Commits/sec", commits.getValue() / testDuration, true));
 		m.push_back(conflicts.getMetric());
 		m.push_back(PerfMetric("Conflicts/sec", conflicts.getValue() / testDuration, true));
-		m.push_back(retries.getMetric());
-		m.push_back(PerfMetric("Retries/sec", retries.getValue() / testDuration, true));
 	}
 
 	// disable the default timeout setting
@@ -169,10 +167,12 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 					// The getRange here using the special key prefix "\xff\xff/transaction/conflicting_keys/" happens
 					// locally Thus, the error handling is not needed here
 					Future<Standalone<RangeResultRef>> conflictingKeyRangesFuture =
-					    tr2.getRange(ckr, readConflictRanges.size() * 2);
+					    tr2.getRange(ckr, CLIENT_KNOBS->TOO_MANY);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
-					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() % 2 == 0));
+					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() <= readConflictRanges.size() * 2));
+					ASSERT(conflictingKeyRanges.size() % 2 == 0);
+					ASSERT(!conflictingKeyRanges.more);
 					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {
 						KeyValueRef startKeyWithPrefix = conflictingKeyRanges[i];
 						ASSERT(startKeyWithPrefix.value == conflictingKeysTrue);

From 9dc441c65ab0757b2a3dae41809b2efd9c9305f6 Mon Sep 17 00:00:00 2001
From: chaoguang <13974480+zjuLcg@users.noreply.github.com>
Date: Fri, 13 Mar 2020 15:43:01 -0700
Subject: [PATCH 45/45] clang-format

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

diff --git a/fdbserver/workloads/ReportConflictingKeys.actor.cpp b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
index 4955017342..a51a80edab 100644
--- a/fdbserver/workloads/ReportConflictingKeys.actor.cpp
+++ b/fdbserver/workloads/ReportConflictingKeys.actor.cpp
@@ -38,8 +38,8 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 	PerfIntCounter invalidReports, commits, conflicts, xacts;
 
 	ReportConflictingKeysWorkload(WorkloadContext const& wcx)
-	  : TestWorkload(wcx), invalidReports("InvalidReports"), conflicts("Conflicts"),
-	    commits("Commits"), xacts("Transactions") {
+	  : TestWorkload(wcx), invalidReports("InvalidReports"), conflicts("Conflicts"), commits("Commits"),
+	    xacts("Transactions") {
 		testDuration = getOption(options, LiteralStringRef("testDuration"), 10.0);
 		// transactionsPerSecond = getOption(options, LiteralStringRef("transactionsPerSecond"), 5000.0) / clientCount;
 		actorCount = getOption(options, LiteralStringRef("actorsPerClient"), 1);
@@ -170,7 +170,8 @@ struct ReportConflictingKeysWorkload : TestWorkload {
 					    tr2.getRange(ckr, CLIENT_KNOBS->TOO_MANY);
 					ASSERT(conflictingKeyRangesFuture.isReady());
 					const Standalone<RangeResultRef> conflictingKeyRanges = conflictingKeyRangesFuture.get();
-					ASSERT(conflictingKeyRanges.size() && (conflictingKeyRanges.size() <= readConflictRanges.size() * 2));
+					ASSERT(conflictingKeyRanges.size() &&
+					       (conflictingKeyRanges.size() <= readConflictRanges.size() * 2));
 					ASSERT(conflictingKeyRanges.size() % 2 == 0);
 					ASSERT(!conflictingKeyRanges.more);
 					for (int i = 0; i < conflictingKeyRanges.size(); i += 2) {