try again in case of a conflict when updating users (#7615)

arangod/Auth/UserManager.cpp

@@ -334,7 +334,8 @@ Result auth::UserManager::storeUserInternal(auth::User const& entry, bool replac
       }
 #endif
     } else if (res.is(TRI_ERROR_ARANGO_CONFLICT)) {  // user was outdated
-      _userCache.erase(entry.username());
+      // we didn't succeed in updating the user, so we must not remove the
+      // user from the cache here. however, we should trigger a reload here
       triggerLocalReload();
       LOG_TOPIC(WARN, Logger::AUTHENTICATION)
           << "Cannot update user due to conflict";
@@ -562,6 +563,7 @@ Result auth::UserManager::accessUser(std::string const& user,
   }
 
   loadFromDB();
+
   READ_LOCKER(readGuard, _userCacheLock);
   UserMap::iterator const& it = _userCache.find(user);
   if (it != _userCache.end()) {
@@ -574,8 +576,8 @@ bool auth::UserManager::userExists(std::string const& user) {
   if (user.empty()) {
     return false;
   }
-  loadFromDB();
   
+  loadFromDB();
   READ_LOCKER(readGuard, _userCacheLock);
   UserMap::iterator const& it = _userCache.find(user);
   return it != _userCache.end();

arangod/VocBase/Methods/Collections.cpp

@@ -154,8 +154,7 @@ Result methods::Collections::lookup(TRI_vocbase_t* vocbase,
         func(coll);
 
         return Result();
-      }
+      } else {
-      else {
         return Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
       }
     } catch (basics::Exception const& ex) {
@@ -267,10 +266,25 @@ Result Collections::create(TRI_vocbase_t* vocbase, std::string const& name,
       if (name[0] != '_' && um != nullptr && exe != nullptr &&
           !exe->isSuperuser()) {
         // this should not fail, we can not get here without database RW access
-        um->updateUser(ExecContext::CURRENT->user(), [&](auth::User& entry) {
+        // however, there may be races for updating the users account, so we try a
+        // few times in case of a conflict
+        int tries = 0;
+        while (true) {
+          Result r = um->updateUser(ExecContext::CURRENT->user(), [&](auth::User& entry) {
             entry.grantCollection(vocbase->name(), name, auth::Level::RW);
             return TRI_ERROR_NO_ERROR;
           });
+          if (r.ok() || r.is(TRI_ERROR_USER_NOT_FOUND)) {
+            // it seems to be allowed to created collections with an unknown user
+            break;
+          }
+          if (!r.is(TRI_ERROR_ARANGO_CONFLICT) || ++tries == 10) {
+            LOG_TOPIC(WARN, Logger::FIXME) << "Updating user failed with error: " << r.errorMessage() << ". giving up!";
+            return r;
+          }
+          // try again in case of conflict
+          LOG_TOPIC(TRACE, Logger::FIXME) << "Updating user failed with error: " << r.errorMessage() << ". trying again";
+        }
       }
 
       // reload otherwise collection might not be in yet
@@ -286,10 +300,25 @@ Result Collections::create(TRI_vocbase_t* vocbase, std::string const& name,
       if (name[0] != '_' && um != nullptr && exe != nullptr &&
           !exe->isSuperuser()) {
         // this should not fail, we can not get here without database RW access
-        um->updateUser(ExecContext::CURRENT->user(), [&](auth::User& u) {
+        // however, there may be races for updating the users account, so we try a
-          u.grantCollection(vocbase->name(), name, auth::Level::RW);
+        // few times in case of a conflict
+        int tries = 0;
+        while (true) {
+          Result r = um->updateUser(ExecContext::CURRENT->user(), [&](auth::User& entry) {
+            entry.grantCollection(vocbase->name(), name, auth::Level::RW);
             return TRI_ERROR_NO_ERROR;
           });
+          if (r.ok() || r.is(TRI_ERROR_USER_NOT_FOUND)) {
+            // it seems to be allowed to created collections with an unknown user
+            break;
+          }
+          if (!r.is(TRI_ERROR_ARANGO_CONFLICT) || ++tries == 10) {
+            LOG_TOPIC(WARN, Logger::FIXME) << "Updating user failed with error: " << r.errorMessage() << ". giving up!";
+            return r;
+          }
+          // try again in case of conflict
+          LOG_TOPIC(TRACE, Logger::FIXME) << "Updating user failed with error: " << r.errorMessage() << ". trying again";
+        }
       }
 
       func(col);
@@ -567,7 +596,7 @@ Result Collections::drop(TRI_vocbase_t* vocbase, LogicalCollection* coll,
   }
 
   TRI_ASSERT(coll);
-  auto& dbname = coll->vocbase().name();
+  auto const& dbname = coll->vocbase().name();
   std::string const collName = coll->name();
 
   Result res;
@@ -590,9 +619,23 @@ Result Collections::drop(TRI_vocbase_t* vocbase, LogicalCollection* coll,
 
   auth::UserManager* um = AuthenticationFeature::instance()->userManager();
   if (res.ok() && um != nullptr) {
-    um->enumerateUsers([&](auth::User& entry) -> bool {
+    int tries = 0;
+    while (true) {
+      res = um->enumerateUsers([&](auth::User& entry) -> bool {
         return entry.removeCollection(dbname, collName);
       });
+
+      if (res.ok() || !res.is(TRI_ERROR_ARANGO_CONFLICT)) {
+        break;
+      }
+      
+      if (++tries == 10) {
+        LOG_TOPIC(WARN, Logger::FIXME) << "Enumerating users failed with " << res.errorMessage() << ". giving up!";
+        break;
+      }
+      // try again in case of conflict
+      LOG_TOPIC(TRACE, Logger::FIXME) << "Enumerating users failed with error: " << res.errorMessage() << ". trying again";
+    }
   }
   return res;
 }