From 473e2bbf5fe9a79fac4ab6964f86b25fc4dfbbaa Mon Sep 17 00:00:00 2001
From: Daniel Smith <daniel_b_smith@apple.com>
Date: Fri, 20 Aug 2021 12:30:29 -0400
Subject: [PATCH 1/2] Fix error handling of reads.

---
 fdbserver/KeyValueStoreRocksDB.actor.cpp | 36 ++++++++++++------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/fdbserver/KeyValueStoreRocksDB.actor.cpp b/fdbserver/KeyValueStoreRocksDB.actor.cpp
index 6dbddf8c62..edbd6df823 100644
--- a/fdbserver/KeyValueStoreRocksDB.actor.cpp
+++ b/fdbserver/KeyValueStoreRocksDB.actor.cpp
@@ -181,6 +181,14 @@ ACTOR Future<Void> rocksDBMetricLogger(std::shared_ptr<rocksdb::Statistics> stat
 	}
 }
 
+Error statusToError(const rocksdb::Status& s) {
+	if (s == rocksdb::Status::IOError()) {
+		return io_error();
+	} else {
+		return unknown_error();
+	}
+}
+
 struct RocksDBKeyValueStore : IKeyValueStore {
 	using DB = rocksdb::DB*;
 	using CF = rocksdb::ColumnFamilyHandle*;
@@ -199,14 +207,6 @@ struct RocksDBKeyValueStore : IKeyValueStore {
 
 		void init() override {}
 
-		Error statusToError(const rocksdb::Status& s) {
-			if (s == rocksdb::Status::IOError()) {
-				return io_error();
-			} else {
-				return unknown_error();
-			}
-		}
-
 		struct OpenAction : TypedAction<Writer, OpenAction> {
 			std::string path;
 			ThreadReturnPromise<Void> done;
@@ -365,11 +365,11 @@ struct RocksDBKeyValueStore : IKeyValueStore {
 			}
 			if (s.ok()) {
 				a.result.send(Value(toStringRef(value)));
-			} else {
-				if (!s.IsNotFound()) {
-					TraceEvent(SevError, "RocksDBError").detail("Error", s.ToString()).detail("Method", "ReadValue");
-				}
+			} else if (s.IsNotFound()) {
 				a.result.send(Optional<Value>());
+			} else {
+				TraceEvent(SevError, "RocksDBError").detail("Error", s.ToString()).detail("Method", "ReadValue");
+				a.result.sendError(statusToError(s));
 			}
 		}
 
@@ -415,13 +415,11 @@ struct RocksDBKeyValueStore : IKeyValueStore {
 			if (s.ok()) {
 				a.result.send(Value(StringRef(reinterpret_cast<const uint8_t*>(value.data()),
 				                              std::min(value.size(), size_t(a.maxLength)))));
-			} else {
-				if (!s.IsNotFound()) {
-					TraceEvent(SevError, "RocksDBError")
-					    .detail("Error", s.ToString())
-					    .detail("Method", "ReadValuePrefix");
-				}
+			} else if (s.IsNotFound()) {
 				a.result.send(Optional<Value>());
+			} else {
+				TraceEvent(SevError, "RocksDBError").detail("Error", s.ToString()).detail("Method", "ReadValuePrefix");
+				a.result.sendError(statusToError(s));
 			}
 		}
 
@@ -513,6 +511,8 @@ struct RocksDBKeyValueStore : IKeyValueStore {
 
 			if (!s.ok()) {
 				TraceEvent(SevError, "RocksDBError").detail("Error", s.ToString()).detail("Method", "ReadRange");
+				a.result.sendError(statusToError(s));
+				return;
 			}
 			result.more =
 			    (result.size() == a.rowLimit) || (result.size() == -a.rowLimit) || (accumulatedBytes >= a.byteLimit);

From b75fb0d1056ceee0b3674b9b72dfcec1fb86fa75 Mon Sep 17 00:00:00 2001
From: Daniel Smith <daniel_b_smith@apple.com>
Date: Fri, 20 Aug 2021 12:45:26 -0400
Subject: [PATCH 2/2] Improve error to status conversion

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

diff --git a/fdbserver/KeyValueStoreRocksDB.actor.cpp b/fdbserver/KeyValueStoreRocksDB.actor.cpp
index edbd6df823..2840f2be19 100644
--- a/fdbserver/KeyValueStoreRocksDB.actor.cpp
+++ b/fdbserver/KeyValueStoreRocksDB.actor.cpp
@@ -182,8 +182,10 @@ ACTOR Future<Void> rocksDBMetricLogger(std::shared_ptr<rocksdb::Statistics> stat
 }
 
 Error statusToError(const rocksdb::Status& s) {
-	if (s == rocksdb::Status::IOError()) {
+	if (s.IsIOError()) {
 		return io_error();
+	} else if (s.IsTimedOut()) {
+		return transaction_too_old();
 	} else {
 		return unknown_error();
 	}