From 03c888af31a356d67e50ca9f2ba1e7c276205f3e Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 28 Nov 2017 13:12:33 +0100 Subject: Revert "Revert "Balder/fix logic flaw causing missing config changes"" --- config/src/tests/configagent/configagent.cpp | 50 ++++-- config/src/tests/frt/frt.cpp | 190 ++------------------- config/src/vespa/config/common/configholder.cpp | 2 +- config/src/vespa/config/common/configrequest.h | 6 +- config/src/vespa/config/frt/CMakeLists.txt | 2 - config/src/vespa/config/frt/frtconfigagent.cpp | 24 +-- config/src/vespa/config/frt/frtconfigagent.h | 2 +- config/src/vespa/config/frt/frtconfigrequest.cpp | 52 +----- config/src/vespa/config/frt/frtconfigrequest.h | 24 +-- .../vespa/config/frt/frtconfigrequestfactory.cpp | 18 +- config/src/vespa/config/frt/frtconfigrequestv2.cpp | 32 ---- config/src/vespa/config/frt/frtconfigrequestv2.h | 29 ---- config/src/vespa/config/frt/frtconfigresponse.cpp | 50 +----- config/src/vespa/config/frt/frtconfigresponse.h | 28 --- .../src/vespa/config/frt/frtconfigresponsev2.cpp | 44 ----- config/src/vespa/config/frt/frtconfigresponsev2.h | 28 --- config/src/vespa/config/frt/slimeconfigrequest.cpp | 13 +- config/src/vespa/config/frt/slimeconfigrequest.h | 1 - 18 files changed, 73 insertions(+), 522 deletions(-) delete mode 100644 config/src/vespa/config/frt/frtconfigrequestv2.cpp delete mode 100644 config/src/vespa/config/frt/frtconfigrequestv2.h delete mode 100644 config/src/vespa/config/frt/frtconfigresponsev2.cpp delete mode 100644 config/src/vespa/config/frt/frtconfigresponsev2.h (limited to 'config') diff --git a/config/src/tests/configagent/configagent.cpp b/config/src/tests/configagent/configagent.cpp index 7eb9442f492..f6593890975 100644 --- a/config/src/tests/configagent/configagent.cpp +++ b/config/src/tests/configagent/configagent.cpp @@ -22,17 +22,17 @@ 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; }; class MyConfigResponse : public ConfigResponse { public: - MyConfigResponse(const ConfigKey & key, const ConfigValue & value, bool isUpdated, bool valid, - int64_t timestamp, const vespalib::string & md5, const std::string & errorMsg, int errorC0de, bool iserror) + MyConfigResponse(const ConfigKey & key, const ConfigValue & value, bool valid, int64_t timestamp, + const vespalib::string & md5, const std::string & errorMsg, int errorC0de, bool iserror) : _key(key), _value(value), - _isUpdated(isUpdated), _fillCalled(false), _valid(valid), _state(md5, timestamp), @@ -54,7 +54,6 @@ public: const ConfigKey _key; const ConfigValue _value; - bool _isUpdated; bool _fillCalled; bool _valid; const ConfigState _state; @@ -64,24 +63,19 @@ public: Trace _trace; -/** - MyConfigResponse(const ConfigKey & key, const ConfigValue & value, bool isUpdated, bool valid, - int64_t timestamp, const vespalib::string & md5, int64_t prevTimestamp, const vespalib::string &prevMd5, - const std::string & errorMsg, int errorC0de, bool iserror) -*/ - static ConfigResponse::UP createOKResponse(const ConfigKey & key, const ConfigValue & value) + static ConfigResponse::UP createOKResponse(const ConfigKey & key, const ConfigValue & value, uint64_t timestamp = 10, const vespalib::string & md5 = "a") { - return ConfigResponse::UP(new MyConfigResponse(key, value, true, true, 10, "a", "", 0, false)); + return ConfigResponse::UP(new MyConfigResponse(key, value, true, timestamp, md5, "", 0, false)); } static ConfigResponse::UP createServerErrorResponse(const ConfigKey & key, const ConfigValue & value) { - return ConfigResponse::UP(new MyConfigResponse(key, value, false, true, 10, "a", "whinewhine", 2, true)); + return ConfigResponse::UP(new MyConfigResponse(key, value, true, 10, "a", "whinewhine", 2, true)); } static ConfigResponse::UP createConfigErrorResponse(const ConfigKey & key, const ConfigValue & value) { - return ConfigResponse::UP(new MyConfigResponse(key, value, false, false, 10, "a", "", 0, false)); + return ConfigResponse::UP(new MyConfigResponse(key, value, false, 10, "a", "", 0, false)); } }; @@ -154,12 +148,40 @@ TEST("require that successful request is delivered to holder") { handler.handleResponse(MyConfigRequest(testKey), MyConfigResponse::createOKResponse(testKey, testValue)); ASSERT_TRUE(latch->poll()); ConfigUpdate::UP update(latch->provide()); - ASSERT_TRUE(update.get() != NULL); + ASSERT_TRUE(update); ASSERT_TRUE(update->hasChanged()); MyConfig cfg(update->getValue()); ASSERT_EQUAL("l33t", cfg.myField); } +TEST("require that important(the change) request is delivered to holder even if it was not the last") { + const ConfigKey testKey(ConfigKey::create("mykey")); + const ConfigValue testValue1(createValue("l33t", "a")); + const ConfigValue testValue2(createValue("l34t", "b")); + IConfigHolder::SP latch(new MyHolder()); + + FRTConfigAgent handler(latch, testTimingValues); + handler.handleResponse(MyConfigRequest(testKey), + MyConfigResponse::createOKResponse(testKey, testValue1, 1, testValue1.getMd5())); + ASSERT_TRUE(latch->poll()); + ConfigUpdate::UP update(latch->provide()); + ASSERT_TRUE(update); + ASSERT_TRUE(update->hasChanged()); + MyConfig cfg(update->getValue()); + ASSERT_EQUAL("l33t", cfg.myField); + + handler.handleResponse(MyConfigRequest(testKey), + MyConfigResponse::createOKResponse(testKey, testValue2, 2, testValue2.getMd5())); + handler.handleResponse(MyConfigRequest(testKey), + MyConfigResponse::createOKResponse(testKey, testValue2, 3, testValue2.getMd5())); + ASSERT_TRUE(latch->poll()); + update = latch->provide(); + ASSERT_TRUE(update); + ASSERT_TRUE(update->hasChanged()); + MyConfig cfg2(update->getValue()); + ASSERT_EQUAL("l34t", cfg2.myField); +} + TEST("require that successful request sets correct wait time") { const ConfigKey testKey(ConfigKey::create("mykey")); const ConfigValue testValue(createValue("l33t", "a")); diff --git a/config/src/tests/frt/frt.cpp b/config/src/tests/frt/frt.cpp index 1fe91af885b..5f4b7dee215 100644 --- a/config/src/tests/frt/frt.cpp +++ b/config/src/tests/frt/frt.cpp @@ -6,10 +6,6 @@ #include #include #include -#include -#include -#include -#include #include #include #include @@ -232,107 +228,27 @@ namespace { TEST_F("require that empty config response does not validate", RPCFixture()) { - FRTConfigResponseV1 fail1(f1.createEmptyRequest()); + FRTConfigResponseV3 fail1(f1.createEmptyRequest()); ASSERT_FALSE(fail1.validateResponse()); ASSERT_FALSE(fail1.hasValidResponse()); ASSERT_TRUE(fail1.isError()); } TEST_F("require that response containing errors does not validate", RPCFixture()) { - FRTConfigResponseV1 fail1(f1.createErrorRequest()); + FRTConfigResponseV3 fail1(f1.createErrorRequest()); ASSERT_FALSE(fail1.validateResponse()); ASSERT_FALSE(fail1.hasValidResponse()); ASSERT_TRUE(fail1.isError()); ASSERT_TRUE(fail1.errorCode() != 0); } -TEST_F("require that valid response validates", RPCFixture()) { - std::vector vec; - vec.push_back("bar \"foo\""); - FRTConfigResponseV1 ok(f1.createOKResponse("foo", "baz", "bim", "boo", 12, 15, vec, "mn")); - ASSERT_TRUE(ok.validateResponse()); - ASSERT_TRUE(ok.hasValidResponse()); -} - TEST_F("require that response contains all values", RPCFixture()) { - FRTConfigResponseV1 ok(f1.createOKResponse("foo", "baz", "bim", "boo", 12, 15)); + FRTConfigResponseV3 ok(f1.createOKResponse("foo", "baz", "bim", "boo", 12, 15)); ASSERT_FALSE(ok.validateResponse()); ASSERT_FALSE(ok.hasValidResponse()); } -TEST_F("require that valid response returns values after fill", RPCFixture()) { - std::vector vec; - vec.push_back("bar \"foo\""); - FRTConfigResponseV1 ok(f1.createOKResponse("foo", "baz", "bim", "boo", 12, 15, vec, "mn")); - ASSERT_TRUE(ok.validateResponse()); - ASSERT_TRUE(ok.hasValidResponse()); - - // Should not be valid - ASSERT_TRUE(ConfigKey() == ok.getKey()); - ASSERT_TRUE(ConfigValue() == ok.getValue()); - - ok.fill(); - ConfigKey key(ok.getKey()); - ASSERT_EQUAL("foo", key.getDefName()); - ASSERT_EQUAL("baz", key.getDefMd5()); - ASSERT_EQUAL("bim", key.getConfigId()); - ASSERT_EQUAL("mn", key.getDefNamespace()); - - ConfigValue value(ok.getValue()); - ASSERT_TRUE(vec == value.getLines()); -} - -TEST("require that request parameters are correctly initialized") { - ConnectionMock conn; - std::vector schema; - schema.push_back("foo"); - schema.push_back("bar"); - ConfigKey key("foo", "bar", "bim", "boo", schema); - FRTConfigRequestV1 frtReq(key, &conn, "mymd5", 1337, 8); - - FRT_RPCRequest * req = frtReq.getRequest(); - FRT_Values & params(*req->GetParams()); - ASSERT_EQUAL("bar", std::string(params[0]._string._str)); - ASSERT_EQUAL("", std::string(params[1]._string._str)); - ASSERT_EQUAL("boo", std::string(params[2]._string._str)); - ASSERT_EQUAL("foo", std::string(params[3]._string._str)); - ASSERT_EQUAL("mymd5", std::string(params[4]._string._str)); - ASSERT_EQUAL(1337u, params[5]._intval64); - ASSERT_EQUAL(8u, params[6]._intval64); - ASSERT_EQUAL("bim", std::string(params[7]._string._str)); - ASSERT_EQUAL(2u, params[8]._string_array._len); - ASSERT_EQUAL("foo", std::string(params[8]._string_array._pt[0]._str)); - ASSERT_EQUAL("bar", std::string(params[8]._string_array._pt[1]._str)); - ASSERT_EQUAL(1u, params[9]._intval32); -} - -TEST("require that request is aborted") { - MyAbortHandler handler; - ConnectionMock conn; - ConfigKey key("foo", "bar", "bim", "boo"); - FRTConfigRequestV1 frtReq(key, &conn, "mymd5", 1337, 8); - frtReq.getRequest()->SetAbortHandler(&handler); - ASSERT_FALSE(frtReq.isAborted()); - ASSERT_TRUE(frtReq.abort()); -} - -TEST_FF("require that request is invoked", SourceFixture(), - FRTFixture(f1)) -{ - f2.result.state = ConfigState("foo", 3); - f2.src.getConfig(); - ASSERT_TRUE(f2.src.getCurrentRequest().verifyKey(f1.key)); - ASSERT_FALSE(f2.src.getCurrentRequest().verifyKey(ConfigKey("foo", "bal", "bim", "boo", std::vector()))); - ASSERT_FALSE(f2.src.getCurrentRequest().verifyState(ConfigState("foo", 0))); - ASSERT_FALSE(f2.src.getCurrentRequest().verifyState(ConfigState("foo", 1))); - ASSERT_FALSE(f2.src.getCurrentRequest().verifyState(ConfigState("bar", 1))); - ASSERT_TRUE(f2.src.getCurrentRequest().verifyState(ConfigState("foo", 3))); - ASSERT_TRUE(f2.result.notified); - f2.src.close(); -} - -TEST_FF("require that request is config task is scheduled", SourceFixture(), - FRTFixture(f1)) +TEST_FF("require that request is config task is scheduled", SourceFixture(), FRTFixture(f1)) { f2.src.getConfig(); ASSERT_TRUE(f2.result.notified); @@ -349,96 +265,6 @@ TEST_FF("require that request is config task is scheduled", SourceFixture(), f2.src.close(); } -TEST("require that v2 request is correctly initialized") { - ConnectionMock conn; - ConfigKey key = ConfigKey::create("foobi"); - vespalib::string md5 = "mymd5"; - int64_t currentGeneration = 3; - int64_t wantedGeneration = 4; - vespalib::string hostName = "myhost"; - int64_t timeout = 3000; - Trace traceIn(3); - traceIn.trace(2, "Hei"); - FRTConfigRequestV2 v2req(&conn, key, md5, currentGeneration, wantedGeneration, hostName, timeout, traceIn); - ConfigDefinition origDef(MyConfig::CONFIG_DEF_SCHEMA); - - FRT_RPCRequest * req = v2req.getRequest(); - ASSERT_TRUE(req != NULL); - FRT_Values & params(*req->GetParams()); - std::string json(params[0]._string._str); - Slime slime; - JsonFormat::decode(Memory(json), slime); - Inspector & root(slime.get()); - EXPECT_EQUAL(2, root[REQUEST_VERSION].asLong()); - EXPECT_EQUAL(key.getDefName(), root[REQUEST_DEF_NAME].asString().make_string()); - EXPECT_EQUAL(key.getDefNamespace(), root[REQUEST_DEF_NAMESPACE].asString().make_string()); - EXPECT_EQUAL(key.getDefMd5(), root[REQUEST_DEF_MD5].asString().make_string()); - EXPECT_EQUAL(key.getConfigId(), root[REQUEST_CLIENT_CONFIGID].asString().make_string()); - EXPECT_EQUAL(hostName, root[REQUEST_CLIENT_HOSTNAME].asString().make_string()); - EXPECT_EQUAL(currentGeneration, root[REQUEST_CURRENT_GENERATION].asLong()); - EXPECT_EQUAL(wantedGeneration, root[REQUEST_WANTED_GENERATION].asLong()); - EXPECT_EQUAL(md5, root[REQUEST_CONFIG_MD5].asString().make_string()); - EXPECT_EQUAL(timeout, root[REQUEST_TIMEOUT].asLong()); - Trace trace; - trace.deserialize(root[REQUEST_TRACE]); - EXPECT_TRUE(trace.shouldTrace(2)); - EXPECT_TRUE(trace.shouldTrace(3)); - EXPECT_FALSE(trace.shouldTrace(4)); - EXPECT_EQUAL(timeout, root[REQUEST_TIMEOUT].asLong()); - ConfigDefinition def; - def.deserialize(root[REQUEST_DEF_CONTENT]); - EXPECT_EQUAL(origDef.asString(), def.asString()); - ConfigResponse::UP response(v2req.createResponse(req)); - req->GetReturn()->AddString("foobar"); - EXPECT_TRUE(response->validateResponse()); -} - -TEST("require that v2 reponse is correctly initialized") { - ConnectionMock conn; - Slime slime; - ConfigKey key = ConfigKey::create("foobi"); - vespalib::string md5 = "mymd5"; - int64_t generation = 3; - vespalib::string hostname = "myhhost"; - Trace traceIn(3); - traceIn.trace(2, "Hei!"); - Cursor & root(slime.setObject()); - root.setLong(RESPONSE_VERSION, 2ul); - root.setString(RESPONSE_DEF_NAME, Memory(key.getDefName())); - root.setString(RESPONSE_DEF_NAMESPACE, Memory(key.getDefNamespace())); - root.setString(RESPONSE_DEF_MD5, Memory(key.getDefMd5())); - root.setString(RESPONSE_CONFIGID, Memory(key.getConfigId())); - root.setString(RESPONSE_CLIENT_HOSTNAME, Memory(hostname)); - root.setString(RESPONSE_CONFIG_MD5, Memory(md5)); - root.setLong(RESPONSE_CONFIG_GENERATION, generation); - traceIn.serialize(root.setObject(RESPONSE_TRACE)); - Cursor & payload(root.setObject(RESPONSE_PAYLOAD)); - payload.setString("myField", "foobiar"); - SimpleBuffer buf; - JsonFormat::encode(slime, buf, true); - FRT_RPCRequest * req = conn.allocRPCRequest(); - req->GetReturn()->AddString(buf.get().make_string().c_str()); - FRTConfigResponseV2 response(req); - ASSERT_TRUE(response.validateResponse()); - response.fill(); - Trace trace(response.getTrace()); - EXPECT_TRUE(trace.shouldTrace(3)); - EXPECT_FALSE(trace.shouldTrace(4)); - ConfigKey responseKey(response.getKey()); - EXPECT_EQUAL(key.getDefName(), responseKey.getDefName()); - EXPECT_EQUAL(key.getDefNamespace(), responseKey.getDefNamespace()); - EXPECT_EQUAL(key.getDefMd5(), responseKey.getDefMd5()); - EXPECT_EQUAL(key.getConfigId(), responseKey.getConfigId()); - EXPECT_EQUAL(hostname, response.getHostName()); - ConfigState state(response.getConfigState()); - EXPECT_EQUAL(md5, state.md5); - EXPECT_EQUAL(generation, state.generation); - ConfigValue value(response.getValue()); - MyConfig::UP config(value.newInstance()); - EXPECT_EQUAL("foobiar", config->myField); - req->SubRef(); -} - TEST("require that v3 request is correctly initialized") { ConnectionMock conn; ConfigKey key = ConfigKey::create("foobi"); @@ -449,7 +275,13 @@ TEST("require that v3 request is correctly initialized") { int64_t timeout = 3000; Trace traceIn(3); traceIn.trace(2, "Hei"); - FRTConfigRequestV3 v3req(&conn, key, md5, currentGeneration, wantedGeneration, hostName, timeout, traceIn, VespaVersion::fromString("1.2.3"), CompressionType::LZ4); + FRTConfigRequestV3 v3req(&conn, key, md5, currentGeneration, wantedGeneration, hostName, + timeout, traceIn, VespaVersion::fromString("1.2.3"), CompressionType::LZ4); + ASSERT_TRUE(v3req.verifyState(ConfigState(md5, 3))); + ASSERT_FALSE(v3req.verifyState(ConfigState(md5, 2))); + ASSERT_FALSE(v3req.verifyState(ConfigState("xxx", 3))); + ASSERT_FALSE(v3req.verifyState(ConfigState("xxx", 2))); + ConfigDefinition origDef(MyConfig::CONFIG_DEF_SCHEMA); FRT_RPCRequest * req = v3req.getRequest(); diff --git a/config/src/vespa/config/common/configholder.cpp b/config/src/vespa/config/common/configholder.cpp index e94510dcdea..6eeb1bd79a2 100644 --- a/config/src/vespa/config/common/configholder.cpp +++ b/config/src/vespa/config/common/configholder.cpp @@ -39,7 +39,7 @@ bool ConfigHolder::poll() { vespalib::MonitorGuard guard(_monitor); - return (_current.get() != NULL); + return static_cast(_current); } void 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/CMakeLists.txt b/config/src/vespa/config/frt/CMakeLists.txt index 9d0966bb09a..879b32684a7 100644 --- a/config/src/vespa/config/frt/CMakeLists.txt +++ b/config/src/vespa/config/frt/CMakeLists.txt @@ -8,9 +8,7 @@ vespa_add_library(config_frt OBJECT frtconfigresponse.cpp frtsourcefactory.cpp frtconfigagent.cpp - frtconfigrequestv2.cpp frtconfigrequestfactory.cpp - frtconfigresponsev2.cpp protocol.cpp slimeconfigrequest.cpp slimeconfigresponse.cpp 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 #include @@ -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.cpp b/config/src/vespa/config/frt/frtconfigrequest.cpp index aa792126aca..0845f7408de 100644 --- a/config/src/vespa/config/frt/frtconfigrequest.cpp +++ b/config/src/vespa/config/frt/frtconfigrequest.cpp @@ -2,9 +2,7 @@ #include "frtconfigrequest.h" #include "frtconfigresponse.h" #include "connection.h" -#include -#include -#include +#include namespace config { @@ -45,52 +43,4 @@ FRTConfigRequest::isAborted() const return (_request->GetErrorCode() == FRTE_RPC_ABORT); } -const vespalib::string FRTConfigRequestV1::REQUEST_TYPES = "sssssllsSi"; - -FRTConfigRequestV1::FRTConfigRequestV1(const ConfigKey & key, - Connection * connection, - const vespalib::string & configMd5, - int64_t generation, - int64_t serverTimeout) - : FRTConfigRequest(connection, key) -{ - _request->SetMethodName("config.v1.getConfig"); - _parameters.AddString(key.getDefName().c_str()); - _parameters.AddString(""); - _parameters.AddString(key.getDefMd5().c_str()); - _parameters.AddString(key.getConfigId().c_str()); - _parameters.AddString(configMd5.c_str()); - _parameters.AddInt64(generation); - _parameters.AddInt64(serverTimeout); - _parameters.AddString(key.getDefNamespace().c_str()); - const std::vector & schema(key.getDefSchema()); - FRT_StringValue * schemaValue = _parameters.AddStringArray(schema.size()); - for (size_t i = 0; i < schema.size(); i++) { - _parameters.SetString(&schemaValue[i], schema[i].c_str()); - } - _parameters.AddInt32(1); -} - -bool -FRTConfigRequestV1::verifyKey(const ConfigKey & key) const -{ - return (key.getDefName().compare(_parameters[0]._string._str) == 0 && - key.getDefNamespace().compare(_parameters[7]._string._str) == 0 && - key.getConfigId().compare(_parameters[3]._string._str) == 0 && - key.getDefMd5().compare(_parameters[2]._string._str) == 0); -} - -bool -FRTConfigRequestV1::verifyState(const ConfigState & state) const -{ - return (state.md5.compare(_parameters[4]._string._str) == 0 && - state.generation == static_cast(_parameters[5]._intval64)); -} - -ConfigResponse::UP -FRTConfigRequestV1::createResponse(FRT_RPCRequest * request) const -{ - return ConfigResponse::UP(new FRTConfigResponseV1(request)); -} - } // namespace config diff --git a/config/src/vespa/config/frt/frtconfigrequest.h b/config/src/vespa/config/frt/frtconfigrequest.h index e1d6b5590bd..061151d5f39 100644 --- a/config/src/vespa/config/frt/frtconfigrequest.h +++ b/config/src/vespa/config/frt/frtconfigrequest.h @@ -22,8 +22,6 @@ public: typedef std::unique_ptr UP; 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,25 +31,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; -}; - -class FRTConfigRequestV1 : public FRTConfigRequest { -public: - FRTConfigRequestV1(const ConfigKey & key, - Connection * connection, - const vespalib::string & configMd5, - int64_t generation, - int64_t serverTimeout); - bool verifyKey(const ConfigKey & key) const override; - bool verifyState(const ConfigState & state) const override; - ConfigResponse::UP createResponse(FRT_RPCRequest * request) const override; -private: - static const vespalib::string REQUEST_TYPES; + Connection * _connection; + const ConfigKey _key; }; } diff --git a/config/src/vespa/config/frt/frtconfigrequestfactory.cpp b/config/src/vespa/config/frt/frtconfigrequestfactory.cpp index 9fe3e073b65..1f1ddb196b0 100644 --- a/config/src/vespa/config/frt/frtconfigrequestfactory.cpp +++ b/config/src/vespa/config/frt/frtconfigrequestfactory.cpp @@ -1,14 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "frtconfigrequestfactory.h" -#include "frtconfigrequest.h" -#include "frtconfigrequestv2.h" #include "frtconfigrequestv3.h" -#include -#include #include -#include -#include +using std::make_unique; namespace config { @@ -28,14 +23,11 @@ FRTConfigRequestFactory::~FRTConfigRequestFactory() { } FRTConfigRequest::UP -FRTConfigRequestFactory::createConfigRequest(const ConfigKey & key, Connection * connection, const ConfigState & state, int64_t serverTimeout) const +FRTConfigRequestFactory::createConfigRequest(const ConfigKey & key, Connection * connection, + const ConfigState & state, int64_t serverTimeout) const { - if (1 == _protocolVersion) { - return FRTConfigRequest::UP(new FRTConfigRequestV1(key, connection, state.md5, state.generation, serverTimeout)); - } else if (2 == _protocolVersion) { - return FRTConfigRequest::UP(new FRTConfigRequestV2(connection, key, state.md5, state.generation, 0u, _hostName, serverTimeout, Trace(_traceLevel))); - } - return FRTConfigRequest::UP(new FRTConfigRequestV3(connection, key, state.md5, state.generation, 0u, _hostName, serverTimeout, Trace(_traceLevel), _vespaVersion, _compressionType)); + return make_unique(connection, key, state.md5, state.generation, 0u, _hostName, + serverTimeout, Trace(_traceLevel), _vespaVersion, _compressionType); } } // namespace config diff --git a/config/src/vespa/config/frt/frtconfigrequestv2.cpp b/config/src/vespa/config/frt/frtconfigrequestv2.cpp deleted file mode 100644 index ddc79b830b3..00000000000 --- a/config/src/vespa/config/frt/frtconfigrequestv2.cpp +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "frtconfigrequestv2.h" -#include "frtconfigresponsev2.h" -#include "connection.h" -#include -#include - -using namespace config::protocol; - -namespace config { - -FRTConfigRequestV2::FRTConfigRequestV2(Connection * connection, - const ConfigKey & key, - const vespalib::string & configMd5, - int64_t currentGeneration, - int64_t wantedGeneration, - const vespalib::string & hostName, - int64_t serverTimeout, - const Trace & trace) - : SlimeConfigRequest(connection, key, configMd5, currentGeneration, wantedGeneration, hostName, serverTimeout, trace, VespaVersion::getCurrentVersion(), 2, CompressionType::UNCOMPRESSED, "config.v2.getConfig") -{ -} - - - -ConfigResponse::UP -FRTConfigRequestV2::createResponse(FRT_RPCRequest * request) const -{ - return ConfigResponse::UP(new FRTConfigResponseV2(request)); -} - -} diff --git a/config/src/vespa/config/frt/frtconfigrequestv2.h b/config/src/vespa/config/frt/frtconfigrequestv2.h deleted file mode 100644 index 5f055153b8c..00000000000 --- a/config/src/vespa/config/frt/frtconfigrequestv2.h +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "slimeconfigrequest.h" - -class FRT_Values; -class FRT_RPCRequest; - -namespace config { - -class ConfigKey; -class Connection; -class Trace; - -class FRTConfigRequestV2 : public SlimeConfigRequest { -public: - FRTConfigRequestV2(Connection * connection, - const ConfigKey & key, - const vespalib::string & configMd5, - int64_t currentGeneration, - int64_t wantedGeneration, - const vespalib::string & hostName, - int64_t serverTimeout, - const Trace & trace); - ConfigResponse::UP createResponse(FRT_RPCRequest * request) const override; -}; - -} - diff --git a/config/src/vespa/config/frt/frtconfigresponse.cpp b/config/src/vespa/config/frt/frtconfigresponse.cpp index 98922ad4729..046416bcfd6 100644 --- a/config/src/vespa/config/frt/frtconfigresponse.cpp +++ b/config/src/vespa/config/frt/frtconfigresponse.cpp @@ -1,7 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "frtconfigresponse.h" -#include +#include namespace config { @@ -42,52 +42,4 @@ vespalib::string FRTConfigResponse::errorMessage() const { return _request->GetE int FRTConfigResponse::errorCode() const { return _request->GetErrorCode(); } bool FRTConfigResponse::isError() const { return _request->IsError(); } -// -// V1 Implementation -// -const vespalib::string FRTConfigResponseV1::RESPONSE_TYPES = "sssssilSs"; - -FRTConfigResponseV1::FRTConfigResponseV1(FRT_RPCRequest * request) - : FRTConfigResponse(request), - _key(), - _value() -{ -} - -FRTConfigResponseV1::~FRTConfigResponseV1() {} - -const vespalib::string & -FRTConfigResponseV1::getResponseTypes() const -{ - return RESPONSE_TYPES; -} - -void -FRTConfigResponseV1::fill() -{ - const std::vector payload(getPayLoad()); - _value = ConfigValue(payload, calculateContentMd5(payload)); - _key = readKey(); - _state = ConfigState(vespalib::string((*_returnValues)[4]._string._str), (*_returnValues)[6]._intval64); -} - -const ConfigKey -FRTConfigResponseV1::readKey() const -{ - return ConfigKey((*_returnValues)[3]._string._str, (*_returnValues)[0]._string._str, (*_returnValues)[8]._string._str, (*_returnValues)[2]._string._str); -} - -const std::vector -FRTConfigResponseV1::getPayLoad() const -{ - uint32_t numStrings = (*_returnValues)[7]._string_array._len; - FRT_StringValue *s = (*_returnValues)[7]._string_array._pt; - std::vector payload; - payload.reserve(numStrings); - for (uint32_t i = 0; i < numStrings; i++) { - payload.push_back(vespalib::string(s[i]._str)); - } - return payload; -} - } // namespace config diff --git a/config/src/vespa/config/frt/frtconfigresponse.h b/config/src/vespa/config/frt/frtconfigresponse.h index f829044e698..31811c6a38f 100644 --- a/config/src/vespa/config/frt/frtconfigresponse.h +++ b/config/src/vespa/config/frt/frtconfigresponse.h @@ -39,33 +39,5 @@ protected: FRT_Values * _returnValues; }; -class FRTConfigResponseV1 : public FRTConfigResponse { -private: - FRTConfigResponseV1& operator=(const FRTConfigResponseV1&); -public: - FRTConfigResponseV1(FRT_RPCRequest * request); - ~FRTConfigResponseV1(); - - const ConfigKey & getKey() const override { return _key; } - const ConfigValue & getValue() const override { return _value; } - const Trace & getTrace() const override { return _trace; } - - const ConfigState & getConfigState() const override { return _state; } - - void fill() override; - -private: - static const vespalib::string RESPONSE_TYPES; - - const std::vector getPayLoad() const; - const ConfigKey readKey() const; - const vespalib::string & getResponseTypes() const override; - - ConfigKey _key; - ConfigValue _value; - ConfigState _state; - Trace _trace; -}; - } // namespace config diff --git a/config/src/vespa/config/frt/frtconfigresponsev2.cpp b/config/src/vespa/config/frt/frtconfigresponsev2.cpp deleted file mode 100644 index 1527700f99f..00000000000 --- a/config/src/vespa/config/frt/frtconfigresponsev2.cpp +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "frtconfigresponsev2.h" -#include - -using namespace vespalib; -using namespace vespalib::slime; -using namespace vespalib::slime::convenience; -using namespace config::protocol::v2; - -namespace config { - -class V2Payload : public protocol::Payload { -public: - V2Payload(const SlimePtr & data) - : _data(data) - {} - const Inspector & getSlimePayload() const override { - return extractPayload(*_data); - } -private: - SlimePtr _data; -}; - -const vespalib::string FRTConfigResponseV2::RESPONSE_TYPES = "s"; - -FRTConfigResponseV2::FRTConfigResponseV2(FRT_RPCRequest * request) - : SlimeConfigResponse(request) -{ -} - -const vespalib::string & -FRTConfigResponseV2::getResponseTypes() const -{ - return RESPONSE_TYPES; -} - -const ConfigValue -FRTConfigResponseV2::readConfigValue() const -{ - vespalib::string md5(_data->get()[RESPONSE_CONFIG_MD5].asString().make_string()); - return ConfigValue(PayloadPtr(new V2Payload(_data)), md5); -} - -} // namespace config diff --git a/config/src/vespa/config/frt/frtconfigresponsev2.h b/config/src/vespa/config/frt/frtconfigresponsev2.h deleted file mode 100644 index 89d1a9157e3..00000000000 --- a/config/src/vespa/config/frt/frtconfigresponsev2.h +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "slimeconfigresponse.h" -#include - -class FRT_RPCRequest; -class FRT_Values; - -namespace config { - -/** - * Baseclass for config responses. - */ -class FRTConfigResponseV2 : public SlimeConfigResponse { -private: - FRTConfigResponseV2& operator=(const FRTConfigResponseV2&); -public: - FRTConfigResponseV2(FRT_RPCRequest * request); - -private: - static const vespalib::string RESPONSE_TYPES; - const vespalib::string & getResponseTypes() const override; - const ConfigValue readConfigValue() const override; -}; - -} // namespace config - diff --git a/config/src/vespa/config/frt/slimeconfigrequest.cpp b/config/src/vespa/config/frt/slimeconfigrequest.cpp index 12dbbab2eb7..27ac39ae56a 100644 --- a/config/src/vespa/config/frt/slimeconfigrequest.cpp +++ b/config/src/vespa/config/frt/slimeconfigrequest.cpp @@ -39,20 +39,11 @@ SlimeConfigRequest::SlimeConfigRequest(Connection * connection, _parameters.AddString(createJsonFromSlime(_data).c_str()); } -bool -SlimeConfigRequest::verifyKey(const ConfigKey & key) const -{ - return (key.getDefName().compare(_parameters[0]._string._str) == 0 && - key.getDefNamespace().compare(_parameters[7]._string._str) == 0 && - key.getConfigId().compare(_parameters[3]._string._str) == 0 && - key.getDefMd5().compare(_parameters[2]._string._str) == 0); -} - bool SlimeConfigRequest::verifyState(const ConfigState & state) const { - return (state.md5.compare(_parameters[4]._string._str) == 0 && - state.generation == static_cast(_parameters[5]._intval64)); + return (state.md5.compare(_data[REQUEST_CONFIG_MD5].asString().make_stringref()) == 0 && + state.generation == _data[REQUEST_CURRENT_GENERATION].asLong()); } void diff --git a/config/src/vespa/config/frt/slimeconfigrequest.h b/config/src/vespa/config/frt/slimeconfigrequest.h index d616876780e..625d70c094b 100644 --- a/config/src/vespa/config/frt/slimeconfigrequest.h +++ b/config/src/vespa/config/frt/slimeconfigrequest.h @@ -30,7 +30,6 @@ public: const CompressionType & compressionType, const vespalib::string & methodName); ~SlimeConfigRequest() {} - bool verifyKey(const ConfigKey & key) const override; bool verifyState(const ConfigState & state) const override; virtual ConfigResponse::UP createResponse(FRT_RPCRequest * request) const override = 0; private: -- cgit v1.2.3