Maintenance delayed by incomplete hashing maintenance actions (#6448)

arangod/Cluster/ActionBase.cpp

@@ -225,6 +225,14 @@ VPackBuilder ActionBase::toVelocyPack() const {
 }
 
 
+ActionState ActionBase::getState() const {
+  return _state;
+}
+
+void ActionBase::setState(ActionState state) {
+  _state = state;
+}
+
 
 /**
  * kill() operation is an expected future feature.  Not supported in the

arangod/Cluster/ActionBase.h

@@ -99,15 +99,11 @@ class ActionBase {
   VPackSlice const properties() const;
 
   /// @brief adjust state of object, assumes WRITE lock on _actionRegistryLock
-  ActionState getState() const {
+  ActionState getState() const;
-    return _state;
-  }
 
   /// @brief adjust state of object, assumes WRITE lock on _actionRegistryLock
-  void setState(ActionState state) {
+  virtual void setState(ActionState state);
-    _state = state;
+  
-  }
-
   /// @brief update incremental statistics
   void startStats();
 

arangod/Cluster/ActionDescription.h

@@ -159,8 +159,6 @@ public:
 
 private:
 
-  /// Note: members are const. No thread safety guards needed.
-
   /** @brief discriminatory properties */
   std::map<std::string, std::string> const _description;
 

arangod/Cluster/CreateCollection.cpp

@@ -97,7 +97,7 @@ CreateCollection::CreateCollection(
     _result.reset(TRI_ERROR_INTERNAL, error.str());
     setState(FAILED);
   }
-
+  
 }
 
 
@@ -186,11 +186,23 @@ bool CreateCollection::first() {
   }
 
   if (_result.fail()) {
-    _feature.storeShardError(database, collection, shard,
+    _feature.storeShardError(
-        _description.get(SERVER_ID), _result);
+      database, collection, shard, _description.get(SERVER_ID), _result);
   }
 
   notify();
 
   return false;
 }
+
+
+void CreateCollection::setState(ActionState state) {
+  
+  if ((COMPLETE==state || FAILED==state) && _state != state) {
+    TRI_ASSERT(_description.has("shard"));
+    _feature.incShardVersion(_description.get("shard"));
+  }
+  
+  ActionBase::setState(state);
+  
+}

arangod/Cluster/CreateCollection.h

@@ -43,6 +43,8 @@ public:
 
   virtual bool first() override final;
 
+  virtual void setState(ActionState state) override final;
+
 };
 
 }}

arangod/Cluster/DBServerAgencySync.cpp

@@ -184,24 +184,22 @@ DBServerAgencySyncResult DBServerAgencySync::execute() {
   }
 
   if (rb.isClosed()) {
-    // FIXMEMAINTENANCE: when would rb not be closed? and if "catch"
-    // just happened, would you want to be doing this anyway?
 
     auto report = rb.slice();
     if (report.isObject()) {
 
-      std::vector<std::string> agency = {maintenance::PHASE_TWO, "agency"};
+      std::vector<std::string> path = {maintenance::PHASE_TWO, "agency"};
-      if (report.hasKey(agency) && report.get(agency).isObject()) {
+      if (report.hasKey(path) && report.get(path).isObject()) {
-
+        
-        auto phaseTwo = report.get(agency);
+        auto agency = report.get(path);
         LOG_TOPIC(DEBUG, Logger::MAINTENANCE)
-          << "DBServerAgencySync reporting to Current: " << phaseTwo.toJson();
+          << "DBServerAgencySync reporting to Current: " << agency.toJson();
 
         // Report to current
-        if (!phaseTwo.isEmptyObject()) {
+        if (!agency.isEmptyObject()) {
-
+          
           std::vector<AgencyOperation> operations;
-          for (auto const& ao : VPackObjectIterator(phaseTwo)) {
+          for (auto const& ao : VPackObjectIterator(agency)) {
             auto const key = ao.key.copyString();
             auto const op = ao.value.get("op").copyString();
             if (op == "set") {
@@ -225,10 +223,9 @@ DBServerAgencySyncResult DBServerAgencySync::execute() {
             clusterInfo->invalidateCurrent();
           }
         }
-      }
 
-      // FIXMEMAINTENANCE: If comm.sendTransactionWithFailover()
+      }
-      // fails, the result is ok() based upon phaseTwo()'s execution?
+            
       result = DBServerAgencySyncResult(
         tmp.ok(),
         report.hasKey("Plan") ?

arangod/Cluster/DropCollection.cpp

@@ -27,6 +27,7 @@
 #include "ApplicationFeatures/ApplicationServer.h"
 #include "Basics/VelocyPackHelper.h"
 #include "Cluster/ClusterFeature.h"
+#include "Cluster/MaintenanceFeature.h"
 #include "Utils/DatabaseGuard.h"
 #include "VocBase/Methods/Collections.h"
 #include "VocBase/Methods/Databases.h"
@@ -107,6 +108,10 @@ bool DropCollection::first() {
     return false;
   }
 
+  // We're removing the shard version from MaintenanceFeature before notifying
+  // for new Maintenance run. This should make sure that the next round does not
+  // get rejected.
+  _feature.delShardVersion(collection);
   notify();
 
   return false;

arangod/Cluster/Maintenance.cpp

@@ -190,7 +190,8 @@ void handlePlanShard(
   std::string const& dbname, std::string const& colname, std::string const& shname,
   std::string const& serverId, std::string const& leaderId,
   std::unordered_set<std::string>& commonShrds, std::unordered_set<std::string>& indis,
-  MaintenanceFeature::errors_t& errors, std::vector<ActionDescription>& actions) {
+  MaintenanceFeature::errors_t& errors, MaintenanceFeature& feature,
+  std::vector<ActionDescription>& actions) {
 
   bool shouldBeLeading = serverId == leaderId;
 
@@ -350,7 +351,8 @@ struct NotEmpty {
 /// @brief calculate difference between plan and local for for databases
 arangodb::Result arangodb::maintenance::diffPlanLocal (
   VPackSlice const& plan, VPackSlice const& local, std::string const& serverId,
-  MaintenanceFeature::errors_t& errors, std::vector<ActionDescription>& actions) {
+  MaintenanceFeature::errors_t& errors, MaintenanceFeature& feature,
+  std::vector<ActionDescription>& actions) {
 
   arangodb::Result result;
   std::unordered_set<std::string> commonShrds; // Intersection collections plan&local
@@ -405,7 +407,7 @@ arangodb::Result arangodb::maintenance::diffPlanLocal (
                 handlePlanShard(
                   cprops, ldb, dbname, pcol.key.copyString(),
                   shard.key.copyString(), serverId, shard.value[0].copyString(),
-                  commonShrds, indis, errors, actions);
+                  commonShrds, indis, errors, feature, actions);
                 break ;
               }
             }
@@ -494,7 +496,7 @@ arangodb::Result arangodb::maintenance::executePlan (
   std::vector<ActionDescription> actions;
   report.add(VPackValue(AGENCY));
   { VPackArrayBuilder a(&report);
-    diffPlanLocal(plan, local, serverId, errors, actions); }
+    diffPlanLocal(plan, local, serverId, errors, feature, actions); }
 
   for (auto const& i : errors.databases) {
     if (i.second == nullptr) {
@@ -974,7 +976,8 @@ arangodb::Result arangodb::maintenance::reportInCurrent(
 
 arangodb::Result arangodb::maintenance::syncReplicatedShardsWithLeaders(
   VPackSlice const& plan, VPackSlice const& current, VPackSlice const& local,
-  std::string const& serverId, std::vector<ActionDescription>& actions) {
+  std::string const& serverId, MaintenanceFeature& feature,
+  std::vector<ActionDescription>& actions) {
 
   auto pdbs = plan.get(COLLECTIONS);
   auto cdbs = current.get(COLLECTIONS);
@@ -1035,7 +1038,8 @@ arangodb::Result arangodb::maintenance::syncReplicatedShardsWithLeaders(
             actions.emplace_back(
               ActionDescription(
                 {{NAME, "SynchronizeShard"}, {DATABASE, dbname},
-                 {COLLECTION, colname}, {SHARD, shname}, {THE_LEADER, leader}}));
+                 {COLLECTION, colname}, {SHARD, shname}, {THE_LEADER, leader},
+                 {SHARD_VERSION, std::to_string(feature.shardVersion(shname))}}));
 
           }
         }
@@ -1080,7 +1084,7 @@ arangodb::Result arangodb::maintenance::phaseTwo (
       try {
         std::vector<ActionDescription> actions;
         result = syncReplicatedShardsWithLeaders(
-          plan, cur, local, serverId, actions);
+          plan, cur, local, serverId, feature, actions);
 
         for (auto const& action : actions) {
           feature.addAction(std::make_shared<ActionDescription>(action), false);

arangod/Cluster/Maintenance.h

@@ -50,13 +50,15 @@ arangodb::Result diffPlanLocalForDatabases(
  * @param local    Snapshot of local state
  * @param serverId This server's UUID
  * @param errors   Copy of last maintenance feature errors
+ * @param feature   The feature itself
  * @param actions  Resulting actions from difference are packed in here
  *
  * @return         Result
  */
 arangodb::Result diffPlanLocal(
   VPackSlice const& plan, VPackSlice const& local, std::string const& serverId,
-  MaintenanceFeature::errors_t& errors, std::vector<ActionDescription>& actions);
+  MaintenanceFeature::errors_t& errors, MaintenanceFeature& feature,
+  std::vector<ActionDescription>& actions);
 
 /**
  * @brief          Difference Plan and local for phase 1 of Maintenance run 
@@ -153,7 +155,8 @@ arangodb::Result reportInCurrent(
  */
 arangodb::Result syncReplicatedShardsWithLeaders(
   VPackSlice const& plan, VPackSlice const& current, VPackSlice const& local,
-  std::string const& serverId, std::vector<ActionDescription>& actions);
+  std::string const& serverId, MaintenanceFeature& feature,
+  std::vector<ActionDescription>& actions);
 
 }}
 

arangod/Cluster/MaintenanceFeature.cpp

@@ -747,3 +747,29 @@ arangodb::Result MaintenanceFeature::copyAllErrors(errors_t& errors) const {
 }
 
 
+uint64_t MaintenanceFeature::shardVersion (std::string const& shname) const {
+  MUTEX_LOCKER(guard, _versionLock);
+  auto const it = _shardVersion.find(shname);
+  LOG_TOPIC(TRACE, Logger::MAINTENANCE)
+    << "getting shard version for '"  << shname << "' from " << _shardVersion;
+  return (it != _shardVersion.end()) ? it->second : 0;
+}
+
+
+uint64_t MaintenanceFeature::incShardVersion (std::string const& shname) {
+  MUTEX_LOCKER(guard, _versionLock);
+  auto ret = ++_shardVersion[shname];
+  LOG_TOPIC(TRACE, Logger::MAINTENANCE)
+    << "incremented shard version for " << shname << " to " << ret;
+  return ret;
+}
+
+void MaintenanceFeature::delShardVersion (std::string const& shname) {
+  MUTEX_LOCKER(guard, _versionLock);
+  auto const it = _shardVersion.find(shname);
+  if (it != _shardVersion.end()) {
+    _shardVersion.erase(it);
+  }
+}
+
+

arangod/Cluster/MaintenanceFeature.h

@@ -297,6 +297,22 @@ public:
 
   /// @brief Highest limit for worker threads
   static uint32_t const maxThreadLimit;
+
+  /**
+   * @brief get volatile shard version
+   */
+  uint64_t shardVersion(std::string const& shardId) const;
+
+  /**
+   * @brief increment volatile local shard version
+   */
+  uint64_t incShardVersion(std::string const& shardId);
+
+  /**
+   * @brief clean up after shard has been dropped locally
+   * @param  shard  Shard name
+   */
+  void delShardVersion(std::string const& shardId);
   
 protected:
   /// @brief common code used by multiple constructors
@@ -383,6 +399,12 @@ protected:
   std::unordered_map<std::string,
                      std::shared_ptr<VPackBuffer<uint8_t>>> _dbErrors;
 
+  /// @brief lock for shard version map
+  mutable arangodb::Mutex _versionLock;
+  /// @brief shards have versions in order to be able to distinguish between
+  /// independant actions
+  std::unordered_map<std::string, size_t> _shardVersion;
+
   
   
 };

arangod/Cluster/MaintenanceStrings.h

@@ -62,13 +62,13 @@ constexpr char const* SERVER_ID = "serverId";
 constexpr char const* SERVERS = "servers";
 constexpr char const* SHARD = "shard";
 constexpr char const* SHARDS = "shards";
+constexpr char const* SHARD_VERSION = "shardVersion";
 constexpr char const* SYNCHRONIZE_SHARD = "SynchronizeShard";
 constexpr char const* THE_LEADER = "theLeader";
 constexpr char const* UNDERSCORE = "_";
 constexpr char const* UPDATE_COLLECTION = "UpdateCollection";
 constexpr char const* WAIT_FOR_SYNC = "waitForSync";
 
-}
+}}
-}
 
 #endif

arangod/Cluster/SynchronizeShard.cpp

@@ -107,6 +107,11 @@ SynchronizeShard::SynchronizeShard(
   }
   TRI_ASSERT(desc.has(THE_LEADER));
 
+  if (!desc.has(SHARD_VERSION)) {
+    error << "local shard version must be specified. ";
+  }
+  TRI_ASSERT(desc.has(SHARD_VERSION));
+
   if (!error.str().empty()) {
     LOG_TOPIC(ERR, Logger::MAINTENANCE) << "SynchronizeShard: " << error.str();
     _result.reset(TRI_ERROR_INTERNAL, error.str());
@@ -123,6 +128,8 @@ public:
   }
 };
 
+SynchronizeShard::~SynchronizeShard() {}
+
 
 arangodb::Result getReadLockId (
   std::string const& endpoint, std::string const& database,
@@ -951,4 +958,17 @@ bool SynchronizeShard::first() {
 
 }
 
-SynchronizeShard::~SynchronizeShard() {}
+
+void SynchronizeShard::setState(ActionState state) {
+  
+  if ((COMPLETE==state || FAILED==state) && _state != state) {
+    TRI_ASSERT(_description.has("shard"));
+    _feature.incShardVersion(_description.get("shard"));
+  }
+  
+  ActionBase::setState(state);
+  
+}
+
+
+

arangod/Cluster/SynchronizeShard.h

@@ -31,9 +31,6 @@
 #include <chrono>
 
 namespace arangodb {
-
-class MaintenanceAction;
-
 namespace maintenance {
 
 class SynchronizeShard : public ActionBase {
@@ -44,7 +41,9 @@ public:
 
   virtual ~SynchronizeShard();
 
-  virtual bool first() override;
+  virtual bool first() override final;
+
+  virtual void setState(ActionState state) override final;
 
 private:
   arangodb::Result getReadLock(

tests/Maintenance/MaintenanceFeatureMock.h

@@ -0,0 +1,198 @@
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test suite for Cluster maintenance
+///
+/// @file
+///
+/// DISCLAIMER
+///
+/// Copyright 2017 ArangoDB GmbH, Cologne, Germany
+///
+/// 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.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Kaveh Vahedipour
+/// @author Matthew Von-Maszewski
+/// @author Copyright 2017-2018, ArangoDB GmbH, Cologne, Germany
+////////////////////////////////////////////////////////////////////////////////
+
+#include "Basics/ConditionLocker.h"
+#include "Basics/ConditionVariable.h"
+
+#include "ApplicationFeatures/ApplicationServer.h"
+
+//
+// structure used to store expected states of action properties
+//
+struct Expected {
+  int _id;
+  int _result;
+  int _state;
+  int _progress;
+};
+
+typedef std::vector<Expected> ExpectedVec_t;
+
+//
+// TestProgressHandler lets us know once ApplicationServer is ready
+//
+class TestProgressHandler : public arangodb::application_features::ProgressHandler {
+public:
+  TestProgressHandler() {
+    _serverReady=false;
+
+    using std::placeholders::_1;
+    _state = std::bind(& TestProgressHandler::StateChange, this, _1);
+
+    using std::placeholders::_2;
+    _feature = std::bind(& TestProgressHandler::FeatureChange, this, _1, _2);
+  }
+
+
+  void StateChange(arangodb::application_features::ServerState newState) {
+    if (arangodb::application_features::ServerState::IN_WAIT == newState) {
+      CONDITION_LOCKER(clock, _serverReadyCond);
+      _serverReady = true;
+      _serverReadyCond.broadcast();
+    }
+  }
+
+  void FeatureChange(arangodb::application_features::ServerState newState, std::string const &) {
+  }
+
+  arangodb::basics::ConditionVariable _serverReadyCond;
+  std::atomic_bool _serverReady;
+
+};// class TestProgressHandler
+
+
+using namespace arangodb::maintenance;
+
+//
+// TestFeature wraps MaintenanceFeature to all test specific action objects
+//  by overriding the actionFactory() virtual function.  Two versions:
+//  1. default constructor for non-threaded actions
+//  2. constructor with ApplicationServer pointer for threaded actions
+//
+class TestMaintenanceFeature : public arangodb::MaintenanceFeature {
+public:
+  TestMaintenanceFeature(arangodb::application_features::ApplicationServer& as)
+    : arangodb::MaintenanceFeature(as) {
+
+    // force activation of the feature, even in agency/single-server mode
+    // (the catch tests use single-server mode)
+    _forceActivation = true;
+
+    // begin with no threads to allow queue validation
+    _maintenanceThreadsMax = 0;
+    as.addReporter(_progressHandler);
+  }
+
+  virtual ~TestMaintenanceFeature() {}
+
+  void validateOptions(std::shared_ptr<arangodb::options::ProgramOptions> options) override {}
+
+  void setSecondsActionsBlock(uint32_t seconds) { _secondsActionsBlock = seconds; }
+
+  /// @brief set thread count, then activate the threads via start().  One time usage only.
+  ///   Code waits until background ApplicationServer known to have fully started.
+  void setMaintenanceThreadsMax(uint32_t threads) {
+    CONDITION_LOCKER(clock, _progressHandler._serverReadyCond);
+    while(!_progressHandler._serverReady) {
+      _progressHandler._serverReadyCond.wait();
+    } // while
+
+    _maintenanceThreadsMax = threads;
+    start();
+  } // setMaintenanceThreadsMax
+
+
+  virtual arangodb::Result addAction(
+    std::shared_ptr<arangodb::maintenance::Action> action, bool executeNow = false) override {
+    _recentAction = action;
+    return MaintenanceFeature::addAction(action, executeNow);
+  }
+
+  virtual arangodb::Result addAction(
+    std::shared_ptr<arangodb::maintenance::ActionDescription> const & description,
+    bool executeNow = false) override {
+    return MaintenanceFeature::addAction(description, executeNow);
+  }
+
+
+  bool verifyRegistryState(ExpectedVec_t & expected) {
+    bool good(true);
+
+    VPackBuilder registryBuilder(toVelocyPack());
+    VPackArrayIterator registry(registryBuilder.slice());
+
+    auto action = registry.begin();
+    auto check = expected.begin();
+
+    for ( ; registry.end() != action && expected.end()!=check; ++action, ++check) {
+      VPackSlice id = (*action).get("id");
+      if (!(id.isInteger() && id.getInt() == check->_id)) {
+        std::cerr << "Id mismatch: action has " << id.getInt()
+                  << " expected " << check->_id << std::endl;
+        good = false;
+      } // if
+
+      VPackSlice result = (*action).get("result");
+      if (!(result.isInteger() && check->_result == result.getInt())) {
+        std::cerr << "Result mismatch: action has " << result.getInt()
+                  << " expected " << check->_result << std::endl;
+        good = false;
+      } // if
+
+      VPackSlice state = (*action).get("state");
+      if (!(state.isInteger() && check->_state == state.getInt())) {
+        std::cerr << "State mismatch: action has " << state.getInt()
+                  << " expected " << check->_state << std::endl;
+        good = false;
+      } // if
+
+      VPackSlice progress = (*action).get("progress");
+      if (!(progress.isInteger() && check->_progress == progress.getInt())) {
+        std::cerr << "Progress mismatch: action has " << progress.getInt()
+                  << " expected " << check->_progress << std::endl;
+        good = false;
+      } // if
+    } // for
+
+    return good;
+
+  } // verifyRegistryState
+
+
+  /// @brief poll registry until all actions finish
+  void waitRegistryComplete() {
+    bool again;
+
+    do {
+      again = false;
+      std::this_thread::sleep_for(std::chrono::seconds(1));
+
+      VPackBuilder registryBuilder(toVelocyPack());
+      VPackArrayIterator registry(registryBuilder.slice());
+      for (auto action : registry) {
+        VPackSlice state = action.get("state");
+        again = again || (COMPLETE != state.getInt() && FAILED != state.getInt());
+      } // for
+    } while (again);
+  } // waitRegistryComplete
+
+public:
+  std::shared_ptr<Action> _recentAction;
+  TestProgressHandler _progressHandler;
+
+};// TestMaintenanceFeature

tests/Maintenance/MaintenanceFeatureTest.cpp

@@ -34,178 +34,11 @@
 #include <velocypack/velocypack-aliases.h>
 
 #include "ApplicationFeatures/ApplicationServer.h"
-#include "Basics/ConditionLocker.h"
-#include "Basics/ConditionVariable.h"
 #include "Basics/Result.h"
 #include "Cluster/Action.h"
 #include "Cluster/MaintenanceFeature.h"
 
-//
+#include "MaintenanceFeatureMock.h"
-// structure used to store expected states of action properties
-//
-struct Expected {
-  int _id;
-  int _result;
-  int _state;
-  int _progress;
-};
-
-typedef std::vector<Expected> ExpectedVec_t;
-
-//
-// TestProgressHandler lets us know once ApplicationServer is ready
-//
-class TestProgressHandler : public arangodb::application_features::ProgressHandler {
-public:
-  TestProgressHandler() {
-    _serverReady=false;
-
-    using std::placeholders::_1;
-    _state = std::bind(& TestProgressHandler::StateChange, this, _1);
-
-    using std::placeholders::_2;
-    _feature = std::bind(& TestProgressHandler::FeatureChange, this, _1, _2);
-  }
-
-
-  void StateChange(arangodb::application_features::ServerState newState) {
-    if (arangodb::application_features::ServerState::IN_WAIT == newState) {
-      CONDITION_LOCKER(clock, _serverReadyCond);
-      _serverReady = true;
-      _serverReadyCond.broadcast();
-    }
-  }
-
-  void FeatureChange(arangodb::application_features::ServerState newState, std::string const &) {
-  }
-
-  arangodb::basics::ConditionVariable _serverReadyCond;
-  std::atomic_bool _serverReady;
-
-};// class TestProgressHandler
-
-
-using namespace arangodb::maintenance;
-
-//
-// TestFeature wraps MaintenanceFeature to all test specific action objects
-//  by overriding the actionFactory() virtual function.  Two versions:
-//  1. default constructor for non-threaded actions
-//  2. constructor with ApplicationServer pointer for threaded actions
-//
-class TestMaintenanceFeature : public arangodb::MaintenanceFeature {
-public:
-  TestMaintenanceFeature(arangodb::application_features::ApplicationServer& as)
-    : arangodb::MaintenanceFeature(as) {
-
-    // force activation of the feature, even in agency/single-server mode
-    // (the catch tests use single-server mode)
-    _forceActivation = true;
-
-    // begin with no threads to allow queue validation
-    _maintenanceThreadsMax = 0;
-    as.addReporter(_progressHandler);
-  }
-
-  virtual ~TestMaintenanceFeature() {}
-
-  void validateOptions(std::shared_ptr<arangodb::options::ProgramOptions> options) override {}
-
-  void setSecondsActionsBlock(uint32_t seconds) { _secondsActionsBlock = seconds; }
-
-  /// @brief set thread count, then activate the threads via start().  One time usage only.
-  ///   Code waits until background ApplicationServer known to have fully started.
-  void setMaintenanceThreadsMax(uint32_t threads) {
-    CONDITION_LOCKER(clock, _progressHandler._serverReadyCond);
-    while(!_progressHandler._serverReady) {
-      _progressHandler._serverReadyCond.wait();
-    } // while
-
-    _maintenanceThreadsMax = threads;
-    start();
-  } // setMaintenanceThreadsMax
-
-
-  virtual arangodb::Result addAction(
-    std::shared_ptr<arangodb::maintenance::Action> action, bool executeNow = false) override {
-    _recentAction = action;
-    return MaintenanceFeature::addAction(action, executeNow);
-  }
-
-  virtual arangodb::Result addAction(
-    std::shared_ptr<arangodb::maintenance::ActionDescription> const & description,
-    bool executeNow = false) override {
-    return MaintenanceFeature::addAction(description, executeNow);
-  }
-
-
-  bool verifyRegistryState(ExpectedVec_t & expected) {
-    bool good(true);
-
-    VPackBuilder registryBuilder(toVelocyPack());
-    VPackArrayIterator registry(registryBuilder.slice());
-
-    auto action = registry.begin();
-    auto check = expected.begin();
-
-    for ( ; registry.end() != action && expected.end()!=check; ++action, ++check) {
-      VPackSlice id = (*action).get("id");
-      if (!(id.isInteger() && id.getInt() == check->_id)) {
-        std::cerr << "Id mismatch: action has " << id.getInt()
-                  << " expected " << check->_id << std::endl;
-        good = false;
-      } // if
-
-      VPackSlice result = (*action).get("result");
-      if (!(result.isInteger() && check->_result == result.getInt())) {
-        std::cerr << "Result mismatch: action has " << result.getInt()
-                  << " expected " << check->_result << std::endl;
-        good = false;
-      } // if
-
-      VPackSlice state = (*action).get("state");
-      if (!(state.isInteger() && check->_state == state.getInt())) {
-        std::cerr << "State mismatch: action has " << state.getInt()
-                  << " expected " << check->_state << std::endl;
-        good = false;
-      } // if
-
-      VPackSlice progress = (*action).get("progress");
-      if (!(progress.isInteger() && check->_progress == progress.getInt())) {
-        std::cerr << "Progress mismatch: action has " << progress.getInt()
-                  << " expected " << check->_progress << std::endl;
-        good = false;
-      } // if
-    } // for
-
-    return good;
-
-  } // verifyRegistryState
-
-
-  /// @brief poll registry until all actions finish
-  void waitRegistryComplete() {
-    bool again;
-
-    do {
-      again = false;
-      std::this_thread::sleep_for(std::chrono::seconds(1));
-
-      VPackBuilder registryBuilder(toVelocyPack());
-      VPackArrayIterator registry(registryBuilder.slice());
-      for (auto action : registry) {
-        VPackSlice state = action.get("state");
-        again = again || (COMPLETE != state.getInt() && FAILED != state.getInt());
-      } // for
-    } while (again);
-  } // waitRegistryComplete
-
-public:
-  std::shared_ptr<Action> _recentAction;
-  TestProgressHandler _progressHandler;
-
-};// TestMaintenanceFeature
-
 
 //
 // TestActionBasic simulates a multistep action by counting down

tests/Maintenance/MaintenanceTest.cpp

@@ -23,7 +23,7 @@
 ///
 /// @author Kaveh Vahedipour
 /// @author Matthew Von-Maszewski
-/// @author Copyright 2017, ArangoDB GmbH, Cologne, Germany
+/// @author Copyright 2017-2018, ArangoDB GmbH, Cologne, Germany
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "catch.hpp"
@@ -39,6 +39,8 @@
 #include <random>
 #include <typeinfo>
 
+#include "MaintenanceFeatureMock.h"
+
 using namespace arangodb;
 using namespace arangodb::consensus;
 using namespace arangodb::maintenance;
@@ -411,13 +413,18 @@ TEST_CASE("ActionDescription", "[cluster][maintenance]") {
 
 TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
+  std::shared_ptr<arangodb::options::ProgramOptions> po =
+    std::make_shared<arangodb::options::ProgramOptions>(
+      "test", std::string(), std::string(), "path");
+  arangodb::application_features::ApplicationServer as(po, nullptr);
+  TestMaintenanceFeature feature(as);
+  MaintenanceFeature::errors_t errors;
+
   std::map<std::string, Node> localNodes {
     {dbsIds[shortNames[0]], createNode(dbs0Str)},
     {dbsIds[shortNames[1]], createNode(dbs1Str)},
     {dbsIds[shortNames[2]], createNode(dbs2Str)}};
 
-  MaintenanceFeature::errors_t errors;
-  
   SECTION("In sync should have 0 effects") {
 
     std::vector<ActionDescription> actions;
@@ -425,7 +432,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
     for (auto const& node : localNodes) {
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
 
       if (actions.size() != 0) {
         std::cout << __FILE__ << ":" << __LINE__ << " " << actions  << std::endl;
@@ -445,7 +452,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
     arangodb::maintenance::diffPlanLocal(
       plan.toBuilder().slice(), localNodes.begin()->second.toBuilder().slice(),
-      localNodes.begin()->first, errors, actions);
+      localNodes.begin()->first, errors, feature, actions);
 
     if (actions.size() != 1) {
       std::cout << __FILE__ << ":" << __LINE__ << " " << actions  << std::endl;
@@ -465,7 +472,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
     arangodb::maintenance::diffPlanLocal(
       plan.toBuilder().slice(), localNodes.begin()->second.toBuilder().slice(),
-      localNodes.begin()->first, errors, actions);
+      localNodes.begin()->first, errors, feature, actions);
 
     REQUIRE(actions.size() == 1);
     REQUIRE(actions.front().name() == "DropDatabase");
@@ -491,7 +498,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
 
       if (actions.size() != 1) {
         std::cout << __FILE__ << ":" << __LINE__ << " " << actions  << std::endl;
@@ -522,7 +529,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
 
       if (actions.size() != 2) {
         std::cout << __FILE__ << ":" << __LINE__ << " " << actions  << std::endl;
@@ -551,7 +558,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
       auto local = node.second;
       
       arangodb::maintenance::diffPlanLocal(
-        plan.toBuilder().slice(), local.toBuilder().slice(), node.first, errors,
+        plan.toBuilder().slice(), local.toBuilder().slice(), node.first, errors, feature,
         actions);
 
       size_t n = 0;
@@ -591,7 +598,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
       auto local = node.second;
       
       arangodb::maintenance::diffPlanLocal(
-        plan.toBuilder().slice(), local.toBuilder().slice(), node.first, errors,
+        plan.toBuilder().slice(), local.toBuilder().slice(), node.first, errors, feature,
         actions);
       
       size_t n = 0;
@@ -624,7 +631,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
       
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
       
       if (actions.size() != 1) {
         std::cout << __FILE__ << ":" << __LINE__ << " " << actions  << std::endl;
@@ -660,7 +667,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
 
       /*
       if (actions.size() != 1) {
@@ -699,7 +706,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
 
       if (check) {
         if (actions.size() != 1) {
@@ -732,7 +739,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
 
       arangodb::maintenance::diffPlanLocal(
         plan.toBuilder().slice(), node.second.toBuilder().slice(),
-        node.first, errors, actions);
+        node.first, errors, feature, actions);
 
       REQUIRE(actions.size() == node.second("db3").children().size());
       for (auto const& action : actions) {
@@ -780,7 +787,7 @@ TEST_CASE("ActionPhaseOne", "[cluster][maintenance]") {
       
       arangodb::maintenance::diffPlanLocal (
         plan.toBuilder().slice(), node.second.toBuilder().slice(), node.first,
-        errors, actions);
+        errors, feature, actions);
 
       if (actions.size() != 2) {
         std::cout << actions << std::endl;