diff options
author | Harald Musum <musum@yahoo-inc.com> | 2017-11-24 14:10:18 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-11-24 14:10:18 +0100 |
commit | b4390d3f6c7ef55c562bddc8144163905c12bbdd (patch) | |
tree | f52a1549bf8486ab9fad418b55d99ab80456542f | |
parent | 150c057fd45fb34556e55ee82a614e27c878f3ea (diff) |
Revert "Balder/fix logic flaw causing missing config changes"
18 files changed, 522 insertions, 73 deletions
diff --git a/config/src/tests/configagent/configagent.cpp b/config/src/tests/configagent/configagent.cpp index f6593890975..7eb9442f492 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 valid, int64_t timestamp, - const vespalib::string & md5, const std::string & errorMsg, int errorC0de, bool iserror) + 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) : _key(key), _value(value), + _isUpdated(isUpdated), _fillCalled(false), _valid(valid), _state(md5, timestamp), @@ -54,6 +54,7 @@ public: const ConfigKey _key; const ConfigValue _value; + bool _isUpdated; bool _fillCalled; bool _valid; const ConfigState _state; @@ -63,19 +64,24 @@ public: Trace _trace; - static ConfigResponse::UP createOKResponse(const ConfigKey & key, const ConfigValue & value, uint64_t timestamp = 10, const vespalib::string & md5 = "a") +/** + 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) { - return ConfigResponse::UP(new MyConfigResponse(key, value, true, timestamp, md5, "", 0, false)); + return ConfigResponse::UP(new MyConfigResponse(key, value, true, true, 10, "a", "", 0, false)); } static ConfigResponse::UP createServerErrorResponse(const ConfigKey & key, const ConfigValue & value) { - return ConfigResponse::UP(new MyConfigResponse(key, value, true, 10, "a", "whinewhine", 2, true)); + return ConfigResponse::UP(new MyConfigResponse(key, value, false, true, 10, "a", "whinewhine", 2, true)); } static ConfigResponse::UP createConfigErrorResponse(const ConfigKey & key, const ConfigValue & value) { - return ConfigResponse::UP(new MyConfigResponse(key, value, false, 10, "a", "", 0, false)); + return ConfigResponse::UP(new MyConfigResponse(key, value, false, false, 10, "a", "", 0, false)); } }; @@ -148,40 +154,12 @@ 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); + ASSERT_TRUE(update.get() != NULL); 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<MyConfig>("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<MyConfig>("mykey")); const ConfigValue testValue(createValue("l33t", "a")); diff --git a/config/src/tests/frt/frt.cpp b/config/src/tests/frt/frt.cpp index 5f4b7dee215..1fe91af885b 100644 --- a/config/src/tests/frt/frt.cpp +++ b/config/src/tests/frt/frt.cpp @@ -6,6 +6,10 @@ #include <vespa/config/common/configdefinition.h> #include <vespa/config/frt/connection.h> #include <vespa/config/frt/frtsource.h> +#include <vespa/config/frt/frtconfigresponse.h> +#include <vespa/config/frt/frtconfigrequest.h> +#include <vespa/config/frt/frtconfigrequestv2.h> +#include <vespa/config/frt/frtconfigresponsev2.h> #include <vespa/config/frt/frtconfigrequestv3.h> #include <vespa/config/frt/frtconfigresponsev3.h> #include <vespa/vespalib/data/slime/slime.h> @@ -228,27 +232,107 @@ namespace { TEST_F("require that empty config response does not validate", RPCFixture()) { - FRTConfigResponseV3 fail1(f1.createEmptyRequest()); + FRTConfigResponseV1 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()) { - FRTConfigResponseV3 fail1(f1.createErrorRequest()); + FRTConfigResponseV1 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<vespalib::string> 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()) { - FRTConfigResponseV3 ok(f1.createOKResponse("foo", "baz", "bim", "boo", 12, 15)); + FRTConfigResponseV1 ok(f1.createOKResponse("foo", "baz", "bim", "boo", 12, 15)); ASSERT_FALSE(ok.validateResponse()); ASSERT_FALSE(ok.hasValidResponse()); } -TEST_FF("require that request is config task is scheduled", SourceFixture(), FRTFixture(f1)) +TEST_F("require that valid response returns values after fill", RPCFixture()) { + std::vector<vespalib::string> 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<vespalib::string> 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<vespalib::string>()))); + 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)) { f2.src.getConfig(); ASSERT_TRUE(f2.result.notified); @@ -265,7 +349,7 @@ TEST_FF("require that request is config task is scheduled", SourceFixture(), FRT f2.src.close(); } -TEST("require that v3 request is correctly initialized") { +TEST("require that v2 request is correctly initialized") { ConnectionMock conn; ConfigKey key = ConfigKey::create<MyConfig>("foobi"); vespalib::string md5 = "mymd5"; @@ -275,13 +359,97 @@ 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); - 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))); + 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<MyConfig>("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<MyConfig>()); + EXPECT_EQUAL("foobiar", config->myField); + req->SubRef(); +} + +TEST("require that v3 request is correctly initialized") { + ConnectionMock conn; + ConfigKey key = ConfigKey::create<MyConfig>("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"); + FRTConfigRequestV3 v3req(&conn, key, md5, currentGeneration, wantedGeneration, hostName, timeout, traceIn, VespaVersion::fromString("1.2.3"), CompressionType::LZ4); 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 6eeb1bd79a2..e94510dcdea 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 static_cast<bool>(_current); + return (_current.get() != NULL); } void diff --git a/config/src/vespa/config/common/configrequest.h b/config/src/vespa/config/common/configrequest.h index efe1e56cdd7..395b09abcb0 100644 --- a/config/src/vespa/config/common/configrequest.h +++ b/config/src/vespa/config/common/configrequest.h @@ -24,13 +24,15 @@ 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; + virtual void setError(int errorCode) = 0; }; } diff --git a/config/src/vespa/config/frt/CMakeLists.txt b/config/src/vespa/config/frt/CMakeLists.txt index 879b32684a7..9d0966bb09a 100644 --- a/config/src/vespa/config/frt/CMakeLists.txt +++ b/config/src/vespa/config/frt/CMakeLists.txt @@ -8,7 +8,9 @@ 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 e4bae234a01..ff16ef77a1b 100644 --- a/config/src/vespa/config/frt/frtconfigagent.cpp +++ b/config/src/vespa/config/frt/frtconfigagent.cpp @@ -1,6 +1,5 @@ // 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> @@ -30,14 +29,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(request, std::move(response)); + handleOKResponse(std::move(response)); } else { handleErrorResponse(request, std::move(response)); } } void -FRTConfigAgent::handleOKResponse(const ConfigRequest & request, ConfigResponse::UP response) +FRTConfigAgent::handleOKResponse(ConfigResponse::UP response) { _failedRequests = 0; response->fill(); @@ -46,7 +45,8 @@ FRTConfigAgent::handleOKResponse(const ConfigRequest & request, ConfigResponse:: } ConfigState newState = response->getConfigState(); - if ( ! request.verifyState(newState)) { + bool isNewGeneration = newState.isNewerGenerationThan(_configState); + if (isNewGeneration) { handleUpdatedGeneration(response->getKey(), newState, response->getValue()); } setWaitTime(_timingValues.successDelay, 1); @@ -59,15 +59,21 @@ 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()); } - _latest = configValue; - + _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; + } if (LOG_WOULD_LOG(spam)) { - LOG(spam, "updating holder for key %s,", key.toString().c_str()); + LOG(spam, "updating holder for key %s, payload changed: %d", key.toString().c_str(), hasDifferentPayload ? 1 : 0); } - _holder->handle(ConfigUpdate::UP(new ConfigUpdate(_latest, true, newState.generation))); + _holder->handle(ConfigUpdate::UP(new ConfigUpdate(_latest, hasDifferentPayload, _configState.generation))); _numConfigured++; - _configState = newState; } void diff --git a/config/src/vespa/config/frt/frtconfigagent.h b/config/src/vespa/config/frt/frtconfigagent.h index 97edefbded7..e3c362b5278 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(const ConfigRequest & request, ConfigResponse::UP response); + void handleOKResponse(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 0845f7408de..aa792126aca 100644 --- a/config/src/vespa/config/frt/frtconfigrequest.cpp +++ b/config/src/vespa/config/frt/frtconfigrequest.cpp @@ -2,7 +2,9 @@ #include "frtconfigrequest.h" #include "frtconfigresponse.h" #include "connection.h" -#include <vespa/fnet/frt/rpcrequest.h> +#include <vespa/fnet/frt/frt.h> +#include <vespa/config/common/configkey.h> +#include <vespa/config/common/configstate.h> namespace config { @@ -43,4 +45,52 @@ 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<vespalib::string> & 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<int64_t>(_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 061151d5f39..e1d6b5590bd 100644 --- a/config/src/vespa/config/frt/frtconfigrequest.h +++ b/config/src/vespa/config/frt/frtconfigrequest.h @@ -22,6 +22,8 @@ public: typedef std::unique_ptr<FRTConfigRequest> 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; @@ -31,11 +33,25 @@ 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; +}; + +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; }; } diff --git a/config/src/vespa/config/frt/frtconfigrequestfactory.cpp b/config/src/vespa/config/frt/frtconfigrequestfactory.cpp index 1f1ddb196b0..9fe3e073b65 100644 --- a/config/src/vespa/config/frt/frtconfigrequestfactory.cpp +++ b/config/src/vespa/config/frt/frtconfigrequestfactory.cpp @@ -1,9 +1,14 @@ // 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 <vespa/config/common/trace.h> +#include <vespa/config/common/compressiontype.h> #include <vespa/vespalib/util/host_name.h> +#include <unistd.h> +#include <limits.h> -using std::make_unique; namespace config { @@ -23,11 +28,14 @@ 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 { - return make_unique<FRTConfigRequestV3>(connection, key, state.md5, state.generation, 0u, _hostName, - serverTimeout, Trace(_traceLevel), _vespaVersion, _compressionType); + 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)); } } // namespace config diff --git a/config/src/vespa/config/frt/frtconfigrequestv2.cpp b/config/src/vespa/config/frt/frtconfigrequestv2.cpp new file mode 100644 index 00000000000..ddc79b830b3 --- /dev/null +++ b/config/src/vespa/config/frt/frtconfigrequestv2.cpp @@ -0,0 +1,32 @@ +// 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 <vespa/config/common/trace.h> +#include <vespa/config/common/vespa_version.h> + +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 new file mode 100644 index 00000000000..5f055153b8c --- /dev/null +++ b/config/src/vespa/config/frt/frtconfigrequestv2.h @@ -0,0 +1,29 @@ +// 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 046416bcfd6..98922ad4729 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 <vespa/fnet/frt/rpcrequest.h> +#include <vespa/fnet/frt/frt.h> namespace config { @@ -42,4 +42,52 @@ 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<vespalib::string> 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<vespalib::string> +FRTConfigResponseV1::getPayLoad() const +{ + uint32_t numStrings = (*_returnValues)[7]._string_array._len; + FRT_StringValue *s = (*_returnValues)[7]._string_array._pt; + std::vector<vespalib::string> 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 31811c6a38f..f829044e698 100644 --- a/config/src/vespa/config/frt/frtconfigresponse.h +++ b/config/src/vespa/config/frt/frtconfigresponse.h @@ -39,5 +39,33 @@ 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<vespalib::string> 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 new file mode 100644 index 00000000000..1527700f99f --- /dev/null +++ b/config/src/vespa/config/frt/frtconfigresponsev2.cpp @@ -0,0 +1,44 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "frtconfigresponsev2.h" +#include <vespa/fnet/frt/frt.h> + +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 new file mode 100644 index 00000000000..89d1a9157e3 --- /dev/null +++ b/config/src/vespa/config/frt/frtconfigresponsev2.h @@ -0,0 +1,28 @@ +// 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 <vespa/config/common/configvalue.h> + +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 27ac39ae56a..12dbbab2eb7 100644 --- a/config/src/vespa/config/frt/slimeconfigrequest.cpp +++ b/config/src/vespa/config/frt/slimeconfigrequest.cpp @@ -40,10 +40,19 @@ SlimeConfigRequest::SlimeConfigRequest(Connection * connection, } 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(_data[REQUEST_CONFIG_MD5].asString().make_stringref()) == 0 && - state.generation == _data[REQUEST_CURRENT_GENERATION].asLong()); + return (state.md5.compare(_parameters[4]._string._str) == 0 && + state.generation == static_cast<int64_t>(_parameters[5]._intval64)); } void diff --git a/config/src/vespa/config/frt/slimeconfigrequest.h b/config/src/vespa/config/frt/slimeconfigrequest.h index 625d70c094b..d616876780e 100644 --- a/config/src/vespa/config/frt/slimeconfigrequest.h +++ b/config/src/vespa/config/frt/slimeconfigrequest.h @@ -30,6 +30,7 @@ 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: |