Merge pull request #8775 from birarda/master

remove AssetResourceRequest timeout, output occasional progress to log

libraries/networking/src/AssetResourceRequest.cpp

@@ -11,10 +11,20 @@
 
 #include "AssetResourceRequest.h"
 
+#include <QtCore/QLoggingCategory>
+
 #include "AssetClient.h"
 #include "AssetUtils.h"
 #include "MappingRequest.h"
-#include <QtCore/qloggingcategory.h>
+#include "NetworkLogging.h"
+
+static const int DOWNLOAD_PROGRESS_LOG_INTERVAL_SECONDS = 5;
+
+AssetResourceRequest::AssetResourceRequest(const QUrl& url) :
+    ResourceRequest(url)
+{
+    _lastProgressDebug = p_high_resolution_clock::now() - std::chrono::seconds(DOWNLOAD_PROGRESS_LOG_INTERVAL_SECONDS);
+}
 
 AssetResourceRequest::~AssetResourceRequest() {
     if (_assetMappingRequest) {
@@ -24,10 +34,6 @@ AssetResourceRequest::~AssetResourceRequest() {
     if (_assetRequest) {
         _assetRequest->deleteLater();
     }
-
-    if (_sendTimer) {
-        cleanupTimer();
-    }
 }
 
 bool AssetResourceRequest::urlIsAssetHash() const {
@@ -37,24 +43,6 @@ bool AssetResourceRequest::urlIsAssetHash() const {
     return hashRegex.exactMatch(_url.toString());
 }
 
-void AssetResourceRequest::setupTimer() {
-    Q_ASSERT(!_sendTimer);
-    static const int TIMEOUT_MS = 2000;
-
-    _sendTimer = new QTimer(this);
-    connect(_sendTimer, &QTimer::timeout, this, &AssetResourceRequest::onTimeout);
-
-    _sendTimer->setSingleShot(true);
-    _sendTimer->start(TIMEOUT_MS);
-}
-
-void AssetResourceRequest::cleanupTimer() {
-    Q_ASSERT(_sendTimer);
-    disconnect(_sendTimer, 0, this, 0);
-    _sendTimer->deleteLater();
-    _sendTimer = nullptr;
-}
-
 void AssetResourceRequest::doSend() {
     // We'll either have a hash or an ATP path to a file (that maps to a hash)
     if (urlIsAssetHash()) {
@@ -81,8 +69,6 @@ void AssetResourceRequest::requestMappingForPath(const AssetPath& path) {
         Q_ASSERT(_state == InProgress);
         Q_ASSERT(request == _assetMappingRequest);
 
-        cleanupTimer();
-
         switch (request->getError()) {
             case MappingRequest::NoError:
                 // we have no error, we should have a resulting hash - use that to send of a request for that asset
@@ -118,7 +104,6 @@ void AssetResourceRequest::requestMappingForPath(const AssetPath& path) {
         _assetMappingRequest = nullptr;
     });
 
-    setupTimer();
     _assetMappingRequest->start();
 }
 
@@ -133,8 +118,6 @@ void AssetResourceRequest::requestHash(const AssetHash& hash) {
         Q_ASSERT(_state == InProgress);
         Q_ASSERT(req == _assetRequest);
         Q_ASSERT(req->getState() == AssetRequest::Finished);
-
-        cleanupTimer();
         
         switch (req->getError()) {
             case AssetRequest::Error::NoError:
@@ -162,35 +145,29 @@ void AssetResourceRequest::requestHash(const AssetHash& hash) {
         _assetRequest = nullptr;
     });
 
-    setupTimer();
     _assetRequest->start();
 }
 
 void AssetResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytesTotal) {
     Q_ASSERT(_state == InProgress);
 
-    // We've received data, so reset the timer
-    _sendTimer->start();
-
     emit progress(bytesReceived, bytesTotal);
+
+    auto now = p_high_resolution_clock::now();
+
+    // if we haven't received the full asset check if it is time to output progress to log
+    // we do so every X seconds to assist with ATP download tracking
+
+    if (bytesReceived != bytesTotal
+        && now - _lastProgressDebug > std::chrono::seconds(DOWNLOAD_PROGRESS_LOG_INTERVAL_SECONDS)) {
+
+        int percentage =  roundf((float) bytesReceived / (float) bytesTotal * 100.0f);
+
+        qCDebug(networking).nospace() << "Progress for " << _url.path() << " - "
+            << bytesReceived << " of " << bytesTotal << " bytes - " << percentage << "%";
+
+        _lastProgressDebug = now;
+    }
+
 }
 
-void AssetResourceRequest::onTimeout() {
-    if (_state == InProgress) {
-        qWarning() << "Asset request timed out: " << _url;
-        if (_assetRequest) {
-            disconnect(_assetRequest, 0, this, 0);
-            _assetRequest->deleteLater();
-            _assetRequest = nullptr;
-        }
-        if (_assetMappingRequest) {
-            disconnect(_assetMappingRequest, 0, this, 0);
-            _assetMappingRequest->deleteLater();
-            _assetMappingRequest = nullptr;
-        }
-        _result = Timeout;
-        _state = Finished;
-        emit finished();
-    }
-    cleanupTimer();
-}

libraries/networking/src/AssetResourceRequest.h

@@ -14,13 +14,15 @@
 
 #include <QUrl>
 
+#include <PortableHighResolutionClock.h>
+
 #include "AssetRequest.h"
 #include "ResourceRequest.h"
 
 class AssetResourceRequest : public ResourceRequest {
     Q_OBJECT
 public:
-    AssetResourceRequest(const QUrl& url) : ResourceRequest(url) { }
+    AssetResourceRequest(const QUrl& url);
     virtual ~AssetResourceRequest() override;
 
 protected:
@@ -28,21 +30,17 @@ protected:
 
 private slots:
     void onDownloadProgress(qint64 bytesReceived, qint64 bytesTotal);
-    void onTimeout();
 
 private:
-    void setupTimer();
-    void cleanupTimer();
-
     bool urlIsAssetHash() const;
 
     void requestMappingForPath(const AssetPath& path);
     void requestHash(const AssetHash& hash);
 
-    QTimer* _sendTimer { nullptr };
-
     GetMappingRequest* _assetMappingRequest { nullptr };
     AssetRequest* _assetRequest { nullptr };
+
+    p_high_resolution_clock::time_point _lastProgressDebug;
 };
 
 #endif

libraries/networking/src/ReceivedMessage.cpp

@@ -54,7 +54,7 @@ void ReceivedMessage::appendPacket(NLPacket& packet) {
                "We should not be appending to a complete message");
 
     // Limit progress signal to every X packets
-    const int EMIT_PROGRESS_EVERY_X_PACKETS = 100;
+    const int EMIT_PROGRESS_EVERY_X_PACKETS = 50;
 
     ++_numPackets;