Skip to content

Change RedisConnection::Query::value_type from String to std::variant<const char*,String> #10391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/base/string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ String::String(const char *data)
: m_Data(data)
{ }

String::String(const std::string_view& data)
: m_Data(data)
{ }

String::String(std::string data)
: m_Data(std::move(data))
{ }
Expand Down Expand Up @@ -139,6 +143,16 @@ String::operator boost::beast::string_view() const
return boost::beast::string_view(m_Data);
}

/**
* Conversion function to std::string_view.
*
* @return An std::string_view representing this string.
*/
String::operator std::string_view() const
{
return std::string_view(m_Data);
}

const char *String::CStr() const
{
return m_Data.c_str();
Expand Down
3 changes: 3 additions & 0 deletions lib/base/string.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <boost/utility/string_view.hpp>
#include <functional>
#include <string>
#include <string_view>
#include <iosfwd>

namespace icinga {
Expand Down Expand Up @@ -41,6 +42,7 @@ class String

String() = default;
String(const char *data);
String(const std::string_view& data);
String(std::string data);
String(String::SizeType n, char c);
String(const String& other);
Expand Down Expand Up @@ -75,6 +77,7 @@ class String

operator const std::string&() const;
operator boost::beast::string_view() const;
operator std::string_view() const;

const char *CStr() const;

Expand Down
74 changes: 41 additions & 33 deletions lib/icingadb/icingadb-objects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,12 @@ void IcingaDB::UpdateAllConfigObjects()
String configObject = m_PrefixConfigObject + lcType;

// Skimmed away attributes and checksums HMSETs' keys and values by Redis key.
std::map<String, std::vector<std::vector<String>>> ourContentRaw {{configCheckSum, {}}, {configObject, {}}};
std::map<String, RedisConnection::Queries> ourContentRaw {{configCheckSum, {}}, {configObject, {}}};
std::mutex ourContentMutex;

upqObjectType.ParallelFor(objectChunks, [&](decltype(objectChunks)::const_reference chunk) {
std::map<String, std::vector<String>> hMSets;
std::vector<String> hostZAdds = {"ZADD", "icinga:nextupdate:host"}, serviceZAdds = {"ZADD", "icinga:nextupdate:service"};
std::map<String, RedisConnection::Query> hMSets;
RedisConnection::Query hostZAdds = {"ZADD", "icinga:nextupdate:host"}, serviceZAdds = {"ZADD", "icinga:nextupdate:service"};

auto skimObjects ([&]() {
std::lock_guard<std::mutex> l (ourContentMutex);
Expand Down Expand Up @@ -342,7 +342,7 @@ void IcingaDB::UpdateAllConfigObjects()
zAdds->emplace_back(GetObjectIdentifier(checkable));

if (zAdds->size() >= 102u) {
std::vector<String> header (zAdds->begin(), zAdds->begin() + 2u);
RedisConnection::Query header (zAdds->begin(), zAdds->begin() + 2u);

rcon->FireAndForgetQuery(std::move(*zAdds), Prio::CheckResult);

Expand Down Expand Up @@ -383,7 +383,15 @@ void IcingaDB::UpdateAllConfigObjects()
upqObjectType.Enqueue([&]() {
for (auto& hMSet : source.second) {
for (decltype(hMSet.size()) i = 0, stop = hMSet.size() - 1u; i < stop; i += 2u) {
dest.emplace(std::move(hMSet[i]), std::move(hMSet[i + 1u]));
auto variantToString = [](RedisConnection::QueryArg v) -> String {
if (auto str (std::get_if<String>(&v.GetData())); str) {
return std::move(*str);
}

return std::get<std::string_view>(v.GetData());
};

dest.emplace(variantToString(std::move(hMSet[i])), variantToString(std::move(hMSet[i + 1u])));
}

hMSet.clear();
Expand All @@ -398,7 +406,7 @@ void IcingaDB::UpdateAllConfigObjects()

auto& ourCheckSums (ourContent[configCheckSum]);
auto& ourObjects (ourContent[configObject]);
std::vector<String> setChecksum, setObject, delChecksum, delObject;
RedisConnection::Query setChecksum, setObject, delChecksum, delObject;

auto redisCurrent (redisCheckSums.begin());
auto redisEnd (redisCheckSums.end());
Expand All @@ -411,12 +419,12 @@ void IcingaDB::UpdateAllConfigObjects()
setChecksum.insert(setChecksum.begin(), {"HMSET", configCheckSum});
setObject.insert(setObject.begin(), {"HMSET", configObject});

std::vector<std::vector<String>> transaction;
RedisConnection::Queries transaction;

transaction.emplace_back(std::vector<String>{"MULTI"});
transaction.emplace_back(RedisConnection::Query{"MULTI"});
transaction.emplace_back(std::move(setChecksum));
transaction.emplace_back(std::move(setObject));
transaction.emplace_back(std::vector<String>{"EXEC"});
transaction.emplace_back(RedisConnection::Query{"EXEC"});

setChecksum.clear();
setObject.clear();
Expand All @@ -430,12 +438,12 @@ void IcingaDB::UpdateAllConfigObjects()
delChecksum.insert(delChecksum.begin(), {"HDEL", configCheckSum});
delObject.insert(delObject.begin(), {"HDEL", configObject});

std::vector<std::vector<String>> transaction;
RedisConnection::Queries transaction;

transaction.emplace_back(std::vector<String>{"MULTI"});
transaction.emplace_back(RedisConnection::Query{"MULTI"});
transaction.emplace_back(std::move(delChecksum));
transaction.emplace_back(std::move(delObject));
transaction.emplace_back(std::vector<String>{"EXEC"});
transaction.emplace_back(RedisConnection::Query{"EXEC"});

delChecksum.clear();
delObject.clear();
Expand Down Expand Up @@ -563,7 +571,7 @@ std::vector<std::vector<intrusive_ptr<ConfigObject>>> IcingaDB::ChunkObjects(std
}

void IcingaDB::DeleteKeys(const RedisConnection::Ptr& conn, const std::vector<String>& keys, RedisConnection::QueryPriority priority) {
std::vector<String> query = {"DEL"};
RedisConnection::Query query = {"DEL"};
for (auto& key : keys) {
query.emplace_back(key);
}
Expand Down Expand Up @@ -635,7 +643,7 @@ static ConfigObject::Ptr GetObjectByName(const String& name)
return ConfigObject::GetObject<ConfigType>(name);
}

void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const String typeName, std::map<String, std::vector<String>>& hMSets,
void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const String typeName, std::map<String, RedisConnection::Query>& hMSets,
std::vector<Dictionary::Ptr>& runtimeUpdates, bool runtimeUpdate)
{
String objectKey = GetObjectIdentifier(object);
Expand Down Expand Up @@ -691,12 +699,12 @@ void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const S
auto id (HashValue(new Array({m_EnvironmentId, actionUrl})));

if (runtimeUpdate || m_DumpedGlobals.ActionUrl.IsNew(id)) {
actionUrls.emplace_back(std::move(id));
actionUrls.emplace_back(id);
Dictionary::Ptr data = new Dictionary({{"environment_id", m_EnvironmentId}, {"action_url", actionUrl}});
actionUrls.emplace_back(JsonEncode(data));

if (runtimeUpdate) {
AddObjectDataToRuntimeUpdates(runtimeUpdates, actionUrls.at(actionUrls.size() - 2u), m_PrefixConfigObject + "action:url", data);
AddObjectDataToRuntimeUpdates(runtimeUpdates, id, m_PrefixConfigObject + "action:url", data);
}
}
}
Expand All @@ -706,12 +714,12 @@ void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const S
auto id (HashValue(new Array({m_EnvironmentId, notesUrl})));

if (runtimeUpdate || m_DumpedGlobals.NotesUrl.IsNew(id)) {
notesUrls.emplace_back(std::move(id));
notesUrls.emplace_back(id);
Dictionary::Ptr data = new Dictionary({{"environment_id", m_EnvironmentId}, {"notes_url", notesUrl}});
notesUrls.emplace_back(JsonEncode(data));

if (runtimeUpdate) {
AddObjectDataToRuntimeUpdates(runtimeUpdates, notesUrls.at(notesUrls.size() - 2u), m_PrefixConfigObject + "notes:url", data);
AddObjectDataToRuntimeUpdates(runtimeUpdates, id, m_PrefixConfigObject + "notes:url", data);
}
}
}
Expand All @@ -721,12 +729,12 @@ void IcingaDB::InsertObjectDependencies(const ConfigObject::Ptr& object, const S
auto id (HashValue(new Array({m_EnvironmentId, iconImage})));

if (runtimeUpdate || m_DumpedGlobals.IconImage.IsNew(id)) {
iconImages.emplace_back(std::move(id));
iconImages.emplace_back(id);
Dictionary::Ptr data = new Dictionary({{"environment_id", m_EnvironmentId}, {"icon_image", iconImage}});
iconImages.emplace_back(JsonEncode(data));

if (runtimeUpdate) {
AddObjectDataToRuntimeUpdates(runtimeUpdates, iconImages.at(iconImages.size() - 2u), m_PrefixConfigObject + "icon:image", data);
AddObjectDataToRuntimeUpdates(runtimeUpdates, id, m_PrefixConfigObject + "icon:image", data);
}
}
}
Expand Down Expand Up @@ -1333,7 +1341,7 @@ void IcingaDB::UpdateState(const Checkable::Ptr& checkable, StateUpdate mode)
if (mode & StateUpdate::RuntimeOnly) {
ObjectLock olock(stateAttrs);

std::vector<String> streamadd({
RedisConnection::Query streamadd({
"XADD", "icinga:runtime:state", "MAXLEN", "~", "1000000", "*",
"runtime_type", "upsert",
"redis_key", redisStateKey,
Expand Down Expand Up @@ -1466,7 +1474,7 @@ void IcingaDB::SendConfigUpdate(const ConfigObject::Ptr& object, bool runtimeUpd

String typeName = GetLowerCaseTypeNameDB(object);

std::map<String, std::vector<String>> hMSets;
std::map<String, RedisConnection::Query> hMSets;
std::vector<Dictionary::Ptr> runtimeUpdates;

CreateConfigUpdate(object, typeName, hMSets, runtimeUpdates, runtimeUpdate);
Expand Down Expand Up @@ -1806,7 +1814,7 @@ bool IcingaDB::PrepareObject(const ConfigObject::Ptr& object, Dictionary::Ptr& a
* icinga:config:object:downtime) need to be prepended. There is nothing to indicate success or failure.
*/
void
IcingaDB::CreateConfigUpdate(const ConfigObject::Ptr& object, const String typeName, std::map<String, std::vector<String>>& hMSets,
IcingaDB::CreateConfigUpdate(const ConfigObject::Ptr& object, const String typeName, std::map<String, RedisConnection::Query>& hMSets,
std::vector<Dictionary::Ptr>& runtimeUpdates, bool runtimeUpdate)
{
/* TODO: This isn't essentially correct as we don't keep track of config objects ourselves. This would avoid duplicated config updates at startup.
Expand Down Expand Up @@ -1969,7 +1977,7 @@ void IcingaDB::SendStateChange(const ConfigObject::Ptr& object, const CheckResul
Array::Ptr rawId = new Array({m_EnvironmentId, object->GetName()});
rawId->Add(eventTs);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:state", "*",
"id", HashValue(rawId),
"environment_id", m_EnvironmentId,
Expand Down Expand Up @@ -2053,7 +2061,7 @@ void IcingaDB::SendSentNotification(

auto notificationHistoryId (HashValue(rawId));

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:notification", "*",
"id", notificationHistoryId,
"environment_id", m_EnvironmentId,
Expand Down Expand Up @@ -2117,7 +2125,7 @@ void IcingaDB::SendStartedDowntime(const Downtime::Ptr& downtime)
/* Update checkable state as in_downtime may have changed. */
UpdateState(checkable, StateUpdate::Full);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:downtime", "*",
"downtime_id", GetObjectIdentifier(downtime),
"environment_id", m_EnvironmentId,
Expand Down Expand Up @@ -2207,7 +2215,7 @@ void IcingaDB::SendRemovedDowntime(const Downtime::Ptr& downtime)
/* Update checkable state as in_downtime may have changed. */
UpdateState(checkable, StateUpdate::Full);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:downtime", "*",
"downtime_id", GetObjectIdentifier(downtime),
"environment_id", m_EnvironmentId,
Expand Down Expand Up @@ -2289,7 +2297,7 @@ void IcingaDB::SendAddedComment(const Comment::Ptr& comment)
Service::Ptr service;
tie(host, service) = GetHostService(checkable);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:comment", "*",
"comment_id", GetObjectIdentifier(comment),
"environment_id", m_EnvironmentId,
Expand Down Expand Up @@ -2361,7 +2369,7 @@ void IcingaDB::SendRemovedComment(const Comment::Ptr& comment)
Service::Ptr service;
tie(host, service) = GetHostService(checkable);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:comment", "*",
"comment_id", GetObjectIdentifier(comment),
"environment_id", m_EnvironmentId,
Expand Down Expand Up @@ -2424,7 +2432,7 @@ void IcingaDB::SendFlappingChange(const Checkable::Ptr& checkable, double change
Service::Ptr service;
tie(host, service) = GetHostService(checkable);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:flapping", "*",
"environment_id", m_EnvironmentId,
"host_id", GetObjectIdentifier(host),
Expand Down Expand Up @@ -2519,7 +2527,7 @@ void IcingaDB::SendAcknowledgementSet(const Checkable::Ptr& checkable, const Str
/* Update checkable state as is_acknowledged may have changed. */
UpdateState(checkable, StateUpdate::Full);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:acknowledgement", "*",
"environment_id", m_EnvironmentId,
"host_id", GetObjectIdentifier(host),
Expand Down Expand Up @@ -2577,7 +2585,7 @@ void IcingaDB::SendAcknowledgementCleared(const Checkable::Ptr& checkable, const
/* Update checkable state as is_acknowledged may have changed. */
UpdateState(checkable, StateUpdate::Full);

std::vector<String> xAdd ({
RedisConnection::Query xAdd ({
"XADD", "icinga:history:stream:acknowledgement", "*",
"environment_id", m_EnvironmentId,
"host_id", GetObjectIdentifier(host),
Expand Down Expand Up @@ -3327,7 +3335,7 @@ void IcingaDB::DeleteRelationship(const String& id, const String& redisKeyWithou

String redisKey = m_PrefixConfigObject + redisKeyWithoutPrefix;

std::vector<std::vector<String>> queries;
RedisConnection::Queries queries;

if (hasChecksum) {
queries.push_back({"HDEL", m_PrefixConfigCheckSum + redisKeyWithoutPrefix, id});
Expand Down
2 changes: 1 addition & 1 deletion lib/icingadb/icingadb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ void IcingaDB::PublishStats()
status->Set("timestamp", TimestampToMilliseconds(Utility::GetTime()));
status->Set("icingadb_environment", m_EnvironmentId);

std::vector<String> query {"XADD", "icinga:stats", "MAXLEN", "1", "*"};
RedisConnection::Query query {"XADD", "icinga:stats", "MAXLEN", "1", "*"};

{
ObjectLock statusLock (status);
Expand Down
4 changes: 2 additions & 2 deletions lib/icingadb/icingadb.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ class IcingaDB : public ObjectImpl<IcingaDB>
std::vector<String> GetTypeDumpSignalKeys(const Type::Ptr& type);
void InsertCheckableDependencies(const Checkable::Ptr& checkable, std::map<String, RedisConnection::Query>& hMSets,
std::vector<Dictionary::Ptr>* runtimeUpdates, const DependencyGroup::Ptr& onlyDependencyGroup = nullptr);
void InsertObjectDependencies(const ConfigObject::Ptr& object, const String typeName, std::map<String, std::vector<String>>& hMSets,
void InsertObjectDependencies(const ConfigObject::Ptr& object, const String typeName, std::map<String, RedisConnection::Query>& hMSets,
std::vector<Dictionary::Ptr>& runtimeUpdates, bool runtimeUpdate);
void UpdateDependenciesState(const Checkable::Ptr& checkable, const DependencyGroup::Ptr& onlyDependencyGroup = nullptr,
std::set<DependencyGroup*>* seenGroups = nullptr) const;
void UpdateState(const Checkable::Ptr& checkable, StateUpdate mode);
void SendConfigUpdate(const ConfigObject::Ptr& object, bool runtimeUpdate);
void CreateConfigUpdate(const ConfigObject::Ptr& object, const String type, std::map<String, std::vector<String>>& hMSets,
void CreateConfigUpdate(const ConfigObject::Ptr& object, const String type, std::map<String, RedisConnection::Query>& hMSets,
std::vector<Dictionary::Ptr>& runtimeUpdates, bool runtimeUpdate);
void SendConfigDelete(const ConfigObject::Ptr& object);
void SendStateChange(const ConfigObject::Ptr& object, const CheckResult::Ptr& cr, StateType type);
Expand Down
17 changes: 14 additions & 3 deletions lib/icingadb/redisconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ namespace asio = boost::asio;

boost::regex RedisConnection::m_ErrAuth ("\\AERR AUTH ");

RedisConnection::QueryArg::operator std::string_view() const
{
if (auto str (std::get_if<String>(&m_Data)); str) {
return *str;
}

return std::get<std::string_view>(m_Data);
}

RedisConnection::RedisConnection(const String& host, int port, const String& path, const String& username, const String& password, int db,
bool useTls, bool insecure, const String& certPath, const String& keyPath, const String& caPath, const String& crlPath,
const String& tlsProtocolmin, const String& cipherList, double connectTimeout, DebugInfo di, const RedisConnection::Ptr& parent)
Expand Down Expand Up @@ -99,10 +108,12 @@ void LogQuery(RedisConnection::Query& query, Log& msg)
break;
}

if (arg.GetLength() > 64) {
msg << " '" << arg.SubStr(0, 61) << "...'";
std::string_view sv (arg);

if (sv.length() > 64) {
msg << " '" << sv.substr(0, 61) << "...'";
} else {
msg << " '" << arg << '\'';
msg << " '" << sv << '\'';
}
}
}
Expand Down
Loading
Loading