arangodb · maierlars · Jul 17, 2023 · May 26, 2023 · May 30, 2023 · May 30, 2023
diff --git a/arangod/Agency/ActiveFailoverJob.cpp b/arangod/Agency/ActiveFailoverJob.cpp
@@ -154,8 +154,8 @@ bool ActiveFailoverJob::start(bool&) {
     return finish(_server, "", true, reason);  // move to /Target/Finished
   }
 
-  auto leader = _snapshot.hasAsSlice(asyncReplLeader);
-  if (!leader || leader->compareString(_server) != 0) {
+  auto leader = _snapshot.hasAsStringView(asyncReplLeader);
+  if (!leader || leader != _server) {
     std::string reason =
         "Server " + _server + " is not the current replication leader";
     LOG_TOPIC("d468e", INFO, Logger::SUPERVISION) << reason;
@@ -222,7 +222,7 @@ bool ActiveFailoverJob::start(bool&) {
       // Destination server should not be blocked by another job
       addPreconditionServerNotBlocked(pending, newLeader);
       // AsyncReplication leader must be the failed server
-      addPreconditionUnchanged(pending, asyncReplLeader, leader.value());
+      addPreconditionUnchanged(pending, asyncReplLeader, VPackValue(*leader));
     }  // precondition done
 
   }  // array for transaction done

diff --git a/arangod/Agency/AddFollower.cpp b/arangod/Agency/AddFollower.cpp
@@ -147,7 +147,7 @@ bool AddFollower::start(bool&) {
     return false;
   }
   Node const& collection =
-      *_snapshot.hasAsNode(planColPrefix + _database + "/" + _collection);
+      *_snapshot.get(planColPrefix + _database + "/" + _collection);
   if (collection.has("distributeShardsLike")) {
     finish("", "", false,
            "collection must not have 'distributeShardsLike' attribute");
@@ -158,8 +158,8 @@ bool AddFollower::start(bool&) {
   std::string planPath =
       planColPrefix + _database + "/" + _collection + "/shards/" + _shard;
 
-  Slice planned = _snapshot.hasAsSlice(planPath).value();
-
+  auto plannedBuilder = _snapshot.get(planPath)->toBuilder();
+  auto planned = plannedBuilder.slice();
   TRI_ASSERT(planned.isArray());
 
   // First check that we still have too few followers for the current
@@ -350,9 +350,9 @@ bool AddFollower::start(bool&) {
             // "failoverCandidates":
             std::string foCandsPath = curPath.substr(0, curPath.size() - 7);
             foCandsPath += StaticStrings::FailoverCandidates;
-            auto foCands = this->_snapshot.hasAsSlice(foCandsPath);
+            auto foCands = this->_snapshot.hasAsBuilder(foCandsPath);
             if (foCands) {
-              addPreconditionUnchanged(trx, foCandsPath, foCands.value());
+              addPreconditionUnchanged(trx, foCandsPath, foCands->slice());
             }
           });
       addPreconditionShardNotBlocked(trx, _shard);

diff --git a/arangod/Agency/Agent.cpp b/arangod/Agency/Agent.cpp
@@ -2061,13 +2061,13 @@ Store const& Agent::transient() const { return _transient; }
 /// Rebuild from persisted state
 void Agent::setPersistedState(VPackSlice compaction) {
   // Catch up with compacted state, this is only called at startup
-  _spearhead.loadFromVelocyPack(compaction);
+  _spearhead.setNodeValue(compaction);
 
   // Catch up with commit
   try {
     WRITE_LOCKER(oLocker, _outputLock);
     std::lock_guard guard{_waitForCV.mutex};
-    _readDB.loadFromVelocyPack(compaction);
+    _readDB.setNodeValue(compaction);
     _commitIndex = arangodb::basics::StringUtils::uint64(
         compaction.get(StaticStrings::KeyString).copyString());
     _local_index = _commitIndex.load(std::memory_order_relaxed);

diff --git a/arangod/Agency/CleanOutServer.cpp b/arangod/Agency/CleanOutServer.cpp
@@ -233,7 +233,7 @@ bool CleanOutServer::start(bool& aborts) {
 
   // Check that _to is not in `Target/CleanedServers`:
   VPackBuilder cleanedServersBuilder;
-  auto const& cleanedServersNode = _snapshot.hasAsNode(cleanedPrefix);
+  auto const& cleanedServersNode = _snapshot.get(cleanedPrefix);
   if (cleanedServersNode) {
     cleanedServersNode->toBuilder(cleanedServersBuilder);
   } else {
@@ -253,10 +253,10 @@ bool CleanOutServer::start(bool& aborts) {
 
   // Check that _to is not in `Target/FailedServers`:
   //  (this node is expected to NOT exists, so make test before processing
-  //   so that hasAsNode does not generate a warning log message)
+  //   so that get does not generate a warning log message)
   VPackBuilder failedServersBuilder;
   if (_snapshot.has(failedServersPrefix)) {
-    auto const& failedServersNode = _snapshot.hasAsNode(failedServersPrefix);
+    auto const& failedServersNode = _snapshot.get(failedServersPrefix);
     if (failedServersNode) {
       failedServersNode->toBuilder(failedServersBuilder);
     } else {
@@ -354,7 +354,7 @@ bool CleanOutServer::start(bool& aborts) {
       addPreconditionUnchanged(*pending, failedServersPrefix, failedServers);
       addPreconditionUnchanged(*pending, cleanedPrefix, cleanedServers);
       addPreconditionUnchanged(*pending, planVersion,
-                               _snapshot.get(planVersion)->slice());
+                               _snapshot.get(planVersion)->toBuilder().slice());
     }
   }  // array for transaction done
 
@@ -383,7 +383,7 @@ void CleanOutServer::scheduleJobsR2(std::shared_ptr<Builder>& trx,
   auto logs = logsChild;
 
   for (auto const& [logIdString, logNode] : *logs) {
-    auto logTarget = deserialize<replication2::agency::LogTarget>(*logNode);
+    auto logTarget = deserialize<replication2::agency::LogTarget>(logNode);
     bool removeServer = logTarget.participants.contains(_server);
 
     if (removeServer) {
@@ -436,8 +436,9 @@ bool CleanOutServer::scheduleMoveShards(std::shared_ptr<Builder>& trx) {
       for (auto const& shard : *collection.hasAsChildren("shards")) {
         // Only shards, which are affected
         int found = -1;
+
         int count = 0;
-        for (VPackSlice dbserver : VPackArrayIterator(shard.second->slice())) {
+        for (VPackSlice dbserver : *shard.second->getArray()) {
           if (dbserver.stringView() == _server) {
             found = count;
             break;
@@ -481,8 +482,7 @@ bool CleanOutServer::scheduleMoveShards(std::shared_ptr<Builder>& trx) {
           decltype(servers) serversCopy(servers);  // a copy
 
           // Only destinations, which are not already holding this shard
-          for (VPackSlice dbserver :
-               VPackArrayIterator(shard.second->slice())) {
+          for (VPackSlice dbserver : *shard.second->getArray()) {
             serversCopy.erase(
                 std::remove(serversCopy.begin(), serversCopy.end(),
                             dbserver.copyString()),

diff --git a/arangod/Agency/FailedFollower.cpp b/arangod/Agency/FailedFollower.cpp
@@ -153,13 +153,14 @@ bool FailedFollower::start(bool& aborts) {
   // Planned servers vector
   std::string planPath =
       planColPrefix + _database + "/" + _collection + "/shards/" + _shard;
-  auto plannedPair = _snapshot.hasAsSlice(planPath);
+  auto plannedPair = _snapshot.get(planPath);
   if (!plannedPair) {
     finish("", _shard, true,
            "Plan entry for collection " + _collection + " gone");
     return false;
   }
-  Slice const& planned = plannedPair.value();
+  auto builder = plannedPair->toBuilder();
+  Slice planned = builder.slice();
 
   // Now check if _server is still in this plan, note that it could have
   // been removed by RemoveFollower already, in which case we simply stop:
@@ -230,7 +231,7 @@ bool FailedFollower::start(bool& aborts) {
   {
     VPackArrayBuilder a(&todo);
     if (_jb == nullptr) {
-      auto const& jobIdNode = _snapshot.hasAsNode(toDoPrefix + _jobId);
+      auto const& jobIdNode = _snapshot.get(toDoPrefix + _jobId);
       if (jobIdNode) {
         jobIdNode->toBuilder(todo);
       } else {
@@ -310,9 +311,9 @@ bool FailedFollower::start(bool& aborts) {
               // "failoverCandidates":
               std::string foCandsPath = curPath.substr(0, curPath.size() - 7);
               foCandsPath += StaticStrings::FailoverCandidates;
-              auto foCands = this->_snapshot.hasAsSlice(foCandsPath);
+              auto foCands = this->_snapshot.hasAsBuilder(foCandsPath);
               if (foCands) {
-                addPreconditionUnchanged(job, foCandsPath, foCands.value());
+                addPreconditionUnchanged(job, foCandsPath, foCands->slice());
               }
             });
         // toServer not blocked

diff --git a/arangod/Agency/FailedLeader.cpp b/arangod/Agency/FailedLeader.cpp
@@ -99,7 +99,8 @@ void FailedLeader::rollback() {
   // Create new plan servers (exchange _to and _from)
   std::string planPath =
       planColPrefix + _database + "/" + _collection + "/shards/" + _shard;
-  auto planned = _snapshot.hasAsSlice(planPath).value();
+  auto plannedBuilder = _snapshot.hasAsBuilder(planPath).value();
+  auto planned = plannedBuilder.slice();
   std::shared_ptr<Builder> payload = nullptr;
 
   if (_status == PENDING) {  // Only payload if pending. Otherwise just fail.
@@ -218,19 +219,21 @@ bool FailedLeader::start(bool& aborts) {
   // Current servers vector
   std::string curPath =
       curColPrefix + _database + "/" + _collection + "/" + _shard;
-  auto current = _snapshot.hasAsSlice(curPath + "/servers").value();
+  auto currentBuilder = _snapshot.get(curPath + "/servers")->toBuilder();
+  auto current = currentBuilder.slice();
 
   // Planned servers vector
   std::string planPath =
       planColPrefix + _database + "/" + _collection + "/shards/" + _shard;
-  auto planned = _snapshot.hasAsSlice(planPath).value();
+  auto plannedBuilder = _snapshot.get(planPath)->toBuilder();
+  auto planned = plannedBuilder.slice();
 
   // Get todo entry
   Builder todo;
   {
     VPackArrayBuilder t(&todo);
     if (_jb == nullptr) {
-      auto const& jobIdNode = _snapshot.hasAsNode(toDoPrefix + _jobId);
+      auto const& jobIdNode = _snapshot.get(toDoPrefix + _jobId);
       if (jobIdNode) {
         jobIdNode->toBuilder(todo);
       } else {
@@ -371,9 +374,10 @@ bool FailedLeader::start(bool& aborts) {
               // "failoverCandidates":
               std::string foCandsPath = curPath.substr(0, curPath.size() - 7);
               foCandsPath += StaticStrings::FailoverCandidates;
-              auto foCands = this->_snapshot.hasAsSlice(foCandsPath);
+              auto foCands = this->_snapshot.hasAsBuilder(foCandsPath);
               if (foCands) {
-                addPreconditionUnchanged(pending, foCandsPath, *foCands);
+                addPreconditionUnchanged(pending, foCandsPath,
+                                         foCands->slice());
               }
             });
         // Destination server should not be blocked by another job
@@ -498,7 +502,7 @@ JOB_STATUS FailedLeader::status() {
   }
 
   std::string database, shard;
-  auto const& job = _snapshot.hasAsNode(pendingPrefix + _jobId);
+  auto const& job = _snapshot.get(pendingPrefix + _jobId);
   if (job) {
     auto tmp_database = job->hasAsString("database");
     auto tmp_shard = job->hasAsString("shard");
@@ -516,12 +520,12 @@ JOB_STATUS FailedLeader::status() {
   for (auto const& clone : clones(_snapshot, _database, _collection, _shard)) {
     auto sub = database + "/" + clone.collection;
     auto plan_slice =
-        _snapshot.hasAsSlice(planColPrefix + sub + "/shards/" + clone.shard);
-    auto cur_slice = _snapshot.hasAsSlice(curColPrefix + sub + "/" +
-                                          clone.shard + "/servers");
+        _snapshot.hasAsBuilder(planColPrefix + sub + "/shards/" + clone.shard);
+    auto cur_slice = _snapshot.hasAsBuilder(curColPrefix + sub + "/" +
+                                            clone.shard + "/servers");
     if (plan_slice && cur_slice &&
-        !basics::VelocyPackHelper::equal(plan_slice.value()[0],
-                                         cur_slice.value()[0], false)) {
+        !basics::VelocyPackHelper::equal(plan_slice->slice()[0],
+                                         cur_slice->slice()[0], false)) {
       LOG_TOPIC("0d8ca", DEBUG, Logger::SUPERVISION)
           << "FailedLeader waiting for " << sub << "/" << shard;
       break;

diff --git a/arangod/Agency/FailedServer.cpp b/arangod/Agency/FailedServer.cpp
@@ -112,9 +112,9 @@ bool FailedServer::start(bool& aborts) {
     return true;
   };
 
-  auto dbserverLock = _snapshot.hasAsSlice(blockedServersPrefix + _server);
+  auto dbserverLock = _snapshot.hasAsBuilder(blockedServersPrefix + _server);
   if (dbserverLock) {
-    auto s = *dbserverLock;
+    auto s = dbserverLock->slice();
     if (s.isArray()) {
       for (auto const& m : VPackArrayIterator(s)) {
         if (m.isString()) {
@@ -147,7 +147,7 @@ bool FailedServer::start(bool& aborts) {
   {
     VPackArrayBuilder t(&todo);
     if (_jb == nullptr) {
-      auto const& toDoJob = _snapshot.hasAsNode(toDoPrefix + _jobId);
+      auto const& toDoJob = _snapshot.get(toDoPrefix + _jobId);
       if (toDoJob) {
         toDoJob->toBuilder(todo);
       } else {
@@ -184,28 +184,19 @@ bool FailedServer::start(bool& aborts) {
         for (auto const& collptr : database.second->children()) {
           auto const& collection = *(collptr.second);
 
-          auto const& replicationFactorPair =
-              collection.hasAsNode(StaticStrings::ReplicationFactor);
-          if (replicationFactorPair) {
-            VPackSlice const replicationFactor = replicationFactorPair->slice();
+          auto const& replicationFactor =
+              collection.get(StaticStrings::ReplicationFactor);
+          if (replicationFactor) {
             uint64_t number = 1;
             bool isSatellite = false;
 
-            if (replicationFactor.isString() &&
-                replicationFactor.compareString(StaticStrings::Satellite) ==
-                    0) {
+            if (replicationFactor->getStringView() ==
+                StaticStrings::Satellite) {
               isSatellite = true;  // do nothing - number =
                                    // Job::availableServers(_snapshot).size();
-            } else if (replicationFactor.isNumber()) {
-              try {
-                number = replicationFactor.getNumber<uint64_t>();
-              } catch (...) {
-                LOG_TOPIC("f5290", ERR, Logger::SUPERVISION)
-                    << "Failed to read replicationFactor. job: " << _jobId
-                    << " " << collection.hasAsString("id").value();
-                continue;
-              }
-
+            } else if (auto maybeNumber = replicationFactor->getUInt();
+                       maybeNumber) {
+              number = *maybeNumber;
               if (number == 1) {
                 continue;
               }
@@ -220,7 +211,7 @@ bool FailedServer::start(bool& aborts) {
             for (auto const& shard : *collection.hasAsChildren("shards")) {
               size_t pos = 0;
 
-              for (VPackSlice it : VPackArrayIterator(shard.second->slice())) {
+              for (auto it : *shard.second->getArray()) {
                 auto dbs = it.copyString();
 
                 if (dbs == _server || dbs == "_" + _server) {

diff --git a/arangod/Agency/Helpers.cpp b/arangod/Agency/Helpers.cpp
@@ -31,13 +31,13 @@ namespace arangodb::consensus {
 bool isReplicationTwoDB(Node::Children const& databases,
                         std::string const& dbName) {
   auto it = databases.find(dbName);
-  if (it == databases.end()) {
+  if (it == nullptr) {
     // this should actually never happen, but if it does we simply claim that
     // this is an old replication 1 DB.
     return false;
   }
 
-  if (auto v = it->second->hasAsString(StaticStrings::ReplicationVersion); v) {
+  if (auto v = (*it)->hasAsString(StaticStrings::ReplicationVersion); v) {
     auto res = replication::parseVersion(v.value());
     return res.ok() && res.get() == replication::Version::TWO;
   }