Always update if something has changed.

5 files changed, 18 insertions, 26 deletions

diff --git a/config/src/tests/configagent/configagent.cpp b/config/src/tests/configagent/configagent.cpp
index 88aa905bfa3..f6593890975 100644
--- a/config/src/tests/configagent/configagent.cpp
+++ b/config/src/tests/configagent/configagent.cpp
@@ -22,6 +22,7 @@ public:
     bool abort() override { return false; }
     bool isAborted() const override { return false; }
     void setError(int errorCode) override { (void) errorCode; }
+    bool verifyState(const ConfigState &) const override { return false; }
     const ConfigKey _key;
 };
 
@@ -176,7 +177,7 @@ TEST("require that important(the change) request is delivered to holder even if
     ASSERT_TRUE(latch->poll());
     update = latch->provide();
     ASSERT_TRUE(update);
-    ASSERT_FALSE(update->hasChanged());  // This is a bug. We are loosing the change on generation 1->2
+    ASSERT_TRUE(update->hasChanged());
     MyConfig cfg2(update->getValue());
     ASSERT_EQUAL("l34t", cfg2.myField);
 }
diff --git a/config/src/vespa/config/common/configrequest.h b/config/src/vespa/config/common/configrequest.h
index 395b09abcb0..efe1e56cdd7 100644
--- a/config/src/vespa/config/common/configrequest.h
+++ b/config/src/vespa/config/common/configrequest.h
@@ -24,15 +24,13 @@ public:
 
     ConfigRequest() { }
     virtual ~ConfigRequest() { }
-
     virtual const ConfigKey & getKey() const = 0;
-
     /** Abort a request. */
     virtual bool abort() = 0;
-
     virtual bool isAborted() const = 0;
-
     virtual void setError(int errorCode) = 0;
+    virtual bool verifyState(const ConfigState & state) const = 0;
+
 };
 
 }
diff --git a/config/src/vespa/config/frt/frtconfigagent.cpp b/config/src/vespa/config/frt/frtconfigagent.cpp
index ff16ef77a1b..e4bae234a01 100644
--- a/config/src/vespa/config/frt/frtconfigagent.cpp
+++ b/config/src/vespa/config/frt/frtconfigagent.cpp
@@ -1,5 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #include "frtconfigagent.h"
+#include "frtconfigrequestv3.h"
 #include <vespa/config/common/trace.h>
 
 #include <vespa/log/log.h>
@@ -29,14 +30,14 @@ FRTConfigAgent::handleResponse(const ConfigRequest & request, ConfigResponse::UP
         LOG(spam, "current state for %s: generation %ld md5 %s", key.toString().c_str(), _configState.generation, _configState.md5.c_str());
     }
     if (response->validateResponse() && !response->isError()) {
-        handleOKResponse(std::move(response));
+        handleOKResponse(request, std::move(response));
     } else {
         handleErrorResponse(request, std::move(response));
     }
 }
 
 void
-FRTConfigAgent::handleOKResponse(ConfigResponse::UP response)
+FRTConfigAgent::handleOKResponse(const ConfigRequest & request, ConfigResponse::UP response)
 {
     _failedRequests = 0;
     response->fill();
@@ -45,8 +46,7 @@ FRTConfigAgent::handleOKResponse(ConfigResponse::UP response)
     }
 
     ConfigState newState = response->getConfigState();
-    bool isNewGeneration = newState.isNewerGenerationThan(_configState);
-    if (isNewGeneration) {
+    if ( ! request.verifyState(newState)) {
         handleUpdatedGeneration(response->getKey(), newState, response->getValue());
     }
     setWaitTime(_timingValues.successDelay, 1);
@@ -59,21 +59,15 @@ FRTConfigAgent::handleUpdatedGeneration(const ConfigKey & key, const ConfigState
     if (LOG_WOULD_LOG(spam)) {
         LOG(spam, "new generation %ld for key %s", newState.generation, key.toString().c_str());
     }
-    _configState.generation = newState.generation;
-    bool hasDifferentPayload = newState.hasDifferentPayloadFrom(_configState);
-    if (hasDifferentPayload) {
-        if (LOG_WOULD_LOG(spam)) {
-            LOG(spam, "new payload for key %s, existing md5(%s), new md5(%s)", key.toString().c_str(), _configState.md5.c_str(), newState.md5.c_str());
-        }
-        _configState.md5 = newState.md5;
-        _latest = configValue;
-    }
+    _latest = configValue;
+
 
     if (LOG_WOULD_LOG(spam)) {
-        LOG(spam, "updating holder for key %s, payload changed: %d", key.toString().c_str(), hasDifferentPayload ? 1 : 0);
+        LOG(spam, "updating holder for key %s,", key.toString().c_str());
     }
-    _holder->handle(ConfigUpdate::UP(new ConfigUpdate(_latest, hasDifferentPayload, _configState.generation)));
+    _holder->handle(ConfigUpdate::UP(new ConfigUpdate(_latest, true, newState.generation)));
     _numConfigured++;
+    _configState = newState;
 }
 
 void
diff --git a/config/src/vespa/config/frt/frtconfigagent.h b/config/src/vespa/config/frt/frtconfigagent.h
index e3c362b5278..97edefbded7 100644
--- a/config/src/vespa/config/frt/frtconfigagent.h
+++ b/config/src/vespa/config/frt/frtconfigagent.h
@@ -33,7 +33,7 @@ public:
     const ConfigState & getConfigState() const override;
 private:
     void handleUpdatedGeneration(const ConfigKey & key, const ConfigState & newState, const ConfigValue & configValue);
-    void handleOKResponse(ConfigResponse::UP response);
+    void handleOKResponse(const ConfigRequest & request, ConfigResponse::UP response);
     void handleErrorResponse(const ConfigRequest & request, ConfigResponse::UP response);
     void setWaitTime(uint64_t delay, int multiplier);
 
diff --git a/config/src/vespa/config/frt/frtconfigrequest.h b/config/src/vespa/config/frt/frtconfigrequest.h
index 130d095089d..f34c35ed255 100644
--- a/config/src/vespa/config/frt/frtconfigrequest.h
+++ b/config/src/vespa/config/frt/frtconfigrequest.h
@@ -23,7 +23,6 @@ public:
     FRTConfigRequest(Connection * connection, const ConfigKey & key);
     ~FRTConfigRequest();
     virtual bool verifyKey(const ConfigKey & key) const = 0;
-    virtual bool verifyState(const ConfigState & state) const = 0;
 
     bool abort() override;
     bool isAborted() const override;
@@ -33,11 +32,11 @@ public:
     FRT_RPCRequest* getRequest() { return _request; }
     virtual ConfigResponse::UP createResponse(FRT_RPCRequest * request) const = 0;
 protected:
-    FRT_RPCRequest *_request;
-    FRT_Values & _parameters;
+    FRT_RPCRequest * _request;
+    FRT_Values     & _parameters;
 private:
-    Connection * _connection;
-    const ConfigKey _key;
+    Connection     * _connection;
+    const ConfigKey  _key;
 };
 
 }