handle incomplete read errors (#9021)

arangod/RocksDBEngine/RocksDBCommon.cpp

@@ -24,6 +24,7 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "RocksDBCommon.h"
+#include "Basics/Exceptions.h"
 #include "Basics/RocksDBUtils.h"
 #include "Logger/Logger.h"
 #include "RocksDBEngine/RocksDBColumnFamily.h"
@@ -85,6 +86,13 @@ uint64_t latestSequenceNumber() {
   return static_cast<uint64_t>(seq);
 }
 
+void checkIteratorStatus(rocksdb::Iterator const* iterator) {
+  auto s = iterator->status();
+  if (!s.ok()) {
+    THROW_ARANGO_EXCEPTION(arangodb::rocksutils::convertStatus(s));
+  }
+}
+
 std::pair<TRI_voc_tick_t, TRI_voc_cid_t> mapObjectToCollection(uint64_t objectId) {
   return globalRocksEngine()->mapObjectToCollection(objectId);
 }

arangod/RocksDBEngine/RocksDBCommon.h

@@ -42,11 +42,12 @@
 #include <rocksdb/utilities/transaction_db.h>
 
 namespace rocksdb {
-class TransactionDB;
-class DB;
-struct ReadOptions;
 class Comparator;
 class ColumnFamilyHandle;
+class DB;
+class Iterator;
+struct ReadOptions;
+class TransactionDB;
 }  // namespace rocksdb
 
 namespace arangodb {
@@ -70,6 +71,12 @@ arangodb::Result globalRocksDBRemove(rocksdb::ColumnFamilyHandle* cf,
 
 uint64_t latestSequenceNumber();
 
+/// @brief throws an exception of appropriate type if the iterator's status is !ok().
+/// does nothing if the iterator's status is ok().
+/// this function can be used by IndexIterators to verify that an iterator is still
+/// in good shape
+void checkIteratorStatus(rocksdb::Iterator const* iterator);
+
 std::pair<TRI_voc_tick_t, TRI_voc_cid_t> mapObjectToCollection(uint64_t);
 RocksDBEngine::IndexTriple mapObjectToIndex(uint64_t);
 
@@ -87,7 +94,7 @@ Result removeLargeRange(rocksdb::DB* db, RocksDBKeyBounds const& bounds,
 // optional switch to std::function to reduce amount of includes and
 // to avoid template
 // this helper is not meant for transactional usage!
-template <typename T>  // T is a invokeable that takes a rocksdb::Iterator*
+template <typename T>  // T is an invokeable that takes a rocksdb::Iterator*
 void iterateBounds(RocksDBKeyBounds const& bounds, T callback,
                    rocksdb::ReadOptions options = rocksdb::ReadOptions()) {
   rocksdb::Slice const end = bounds.end();

arangod/RocksDBEngine/RocksDBEdgeIndex.cpp

@@ -325,6 +325,9 @@ class RocksDBEdgeIndexLookupIterator final : public IndexIterator {
       _builder.add(VPackValuePair(vertexId.data(), vertexId.size(), VPackValueType::String));
     }
     _builder.close();
+
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
     
     if (cc != nullptr) {
       // TODO Add cache retry on next call

arangod/RocksDBEngine/RocksDBGeoIndex.cpp

@@ -202,6 +202,9 @@ class RDBNearIterator final : public IndexIterator {
         _near.reportFound(documentId, RocksDBValue::centroid(_iter->value()));
         _iter->Next();
       }
+    
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iter.get());
     }
 
     _near.didScanIntervals();  // calculate next bounds

arangod/RocksDBEngine/RocksDBIterators.cpp

@@ -25,6 +25,7 @@
 #include "Random/RandomGenerator.h"
 #include "RocksDBEngine/RocksDBCollection.h"
 #include "RocksDBEngine/RocksDBColumnFamily.h"
+#include "RocksDBEngine/RocksDBCommon.h"
 #include "RocksDBEngine/RocksDBMethods.h"
 #include "RocksDBEngine/RocksDBPrimaryIndex.h"
 #include "RocksDBEngine/RocksDBTransactionState.h"
@@ -80,6 +81,8 @@ bool RocksDBAllIndexIterator::next(LocalDocumentIdCallback const& cb, size_t lim
     // No limit no data, or we are actually done. The last call should have
     // returned false
     TRI_ASSERT(limit > 0);  // Someone called with limit == 0. Api broken
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
     return false;
   }
 
@@ -93,6 +96,8 @@ bool RocksDBAllIndexIterator::next(LocalDocumentIdCallback const& cb, size_t lim
     _iterator->Next();
 
     if (!_iterator->Valid() || outOfRange()) {
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
       return false;
     }
   }
@@ -108,6 +113,8 @@ bool RocksDBAllIndexIterator::nextDocument(IndexIterator::DocumentCallback const
     // No limit no data, or we are actually done. The last call should have
     // returned false
     TRI_ASSERT(limit > 0);  // Someone called with limit == 0. Api broken
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
     return false;
   }
 
@@ -117,6 +124,8 @@ bool RocksDBAllIndexIterator::nextDocument(IndexIterator::DocumentCallback const
     _iterator->Next();
 
     if (!_iterator->Valid() || outOfRange()) {
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
       return false;
     }
   }
@@ -133,6 +142,9 @@ void RocksDBAllIndexIterator::skip(uint64_t count, uint64_t& skipped) {
 
     _iterator->Next();
   }
+    
+  // validate that Iterator is in a good shape and hasn't failed
+  arangodb::rocksutils::checkIteratorStatus(_iterator.get());
 }
 
 void RocksDBAllIndexIterator::reset() {
@@ -189,6 +201,8 @@ bool RocksDBAnyIndexIterator::next(LocalDocumentIdCallback const& cb, size_t lim
     // No limit no data, or we are actually done. The last call should have
     // returned false
     TRI_ASSERT(limit > 0);  // Someone called with limit == 0. Api broken
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
     return false;
   }
 
@@ -198,6 +212,9 @@ bool RocksDBAnyIndexIterator::next(LocalDocumentIdCallback const& cb, size_t lim
     _returned++;
     _iterator->Next();
     if (!_iterator->Valid() || outOfRange()) {
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
+
       if (_returned < _total) {
         _iterator->Seek(_bounds.start());
         continue;
@@ -216,6 +233,8 @@ bool RocksDBAnyIndexIterator::nextDocument(IndexIterator::DocumentCallback const
     // No limit no data, or we are actually done. The last call should have
     // returned false
     TRI_ASSERT(limit > 0);  // Someone called with limit == 0. Api broken
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
     return false;
   }
 
@@ -225,6 +244,8 @@ bool RocksDBAnyIndexIterator::nextDocument(IndexIterator::DocumentCallback const
     _returned++;
     _iterator->Next();
     if (!_iterator->Valid() || outOfRange()) {
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
       if (_returned < _total) {
         _iterator->Seek(_bounds.start());
         continue;
@@ -265,6 +286,9 @@ void RocksDBAnyIndexIterator::reset() {
       }
     }
   }
+    
+  // validate that Iterator is in a good shape and hasn't failed
+  arangodb::rocksutils::checkIteratorStatus(_iterator.get());
 }
 
 bool RocksDBAnyIndexIterator::outOfRange() const {
@@ -333,6 +357,8 @@ bool RocksDBGenericIterator::next(GenericCallback const& cb, size_t limit) {
   if (limit == 0) {
     // No limit no data, or we are actually done. The last call should have
     // returned false
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
     return false;
   }
 
@@ -351,6 +377,9 @@ bool RocksDBGenericIterator::next(GenericCallback const& cb, size_t limit) {
     } else {
       _iterator->Next();
     }
+    
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
   }
 
   return hasMore();

arangod/RocksDBEngine/RocksDBVPackIndex.cpp

@@ -228,6 +228,8 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
       // No limit no data, or we are actually done. The last call should have
       // returned false
       TRI_ASSERT(limit > 0);  // Someone called with limit == 0. Api broken
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
       return false;
     }
 
@@ -239,6 +241,8 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
 
       --limit;
       if (!advance()) {
+        // validate that Iterator is in a good shape and hasn't failed
+        arangodb::rocksutils::checkIteratorStatus(_iterator.get());
         return false;
       }
     }
@@ -253,6 +257,8 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
       // No limit no data, or we are actually done. The last call should have
       // returned false
       TRI_ASSERT(limit > 0);  // Someone called with limit == 0. Api broken
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
       return false;
     }
 
@@ -267,6 +273,8 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
 
       --limit;
       if (!advance()) {
+        // validate that Iterator is in a good shape and hasn't failed
+        arangodb::rocksutils::checkIteratorStatus(_iterator.get());
         return false;
       }
     }
@@ -278,6 +286,8 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
     TRI_ASSERT(_trx->state()->isRunning());
 
     if (!_iterator->Valid() || outOfRange()) {
+      // validate that Iterator is in a good shape and hasn't failed
+      arangodb::rocksutils::checkIteratorStatus(_iterator.get());
       return;
     }
 
@@ -287,6 +297,8 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
       --count;
       ++skipped;
       if (!advance()) {
+        // validate that Iterator is in a good shape and hasn't failed
+        arangodb::rocksutils::checkIteratorStatus(_iterator.get());
         return;
       }
     }
@@ -301,6 +313,9 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
     } else {
       _iterator->Seek(_bounds.start());
     }
+    
+    // validate that Iterator is in a good shape and hasn't failed
+    arangodb::rocksutils::checkIteratorStatus(_iterator.get());
   }
 
   /// @brief we provide a method to provide the index attribute values

js/common/bootstrap/errors.js

@@ -111,6 +111,7 @@
     "ERROR_ARANGO_COLLECTION_TYPE_MISMATCH" : { "code" : 1237, "message" : "collection type mismatch" },
     "ERROR_ARANGO_COLLECTION_NOT_LOADED" : { "code" : 1238, "message" : "collection not loaded" },
     "ERROR_ARANGO_DOCUMENT_REV_BAD" : { "code" : 1239, "message" : "illegal document revision" },
+    "ERROR_ARANGO_INCOMPLETE_READ" : { "code" : 1240, "message" : "incomplete read" },
     "ERROR_ARANGO_DATAFILE_FULL"   : { "code" : 1300, "message" : "datafile full" },
     "ERROR_ARANGO_EMPTY_DATADIR"   : { "code" : 1301, "message" : "server database directory is empty" },
     "ERROR_ARANGO_TRY_AGAIN"       : { "code" : 1302, "message" : "operation should be tried again" },
@@ -366,43 +367,5 @@
 
   // For compatibility with <= 3.3
   internal.errors.ERROR_ARANGO_COLLECTION_NOT_FOUND = internal.errors.ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
-
-  //arg1 can be a code or error object
-  internal.throwArangoError = function (arg1, message, httpCode) {
-    let errorNum;
-
-    if (typeof arg1 === "object" && typeof arg1.code === "number") {
-      errorNum = arg1.code;
-      if(message === undefined && arg1.message) {
-        message = arg1.message;
-      }
-    } else if ( typeof arg1 === "number" ) {
-      errorNum = arg1;
-    } else {
-      errorNum = internal.errors.ERROR_INTERNAL.code;
-    }
-
-    if (message === undefined) {
-      message = "could not resolve errorMessage";
-      for(var key in internal.errors) {
-        let attribute = internal.errors[key];
-        if(attribute.code === errorNum){
-          message = attribute.message;
-          break;
-        }
-      }
-    }
-
-    if (httpCode === undefined) {
-      httpCode = internal.errorNumberToHttpCode(errorNum);
-    }
-
-    throw new internal.ArangoError({
-      errorNum: errorNum,
-      errorMessage: message,
-      code: httpCode
-    });
-  };
-
 }());
 

lib/Basics/RocksDBUtils.cpp

@@ -117,8 +117,8 @@ arangodb::Result convertStatus(rocksdb::Status const& status, StatusHint hint,
     case rocksdb::Status::Code::kMergeInProgress:
       return {TRI_ERROR_ARANGO_MERGE_IN_PROGRESS, std::move(message)};
     case rocksdb::Status::Code::kIncomplete:
-      return {TRI_ERROR_INTERNAL,
+      return {TRI_ERROR_ARANGO_INCOMPLETE_READ,
-              prefix + "'incomplete' error in storage engine" + postfix};
+              prefix + "'incomplete' error in storage engine " + postfix};
     case rocksdb::Status::Code::kShutdownInProgress:
       return {TRI_ERROR_SHUTTING_DOWN, std::move(message)};
     case rocksdb::Status::Code::kTimedOut:
@@ -127,7 +127,7 @@ arangodb::Result convertStatus(rocksdb::Status const& status, StatusHint hint,
       }
       if (status.subcode() == rocksdb::Status::SubCode::kLockTimeout) {
         return {TRI_ERROR_ARANGO_CONFLICT,
-                prefix + "timeout waiting to lock key" + postfix};
+                prefix + "timeout waiting to lock key " + postfix};
       }
       return {TRI_ERROR_LOCK_TIMEOUT, std::move(message)};
     case rocksdb::Status::Code::kAborted:
@@ -139,15 +139,15 @@ arangodb::Result convertStatus(rocksdb::Status const& status, StatusHint hint,
       if (status.subcode() == rocksdb::Status::SubCode::kLockLimit) {
         // should actually not occur with our RocksDB configuration
         return {TRI_ERROR_RESOURCE_LIMIT,
-                prefix + "failed to acquire lock due to lock number limit" + postfix};
+                prefix + "failed to acquire lock due to lock number limit " + postfix};
       }
       return {TRI_ERROR_ARANGO_CONFLICT, "write-write conflict"};
     case rocksdb::Status::Code::kExpired:
-      return {TRI_ERROR_INTERNAL, prefix + "key expired; TTL was set in error" + postfix};
+      return {TRI_ERROR_INTERNAL, prefix + "key expired; TTL was set in error " + postfix};
     case rocksdb::Status::Code::kTryAgain:
       return {TRI_ERROR_ARANGO_TRY_AGAIN, std::move(message)};
     default:
-      return {TRI_ERROR_INTERNAL, prefix + "unknown RocksDB status code" + postfix};
+      return {TRI_ERROR_INTERNAL, prefix + "unknown RocksDB status code " + postfix};
   }
 }
 

lib/Basics/errors.dat

@@ -133,6 +133,7 @@ ERROR_ARANGO_WRITE_THROTTLE_TIMEOUT,1236,"write-throttling timeout","Will be rai
 ERROR_ARANGO_COLLECTION_TYPE_MISMATCH,1237,"collection type mismatch","Will be raised when a collection has a different type from what has been expected."
 ERROR_ARANGO_COLLECTION_NOT_LOADED,1238,"collection not loaded","Will be raised when a collection is accessed that is not yet loaded."
 ERROR_ARANGO_DOCUMENT_REV_BAD,1239,"illegal document revision","Will be raised when a document revision is corrupt or is missing where needed."
+ERROR_ARANGO_INCOMPLETE_READ,1240,"incomplete read","Will be raised by the storage engine when a read cannot be completed."
 
 ################################################################################
 ## Checked ArangoDB storage errors

lib/Basics/voc-errors.cpp

@@ -110,6 +110,7 @@ void TRI_InitializeErrorMessages() {
   REG_ERROR(ERROR_ARANGO_COLLECTION_TYPE_MISMATCH, "collection type mismatch");
   REG_ERROR(ERROR_ARANGO_COLLECTION_NOT_LOADED, "collection not loaded");
   REG_ERROR(ERROR_ARANGO_DOCUMENT_REV_BAD, "illegal document revision");
+  REG_ERROR(ERROR_ARANGO_INCOMPLETE_READ, "incomplete read");
   REG_ERROR(ERROR_ARANGO_DATAFILE_FULL, "datafile full");
   REG_ERROR(ERROR_ARANGO_EMPTY_DATADIR, "server database directory is empty");
   REG_ERROR(ERROR_ARANGO_TRY_AGAIN, "operation should be tried again");

lib/Basics/voc-errors.h

@@ -547,6 +547,11 @@ constexpr int TRI_ERROR_ARANGO_COLLECTION_NOT_LOADED
 /// needed.
 constexpr int TRI_ERROR_ARANGO_DOCUMENT_REV_BAD                                 = 1239;
 
+/// 1240: ERROR_ARANGO_INCOMPLETE_READ
+/// "incomplete read"
+/// Will be raised by the storage engine when a read cannot be completed.
+constexpr int TRI_ERROR_ARANGO_INCOMPLETE_READ                                  = 1240;
+
 /// 1300: ERROR_ARANGO_DATAFILE_FULL
 /// "datafile full"
 /// Will be raised when the datafile reaches its limit.