Preserve runtime objects in a tmp file for the entire validation process

Given that the internal `config::Update` cluster events are using this
as well to create received runtime objects, we don't want to persist
first the conf file and the load and validate it with `CompileFile`.
Otherwise, we are forced to remove the newly created file whenever we
can't validate, commit or activate it. This also would also have the
downside that two cluster events for the same object arriving at the
same moment from two different endpoints would result in two different
threads simultaneously creating and loading the same config file -
whereby only one of the surpasses the validation, while the other is
facing an object `re-definition` error and tries to remove that config
file it mistakenly thinks it has created. As a consequence, an object
successfully created by the former is implicitly deleted by the latter
thread, causing the objects to mysteriously disappear.
This commit is contained in:
Yonas Habteab 2024-01-25 17:06:12 +01:00
parent e936c43e89
commit 008fcd1744
1 changed files with 17 additions and 11 deletions

View File

@ -5,6 +5,7 @@
#include "remote/apilistener.hpp"
#include "config/configcompiler.hpp"
#include "config/configitem.hpp"
#include "base/atomic-file.hpp"
#include "base/configwriter.hpp"
#include "base/exception.hpp"
#include "base/dependencygraph.hpp"
@ -198,13 +199,21 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
return false;
}
// AtomicFile doesn't create not yet existing directories, so we have to do it by ourselves.
Utility::MkDirP(Utility::DirName(path), 0700);
std::ofstream fp(path.CStr(), std::ofstream::out | std::ostream::trunc);
// Using AtomicFile guarantees that two different threads simultaneously creating and loading the same
// configuration file do not interfere with each other, as the configuration is stored in a unique temp file.
// When one thread fails to pass object validation, it only deletes its temporary file and does not affect
// the other thread in any way.
AtomicFile fp(path, 0644);
fp << config;
fp.close();
// Flush the output buffer to catch any errors ASAP and handle them accordingly!
// Note: AtomicFile places these configs in a temp file and will be automatically
// discarded when it is not committed before going out of scope.
fp.flush();
std::unique_ptr<Expression> expr = ConfigCompiler::CompileFile(path, String(), "_api");
std::unique_ptr<Expression> expr = ConfigCompiler::CompileText(path, config, String(), "_api");
try {
ActivationScope ascope;
@ -225,9 +234,7 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
if (!ConfigItem::CommitItems(ascope.GetContext(), upq, newItems, true)) {
if (errors) {
Log(LogNotice, "ConfigObjectUtility")
<< "Failed to commit config item '" << fullName << "'. Aborting and removing config path '" << path << "'.";
Utility::Remove(path);
<< "Failed to commit config item '" << fullName << "'.";
for (const boost::exception_ptr& ex : upq.GetExceptions()) {
errors->Add(DiagnosticInformation(ex, false));
@ -248,9 +255,7 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
if (!ConfigItem::ActivateItems(newItems, true, false, false, cookie)) {
if (errors) {
Log(LogNotice, "ConfigObjectUtility")
<< "Failed to activate config object '" << fullName << "'. Aborting and removing config path '" << path << "'.";
Utility::Remove(path);
<< "Failed to activate config object '" << fullName << "'.";
for (const boost::exception_ptr& ex : upq.GetExceptions()) {
errors->Add(DiagnosticInformation(ex, false));
@ -275,6 +280,9 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
ConfigObject::Ptr obj = ctype->GetObject(fullName);
if (obj) {
// Object has surpassed the compiling/validation processes, we can safely commit the file!
fp.Commit();
Log(LogInformation, "ConfigObjectUtility")
<< "Created and activated object '" << fullName << "' of type '" << type->GetName() << "'.";
} else {
@ -283,8 +291,6 @@ bool ConfigObjectUtility::CreateObject(const Type::Ptr& type, const String& full
}
} catch (const std::exception& ex) {
Utility::Remove(path);
if (errors)
errors->Add(DiagnosticInformation(ex, false));