From 15f7745bcc25c22d490dae906136b543c6b31d31 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 10 Sep 2015 23:43:58 +0200 Subject: [PATCH] JSON stats reliable and ordered --- assignment-client/src/octree/OctreeServer.cpp | 6 -- domain-server/src/DomainGatekeeper.cpp | 7 +- domain-server/src/DomainServer.cpp | 9 +- domain-server/src/DomainServerNodeData.cpp | 74 +++++++++++----- domain-server/src/DomainServerNodeData.h | 15 +++- .../networking/src/JSONBreakableMarshal.cpp | 88 ------------------- .../networking/src/JSONBreakableMarshal.h | 37 -------- libraries/networking/src/NodeList.cpp | 4 +- tools/udt-test/src/UDTTest.cpp | 4 +- 9 files changed, 74 insertions(+), 170 deletions(-) delete mode 100644 libraries/networking/src/JSONBreakableMarshal.cpp delete mode 100644 libraries/networking/src/JSONBreakableMarshal.h diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index e02690a367..e0d0f9eeac 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -1293,12 +1293,6 @@ QString OctreeServer::getStatusLink() { } void OctreeServer::sendStatsPacket() { - // TODO: we have too many stats to fit in a single MTU... so for now, we break it into multiple JSON objects and - // send them separately. What we really should do is change the NodeList::sendStatsToDomainServer() to handle the - // the following features: - // 1) remember last state sent - // 2) only send new data - // Stats Array 1 QJsonArray threadsStats; quint64 oneSecondAgo = usecTimestampNow() - USECS_PER_SECOND; diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index ae827f3a4a..90b22ffdd8 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -17,7 +17,6 @@ #include #include -#include #include "DomainServer.h" #include "DomainServerNodeData.h" @@ -272,9 +271,9 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect // if we have a username from the connect request, set it on the DomainServerNodeData nodeData->setUsername(username); - // also add an interpolation to JSONBreakableMarshal so that servers can get username in stats - JSONBreakableMarshal::addInterpolationForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY, - uuidStringWithoutCurlyBraces(newNode->getUUID()), username); + // also add an interpolation to DomainServerNodeData so that servers can get username in stats + nodeData->addOverrideForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY, + uuidStringWithoutCurlyBraces(newNode->getUUID()), username); return newNode; } diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 2f5e553c9f..49b7f2e183 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include @@ -1010,7 +1009,7 @@ void DomainServer::sendHeartbeatToIceServer() { void DomainServer::processNodeJSONStatsPacket(QSharedPointer packetList, SharedNodePointer sendingNode) { auto nodeData = dynamic_cast(sendingNode->getLinkedData()); if (nodeData) { - nodeData->processJSONStatsPacket(packetList->getMessage()); + nodeData->updateJSONStats(packetList->getMessage()); } } @@ -1676,9 +1675,9 @@ void DomainServer::nodeKilled(SharedNodePointer node) { } } - // If this node was an Agent ask JSONBreakableMarshal to potentially remove the interpolation we stored - JSONBreakableMarshal::removeInterpolationForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY, - uuidStringWithoutCurlyBraces(node->getUUID())); + // If this node was an Agent ask DomainServerNodeData to potentially remove the interpolation we stored + nodeData->removeOverrideForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY, + uuidStringWithoutCurlyBraces(node->getUUID())); // cleanup the connection secrets that we set up for this node (on the other nodes) foreach (const QUuid& otherNodeSessionUUID, nodeData->getSessionSecretHash().keys()) { diff --git a/domain-server/src/DomainServerNodeData.cpp b/domain-server/src/DomainServerNodeData.cpp index ecbe6bd36e..9b86697d2f 100644 --- a/domain-server/src/DomainServerNodeData.cpp +++ b/domain-server/src/DomainServerNodeData.cpp @@ -10,40 +10,72 @@ // #include +#include +#include #include #include -#include #include #include "DomainServerNodeData.h" -DomainServerNodeData::DomainServerNodeData() : - _sessionSecretHash(), - _assignmentUUID(), - _walletUUID(), - _username(), - _paymentIntervalTimer(), - _statsJSONObject(), - _sendingSockAddr(), - _isAuthenticated(true) -{ +QHash, QString> DomainServerNodeData::_overrideHash; + +DomainServerNodeData::DomainServerNodeData() { _paymentIntervalTimer.start(); } -void DomainServerNodeData::processJSONStatsPacket(QByteArray statsByteArray) { - QJsonObject packetJsonObject = JSONBreakableMarshal::fromByteArray(statsByteArray); - _statsJSONObject = mergeJSONStatsFromNewObject(packetJsonObject, _statsJSONObject); +void DomainServerNodeData::updateJSONStats(QByteArray statsByteArray) { + auto document = QJsonDocument::fromBinaryData(statsByteArray); + Q_ASSERT(document.isObject()); + _statsJSONObject = overrideValuesIfNeeded(document.object()); } -QJsonObject DomainServerNodeData::mergeJSONStatsFromNewObject(const QJsonObject& newObject, QJsonObject destinationObject) { - foreach(const QString& key, newObject.keys()) { - if (newObject[key].isObject() && destinationObject.contains(key)) { - destinationObject[key] = mergeJSONStatsFromNewObject(newObject[key].toObject(), destinationObject[key].toObject()); +QJsonObject DomainServerNodeData::overrideValuesIfNeeded(const QJsonObject& newStats) { + QJsonObject result; + for(auto it = newStats.constBegin(); it != newStats.constEnd(); ++it) { + const auto& key = it.key(); + const auto& value = it.value(); + + auto overrideIt = value.isString() ? _overrideHash.find({key, value.toString()}) : _overrideHash.end(); + if (overrideIt != _overrideHash.end()) { + // We have a match, override the value + result[key] = *overrideIt; + } else if (value.isObject()) { + result[key] = overrideValuesIfNeeded(value.toObject()); + } else if (value.isArray()) { + result[key] = overrideValuesIfNeeded(value.toArray()); } else { - destinationObject[key] = newObject[key]; + result[key] = newStats[key]; } } - - return destinationObject; + return result; +} + +QJsonArray DomainServerNodeData::overrideValuesIfNeeded(const QJsonArray& newStats) { + QJsonArray result; + for (int i = 0; i < newStats.size(); ++i) { + const auto& value = newStats[i]; + + if (value.isObject()) { + + result.push_back(overrideValuesIfNeeded(value.toObject())); + } else if (value.isArray()) { + result.push_back(overrideValuesIfNeeded(value.toArray())); + } else { + result.push_back(newStats[i]); + } + } + return result; +} + +void DomainServerNodeData::addOverrideForKey(const QString& key, const QString& value, + const QString& overrideValue) { + // Insert override value + _overrideHash.insert({key, value}, overrideValue); +} + +void DomainServerNodeData::removeOverrideForKey(const QString& key, const QString& value) { + // Remove override value + _overrideHash.remove({key, value}); } diff --git a/domain-server/src/DomainServerNodeData.h b/domain-server/src/DomainServerNodeData.h index 73003f1a7c..fcdc09e248 100644 --- a/domain-server/src/DomainServerNodeData.h +++ b/domain-server/src/DomainServerNodeData.h @@ -27,7 +27,7 @@ public: const QJsonObject& getStatsJSONObject() const { return _statsJSONObject; } - void processJSONStatsPacket(QByteArray statsByteArray); + void updateJSONStats(QByteArray statsByteArray); void setAssignmentUUID(const QUuid& assignmentUUID) { _assignmentUUID = assignmentUUID; } const QUuid& getAssignmentUUID() const { return _assignmentUUID; } @@ -54,17 +54,24 @@ public: void setNodeVersion(const QString& nodeVersion) { _nodeVersion = nodeVersion; } const QString& getNodeVersion() { return _nodeVersion; } + void addOverrideForKey(const QString& key, const QString& value, const QString& overrideValue); + void removeOverrideForKey(const QString& key, const QString& value); + private: - QJsonObject mergeJSONStatsFromNewObject(const QJsonObject& newObject, QJsonObject destinationObject); - + QJsonObject overrideValuesIfNeeded(const QJsonObject& newStats); + QJsonArray overrideValuesIfNeeded(const QJsonArray& newStats); + QHash _sessionSecretHash; QUuid _assignmentUUID; QUuid _walletUUID; QString _username; QElapsedTimer _paymentIntervalTimer; + QJsonObject _statsJSONObject; + static QHash, QString> _overrideHash; + HifiSockAddr _sendingSockAddr; - bool _isAuthenticated; + bool _isAuthenticated = true; NodeSet _nodeInterestSet; QString _nodeVersion; }; diff --git a/libraries/networking/src/JSONBreakableMarshal.cpp b/libraries/networking/src/JSONBreakableMarshal.cpp deleted file mode 100644 index f0a3ff54ef..0000000000 --- a/libraries/networking/src/JSONBreakableMarshal.cpp +++ /dev/null @@ -1,88 +0,0 @@ -// -// JSONBreakableMarshal.cpp -// libraries/networking/src -// -// Created by Stephen Birarda on 04/28/15. -// Copyright 2015 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#include "JSONBreakableMarshal.h" - -#include -#include -#include -#include - -QVariantMap JSONBreakableMarshal::_interpolationMap = QVariantMap(); - -QByteArray JSONBreakableMarshal::toByteArray(const QJsonObject& jsonObject) { - return QJsonDocument(jsonObject).toBinaryData(); -} - -QJsonObject JSONBreakableMarshal::fromByteArray(const QByteArray& buffer) { - auto document = QJsonDocument::fromBinaryData(buffer); - Q_ASSERT(document.isObject()); - auto object = document.object(); - - QStringList currentKey; - for (auto key : object.keys()) { - interpolate(object[key], key); - } - - return object; -} - -void JSONBreakableMarshal::addInterpolationForKey(const QString& rootKey, const QString& interpolationKey, - const QString& interpolationValue) { - // if there is no map already beneath this key in our _interpolationMap create a QVariantMap there now - auto it = _interpolationMap.find(rootKey); - if (it == _interpolationMap.end()) { - it = _interpolationMap.insert(rootKey, QVariantMap()); - } - Q_ASSERT(it->canConvert(QMetaType::QVariantMap)); - static_cast(it->data())->insert(interpolationKey, interpolationValue); -} - -void JSONBreakableMarshal::removeInterpolationForKey(const QString& rootKey, const QString& interpolationKey) { - // make sure the interpolation map contains this root key and that the value is a map - auto it = _interpolationMap.find(rootKey); - if (it != _interpolationMap.end() && !it->isNull()) { - // remove the value at the interpolationKey - Q_ASSERT(it->canConvert(QMetaType::QVariantMap)); - static_cast(it->data())->remove(interpolationKey); - } -} - -void JSONBreakableMarshal::interpolate(QJsonValueRef currentValue, QString lastKey) { - if (currentValue.isArray()) { - auto array = currentValue.toArray(); - for (int i = 0; i < array.size(); ++i) { - // pass last key and recurse array - interpolate(array[i], QString::number(i)); - } - currentValue = array; - } else if (currentValue.isObject()) { - auto object = currentValue.toObject(); - for (auto key : object.keys()) { - // pass last key and recurse object - interpolate(object[key], key); - } - currentValue = object; - } else if (currentValue.isString()) { - // Maybe interpolate - auto mapIt = _interpolationMap.find(lastKey); - if (mapIt != _interpolationMap.end()) { - Q_ASSERT(mapIt->canConvert(QMetaType::QVariantMap)); - auto interpolationMap = mapIt->toMap(); - - auto result = interpolationMap.find(currentValue.toString()); - if (result != interpolationMap.end()) { - // Replace value - currentValue = result->toString(); - } - } - } -} diff --git a/libraries/networking/src/JSONBreakableMarshal.h b/libraries/networking/src/JSONBreakableMarshal.h deleted file mode 100644 index 28b7c205f0..0000000000 --- a/libraries/networking/src/JSONBreakableMarshal.h +++ /dev/null @@ -1,37 +0,0 @@ -// -// JSONBreakableMarshal.h -// libraries/networking/src -// -// Created by Stephen Birarda on 04/28/15. -// Copyright 2015 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_JSONBreakableMarshal_h -#define hifi_JSONBreakableMarshal_h - -#pragma once - -#include -#include -#include -#include - -class JSONBreakableMarshal { -public: - static QByteArray toByteArray(const QJsonObject& jsonObject); - static QJsonObject fromByteArray(const QByteArray& buffer); - - static void addInterpolationForKey(const QString& rootKey, const QString& interpolationKey, - const QString& interpolationValue); - static void removeInterpolationForKey(const QString& rootKey, const QString& interpolationKey); - -private: - static void interpolate(QJsonValueRef currentValue, QString lastKey); - - static QVariantMap _interpolationMap; -}; - -#endif // hifi_JSONBreakableMarshal_h diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 6a27726a6b..b262904c63 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -27,7 +27,6 @@ #include "AddressManager.h" #include "Assignment.h" #include "HifiSockAddr.h" -#include "JSONBreakableMarshal.h" #include "NetworkLogging.h" #include "udt/PacketHeaders.h" @@ -109,7 +108,8 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned qint64 NodeList::sendStats(const QJsonObject& statsObject, const HifiSockAddr& destination) { auto statsPacketList = NLPacketList::create(PacketType::NodeJsonStats, QByteArray(), true, true); - statsPacketList->write(JSONBreakableMarshal::toByteArray(statsObject)); + QJsonDocument jsonDocument(statsObject); + statsPacketList->write(jsonDocument.toBinaryData()); sendPacketList(std::move(statsPacketList), destination); diff --git a/tools/udt-test/src/UDTTest.cpp b/tools/udt-test/src/UDTTest.cpp index 9a828ebafb..d57ff7a874 100644 --- a/tools/udt-test/src/UDTTest.cpp +++ b/tools/udt-test/src/UDTTest.cpp @@ -267,9 +267,7 @@ void UDTTest::sendPacket() { if (call++ % refillCount == 0) { // construct a reliable and ordered packet list - auto packetList = std::unique_ptr({ - new udt::PacketList(PacketType::BulkAvatarData, QByteArray(), true, true) - }); + auto packetList = udt::PacketList::create(PacketType::BulkAvatarData, QByteArray(), true, true); // fill the packet list with random data according to the constant seed (so receiver can verify) for (int i = 0; i < messageSizePackets; ++i) {