fixes database.check-version (#5210)

* Fixed undefined behaviour when starting arangodb with --database.check-version true as done by our scripts.

* Implemented the database version check in c++ and removed v8 dependency. Also hard-code role to SingleServer in that case.

arangod/RestServer/CheckVersionFeature.cpp

@@ -22,19 +22,24 @@
 
 #include "CheckVersionFeature.h"
 
+#include "Basics/FileUtils.h"
 #include "Logger/Logger.h"
 #include "Logger/LoggerFeature.h"
 #include "ProgramOptions/ProgramOptions.h"
 #include "ProgramOptions/Section.h"
 #include "Replication/ReplicationFeature.h"
 #include "RestServer/DatabaseFeature.h"
-#include "V8Server/V8Context.h"
+#include "Rest/Version.h"
-#include "V8Server/V8DealerFeature.h"
+#include "StorageEngine/EngineSelectorFeature.h"
-#include "V8Server/v8-query.h"
+#include "StorageEngine/StorageEngine.h"
-#include "V8Server/v8-vocbase.h"
 #include "VocBase/vocbase.h"
 #include "Basics/exitcodes.h"
 
+#include <velocypack/Parser.h>
+#include <velocypack/Builder.h>
+#include <velocypack/Slice.h>
+#include <velocypack/velocypack-aliases.h>
+
 using namespace arangodb;
 using namespace arangodb::application_features;
 using namespace arangodb::basics;
@@ -50,7 +55,9 @@ CheckVersionFeature::CheckVersionFeature(
   setOptional(false);
   requiresElevatedPrivileges(false);
   startsAfter("Database");
-  startsAfter("V8Dealer");
+  startsAfter("StorageEngine");
+  startsAfter("Logger");
+  startsAfter("Replication");
 }
 
 void CheckVersionFeature::collectOptions(
@@ -64,13 +71,70 @@ void CheckVersionFeature::collectOptions(
                            new BooleanParameter(&_checkVersion));
 }
 
+CheckVersionFeature::CheckVersionResult
+CheckVersionFeature::checkVersionFileForDB(TRI_vocbase_t* vocbase,
+                                           StorageEngine* engine,
+                                           uint32_t currentVersion) const {
+  std::string const versionFilePath = engine->versionFilename(vocbase->id());
+  if (!FileUtils::exists(versionFilePath) ||
+      !FileUtils::isRegularFile(versionFilePath)) {
+    // File not existing
+    LOG_TOPIC(DEBUG, arangodb::Logger::FIXME)
+        << "version file (" << versionFilePath << ") not found";
+    return NO_VERSION_FILE;
+  }
+  std::string const versionFileData = FileUtils::slurp(versionFilePath);
+  LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "found version file "
+                                            << versionFileData;
+  std::shared_ptr<VPackBuilder> versionBuilder;
+  try {
+    versionBuilder = VPackParser::fromJson(versionFileData);
+  } catch (...) {
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "Cannot parse VERSION file '"
+                                            << versionFilePath << "': '"
+                                            << versionFileData << "'";
+    return CANNOT_PARSE_VERSION_FILE;
+  }
+  VPackSlice version = versionBuilder->slice();
+  if (!version.isObject() || !version.hasKey("version")) {
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "Cannot parse VERSION file '"
+                                            << versionFilePath << "': '"
+                                            << versionFileData << "'";
+    return CANNOT_PARSE_VERSION_FILE;
+  }
+  version = version.get("version");
+  if (!version.isNumber()) {
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "Cannot parse VERSION file '"
+                                            << versionFilePath << "': '"
+                                            << versionFileData << "'";
+    return CANNOT_PARSE_VERSION_FILE;
+  }
+  uint32_t lastVersion = version.getNumericValue<uint32_t>();
+
+  // Integer division
+  if (lastVersion / 100 == currentVersion / 100) {
+    LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "version match: last version" << lastVersion << ", current version" << currentVersion;
+    return VERSION_MATCH;
+  }
+  if (lastVersion > currentVersion) {
+    LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "downgrade: last version" << lastVersion << ", current version" << currentVersion;
+    return DOWNGRADE_NEEDED;
+  }
+  TRI_ASSERT(lastVersion < currentVersion);
+  LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "upgrade: last version" << lastVersion << ", current version" << currentVersion;
+  return UPGRADE_NEEDED;
+}
+
 void CheckVersionFeature::validateOptions(
     std::shared_ptr<ProgramOptions> options) {
   if (!_checkVersion) {
     return;
   }
 
+  // This disables cluster feature, so the role will not be set
   ApplicationServer::forceDisableFeatures(_nonServerFeatures);
+  // HardCode it to single server
+  ServerState::instance()->setRole(ServerState::ROLE_SINGLE);
 
   LoggerFeature* logger =
       ApplicationServer::getFeature<LoggerFeature>("Logger");
@@ -83,10 +147,6 @@ void CheckVersionFeature::validateOptions(
   DatabaseFeature* databaseFeature =
       ApplicationServer::getFeature<DatabaseFeature>("Database");
   databaseFeature->enableCheckVersion();
-
-  V8DealerFeature* v8dealer =
-      ApplicationServer::getFeature<V8DealerFeature>("V8Dealer");
-  v8dealer->setMaximumContexts(1);
 }
 
 void CheckVersionFeature::start() {
@@ -114,72 +174,40 @@ void CheckVersionFeature::checkVersion() {
   // run version check
   LOG_TOPIC(TRACE, arangodb::Logger::FIXME) << "starting version check";
 
-  auto* vocbase = DatabaseFeature::DATABASE->systemDatabase();
+  DatabaseFeature* databaseFeature = application_features::ApplicationServer::getFeature<DatabaseFeature>("Database");
 
-  // enter context and isolate
+  int32_t currentVersion = arangodb::rest::Version::getNumericServerVersion();
-  {
+  StorageEngine* engine = EngineSelectorFeature::ENGINE;
-    V8Context* context = V8DealerFeature::DEALER->enterContext(vocbase, true, 0);
+  auto checkDBVersion = [&] (TRI_vocbase_t* vocbase) {
-
+    auto res = checkVersionFileForDB(vocbase, engine, currentVersion);
-    if (context == nullptr) {
+    switch (res) {
-      LOG_TOPIC(FATAL, arangodb::Logger::FIXME) << "could not enter context #0";
+      case VERSION_MATCH:
-      FATAL_ERROR_EXIT();
+        // all good
-    }
+        break;
-
+      case UPGRADE_NEEDED:
-    TRI_DEFER(V8DealerFeature::DEALER->exitContext(context));
+        // this is safe to do even if further databases will be checked
-
+        // because we will never set the status back to success
-    {
+        *_result = 3;
-      v8::HandleScope scope(context->_isolate);
+        LOG_TOPIC(WARN, arangodb::Logger::FIXME) << "Database version check failed for '"
-      auto localContext =
+                   << vocbase->name() << "': upgrade needed";
-          v8::Local<v8::Context>::New(context->_isolate, context->_context);
+        break;
-      localContext->Enter();
+      case DOWNGRADE_NEEDED:
-
+        if (*_result == 1) {
-      {
+          // this is safe to do even if further databases will be checked
-        v8::Context::Scope contextScope(localContext);
+          // because we will never set the status back to success
-
+          *_result = 2;
-        // run version-check script
+          LOG_TOPIC(WARN, arangodb::Logger::FIXME) << "Database version check failed for '"
-        LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "running database version check";
+                     << vocbase->name() << "': downgrade needed";
-
-        // can do this without a lock as this is the startup
-        DatabaseFeature* databaseFeature = application_features::ApplicationServer::getFeature<DatabaseFeature>("Database");
-
-        // iterate over all databases
-        for (auto& name : databaseFeature->getDatabaseNames()) {
-          TRI_vocbase_t* vocbase = databaseFeature->lookupDatabase(name);
-
-          TRI_ASSERT(vocbase != nullptr);
-
-          // special check script to be run just once in first thread (not in
-          // all) but for all databases
-          int status = TRI_CheckDatabaseVersion(vocbase, localContext);
-
-          LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "version check return status " << status;
-
-          if (status < 0) {
-            LOG_TOPIC(FATAL, arangodb::Logger::FIXME) << "Database version check failed for '"
-                       << vocbase->name()
-                       << "'. Please inspect the logs for any errors";
-            FATAL_ERROR_EXIT_CODE(TRI_EXIT_VERSION_CHECK_FAILED);
-          } else if (status == 3) {
-            // this is safe to do even if further databases will be checked
-            // because we will never set the status back to success
-            *_result = 3;
-            LOG_TOPIC(WARN, arangodb::Logger::FIXME) << "Database version check failed for '"
-                       << vocbase->name() << "': upgrade needed";
-          } else if (status == 2 && *_result == 1) {
-            // this is safe to do even if further databases will be checked
-            // because we will never set the status back to success
-            *_result = 2;
-            LOG_TOPIC(WARN, arangodb::Logger::FIXME) << "Database version check failed for '"
-                       << vocbase->name() << "': downgrade needed";
-          }
         }
-      }
+        break;
-
+      default:
-      // issue #391: when invoked with --database.auto-upgrade, the server will
+        LOG_TOPIC(FATAL, arangodb::Logger::FIXME) << "Database version check failed for '"
-      // not always shut down
+                   << vocbase->name()
-      localContext->Exit();
+                   << "'. Please inspect the logs for any errors";
+        FATAL_ERROR_EXIT_CODE(TRI_EXIT_VERSION_CHECK_FAILED);
     }
-  }
+  };
+  databaseFeature->enumerateDatabases(checkDBVersion);
 
   LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "final result of version check: " << *_result;
 

arangod/RestServer/CheckVersionFeature.h

@@ -25,9 +25,26 @@
 
 #include "ApplicationFeatures/ApplicationFeature.h"
 
+struct TRI_vocbase_t;
+
 namespace arangodb {
+
+class StorageEngine;
+
 class CheckVersionFeature final
     : public application_features::ApplicationFeature {
+ private: 
+  enum CheckVersionResult : int {
+    NO_SERVER_VERSION = -5,
+    NO_VERSION_FILE = -4,
+    CANNOT_READ_VERSION_FILE = -3,
+    CANNOT_PARSE_VERSION_FILE = -2,
+    IS_CLUSTER = -1,
+    VERSION_MATCH = 1,
+    DOWNGRADE_NEEDED = 2,
+    UPGRADE_NEEDED = 3,
+  };
+
  public:
   explicit CheckVersionFeature(
       application_features::ApplicationServer* server, int* result,
@@ -44,6 +61,10 @@ class CheckVersionFeature final
  private:
   void checkVersion();
 
+  CheckVersionResult checkVersionFileForDB(TRI_vocbase_t* vocbase,
+                                           StorageEngine* engine,
+                                           uint32_t currentVersion) const;
+
  private:
   int* _result;
   std::vector<std::string> _nonServerFeatures;

arangod/RestServer/LockfileFeature.cpp

@@ -92,3 +92,7 @@ void LockfileFeature::start() {
 void LockfileFeature::unprepare() {
   TRI_DestroyLockFile(_lockFilename.c_str());
 }
+
+void LockfileFeature::beginShutdown() {
+  TRI_DestroyLockFile(_lockFilename.c_str());
+}

arangod/RestServer/LockfileFeature.h

@@ -33,6 +33,7 @@ class LockfileFeature final : public application_features::ApplicationFeature {
  public:
   void start() override final;
   void unprepare() override final;
+  void beginShutdown() override final;
 
  private:
   std::string _lockFilename;

arangod/V8Server/V8DealerFeature.cpp

@@ -109,6 +109,7 @@ V8DealerFeature::V8DealerFeature(
   requiresElevatedPrivileges(false);
   startsAfter("Action");
   startsAfter("Authentication");
+  startsAfter("CheckVersion");
   startsAfter("Database");
   startsAfter("Random");
   startsAfter("Scheduler");