Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

prevent duplicate actions from popping up (#9453)

This commit is contained in:
Jan 2019-07-16 10:02:59 +02:00 committed by GitHub
parent 1baba16dad
commit 83227cb7a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 55 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGELOG
View File

@ -1,6 +1,8 @@
v3.4.8 (XXXX-XX-XX)
-------------------
* Prevent rare cases of duplicate DDL actions being executed by Maintenance.
* coordinator code was reporting rocksdb error codes, but not the associated detail message.
Corrected.
@ -9,6 +11,7 @@ v3.4.8 (XXXX-XX-XX)
* Fixed an error condition in which an ex-leader for a short still believed
to be the leader and wrongly reported to Current.
v3.4.7 (2019-07-02)
-------------------

4
arangod/Cluster/ActionBase.cpp
View File

@ -121,12 +121,12 @@ void ActionBase::createPreAction(std::shared_ptr<ActionDescription> const& descr
/// @brief Retrieve pointer to action that should run before this one
std::shared_ptr<Action> ActionBase::getPreAction() {
return (_preAction != nullptr) ? _feature.findAction(_preAction) : nullptr;
return (_preAction != nullptr) ? _feature.findFirstNotDoneAction(_preAction) : nullptr;
}
/// @brief Retrieve pointer to action that should run after this one
std::shared_ptr<Action> ActionBase::getPostAction() {
return (_postAction != nullptr) ? _feature.findAction(_postAction) : nullptr;
return (_postAction != nullptr) ? _feature.findFirstNotDoneAction(_postAction) : nullptr;
}
// FIXMEMAINTENANCE: Code path could corrupt registry object because

92
arangod/Cluster/MaintenanceFeature.cpp
View File

@ -43,6 +43,15 @@ using namespace arangodb::maintenance;
const uint32_t MaintenanceFeature::minThreadLimit = 2;
const uint32_t MaintenanceFeature::maxThreadLimit = 64;
namespace {
bool findNotDoneActions(std::shared_ptr<maintenance::Action> const& action) {
return !action->done();
}
} // namespace
MaintenanceFeature::MaintenanceFeature(application_features::ApplicationServer& server)
: ApplicationFeature(server, "Maintenance"),
_forceActivation(false),
@ -195,10 +204,10 @@ Result MaintenanceFeature::addAction(std::shared_ptr<maintenance::Action> newAct
size_t action_hash = newAction->hash();
WRITE_LOCKER(wLock, _actionRegistryLock);
std::shared_ptr<Action> curAction = findActionHashNoLock(action_hash);
std::shared_ptr<Action> curAction = findFirstActionHashNoLock(action_hash, ::findNotDoneActions);
// similar action not in the queue (or at least no longer viable)
if (curAction == nullptr || curAction->done()) {
if (!curAction) {
if (newAction && newAction->ok()) {
// Register action only if construction was ok
registerAction(newAction, executeNow);
@ -240,16 +249,13 @@ Result MaintenanceFeature::addAction(std::shared_ptr<maintenance::ActionDescript
try {
std::shared_ptr<Action> newAction;
// is there a known name field
auto find_it = description->get("name");
size_t action_hash = description->hash();
WRITE_LOCKER(wLock, _actionRegistryLock);
std::shared_ptr<Action> curAction = findActionHashNoLock(action_hash);
std::shared_ptr<Action> curAction = findFirstActionHashNoLock(action_hash, ::findNotDoneActions);
// similar action not in the queue (or at least no longer viable)
if (!curAction || curAction->done()) {
if (!curAction) {
newAction = createAndRegisterAction(description, executeNow);
if (!newAction || !newAction->ok()) {
@ -354,55 +360,45 @@ std::shared_ptr<Action> MaintenanceFeature::createAndRegisterAction(
return newAction;
}
std::shared_ptr<Action> MaintenanceFeature::findAction(std::shared_ptr<ActionDescription> const description) {
return findActionHash(description->hash());
std::shared_ptr<Action> MaintenanceFeature::findFirstNotDoneAction(std::shared_ptr<ActionDescription> const& description) {
return findFirstActionHash(description->hash(), ::findNotDoneActions);
}
std::shared_ptr<Action> MaintenanceFeature::findActionHash(size_t hash) {
std::shared_ptr<Action> MaintenanceFeature::findFirstActionHash(size_t hash,
std::function<bool(std::shared_ptr<maintenance::Action> const&)> const& predicate) {
READ_LOCKER(rLock, _actionRegistryLock);
return findActionHashNoLock(hash);
} // MaintenanceFeature::findActionHash
return findFirstActionHashNoLock(hash, predicate);
}
std::shared_ptr<Action> MaintenanceFeature::findActionHashNoLock(size_t hash) {
std::shared_ptr<Action> MaintenanceFeature::findFirstActionHashNoLock(size_t hash,
std::function<bool(std::shared_ptr<maintenance::Action> const&)> const& predicate) {
// assert to test lock held?
std::shared_ptr<Action> ret_ptr;
for (auto action_it = _actionRegistry.begin();
_actionRegistry.end() != action_it && !ret_ptr; ++action_it) {
if ((*action_it)->hash() == hash) {
ret_ptr = *action_it;
} // if
} // for
return ret_ptr;
} // MaintenanceFeature::findActionHashNoLock
for (auto const& action : _actionRegistry) {
if (action->hash() == hash && predicate(action)) {
return action;
}
}
return std::shared_ptr<Action>();
}
std::shared_ptr<Action> MaintenanceFeature::findActionId(uint64_t id) {
READ_LOCKER(rLock, _actionRegistryLock);
return findActionIdNoLock(id);
} // MaintenanceFeature::findActionId
}
std::shared_ptr<Action> MaintenanceFeature::findActionIdNoLock(uint64_t id) {
// assert to test lock held?
std::shared_ptr<Action> ret_ptr;
for (auto action_it = _actionRegistry.begin();
_actionRegistry.end() != action_it && !ret_ptr; ++action_it) {
if ((*action_it)->id() == id) {
// should we return the first match here or the last match?
// if first, we could simply add a break
ret_ptr = *action_it;
} // if
} // for
return ret_ptr;
} // MaintenanceFeature::findActionIdNoLock
for (auto const& action : _actionRegistry) {
if (action->id() == id) {
return action;
}
}
return std::shared_ptr<Action>();
}
std::shared_ptr<Action> MaintenanceFeature::findReadyAction(std::unordered_set<std::string> const& labels) {
std::shared_ptr<Action> ret_ptr;
@ -455,7 +451,7 @@ std::shared_ptr<Action> MaintenanceFeature::findReadyAction(std::unordered_set<s
return ret_ptr;
} // MaintenanceFeature::findReadyAction
}
VPackBuilder MaintenanceFeature::toVelocyPack() const {
VPackBuilder vb;
@ -649,20 +645,6 @@ arangodb::Result MaintenanceFeature::storeIndexError(
return Result();
}
arangodb::Result MaintenanceFeature::indexErrors(
std::string const& database, std::string const& collection, std::string const& shard,
std::map<std::string, std::shared_ptr<VPackBuffer<uint8_t>>>& error) const {
std::string key = database + SLASH + collection + SLASH + shard;
MUTEX_LOCKER(guard, _ieLock);
auto const& it = _indexErrors.find(key);
if (it != _indexErrors.end()) {
error = it->second;
}
return Result();
}
template <typename T>
std::ostream& operator<<(std::ostream& os, std::set<T> const& st) {
size_t j = 0;

36
arangod/Cluster/MaintenanceFeature.h
View File

@ -153,10 +153,10 @@ class MaintenanceFeature : public application_features::ApplicationFeature {
uint32_t getSecondsActionsBlock() const { return _secondsActionsBlock; };
/**
* @brief Find and return found action or nullptr
* @brief Find and return first found not-done action or nullptr
* @param desc Description of sought action
*/
std::shared_ptr<maintenance::Action> findAction(std::shared_ptr<maintenance::ActionDescription> const desc);
std::shared_ptr<maintenance::Action> findFirstNotDoneAction(std::shared_ptr<maintenance::ActionDescription> const& desc);
/**
* @brief add index error to bucket
@ -173,20 +173,6 @@ class MaintenanceFeature : public application_features::ApplicationFeature {
std::string const& shard, std::string const& indexId,
std::shared_ptr<VPackBuffer<uint8_t>> error);
/**
* @brief get all pending index errors for a specific shard
*
* @param database database
* @param collection collection
* @param shard shard
* @param errors errrors map returned to caller
*
* @return success
*/
arangodb::Result indexErrors(
std::string const& database, std::string const& collection, std::string const& shard,
std::map<std::string, std::shared_ptr<VPackBuffer<uint8_t>>>& errors) const;
/**
* @brief remove 1+ errors from index error bucket
* Errors are removed by phaseOne, as soon as indexes no longer in plan
@ -312,17 +298,19 @@ class MaintenanceFeature : public application_features::ApplicationFeature {
*/
void delShardVersion(std::string const& shardId);
protected:
private:
/// @brief common code used by multiple constructors
void init();
/// @brief Search for action by hash
/// @return shared pointer to action object if exists, _actionRegistry.end() if not
std::shared_ptr<maintenance::Action> findActionHash(size_t hash);
/// @brief Search for first action matching hash and predicate
/// @return shared pointer to action object if exists, empty shared_ptr if not
std::shared_ptr<maintenance::Action> findFirstActionHash(size_t hash,
std::function<bool(std::shared_ptr<maintenance::Action> const&)> const& predicate);
/// @brief Search for action by hash (but lock already held by caller)
/// @return shared pointer to action object if exists, nullptr if not
std::shared_ptr<maintenance::Action> findActionHashNoLock(size_t hash);
/// @brief Search for first action matching hash and predicate (with lock already held by caller)
/// @return shared pointer to action object if exists, empty shared_ptr if not
std::shared_ptr<maintenance::Action> findFirstActionHashNoLock(size_t hash,
std::function<bool(std::shared_ptr<maintenance::Action> const&)> const& predicate);
/// @brief Search for action by Id
/// @return shared pointer to action object if exists, nullptr if not
@ -332,6 +320,8 @@ class MaintenanceFeature : public application_features::ApplicationFeature {
/// @return shared pointer to action object if exists, nullptr if not
std::shared_ptr<maintenance::Action> findActionIdNoLock(uint64_t hash);
protected:
/// @brief option for forcing this feature to always be enable - used by the catch tests
bool _forceActivation;