From b16719046a0b9573a6854c4282894079e9038954 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 1 Jun 2021 04:53:16 +0000 Subject: Avoid coredump on no config sources. --- .../tests/frtconnectionpool/frtconnectionpool.cpp | 12 +++--- config/src/vespa/config/frt/frtconnectionpool.cpp | 48 ++++++++++------------ config/src/vespa/config/frt/frtconnectionpool.h | 4 +- config/src/vespa/config/frt/frtsource.cpp | 3 ++ 4 files changed, 32 insertions(+), 35 deletions(-) diff --git a/config/src/tests/frtconnectionpool/frtconnectionpool.cpp b/config/src/tests/frtconnectionpool/frtconnectionpool.cpp index 79ec1ee42f4..6320feb1e12 100644 --- a/config/src/tests/frtconnectionpool/frtconnectionpool.cpp +++ b/config/src/tests/frtconnectionpool/frtconnectionpool.cpp @@ -169,14 +169,12 @@ void Test::testSetErrorAllHashBased() { const ServerSpec spec(_sources); FRTConnectionPool sourcePool(spec, timingValues); FRTConnection* firstSource = sourcePool.getNextHashBased(); - std::vector readySources; - sourcePool.getReadySources(readySources); + auto readySources = sourcePool.getReadySources(); for (int i = 0; i < (int)readySources.size(); i++) { readySources[i]->setError(FRTE_RPC_CONNECTION); } - std::vector tmpSources; - EXPECT_EQUAL((int)sourcePool.getReadySources(tmpSources).size(), 0); - EXPECT_EQUAL((int)sourcePool.getSuspendedSources(tmpSources).size(), 3); + EXPECT_EQUAL(sourcePool.getReadySources().size(), 0u); + EXPECT_EQUAL(sourcePool.getSuspendedSources().size(), 3u); // should get the same source now, since all are suspended EXPECT_EQUAL(firstSource->getAddress(), sourcePool.getNextHashBased()->getAddress()); @@ -188,8 +186,8 @@ void Test::testSetErrorAllHashBased() { } } - EXPECT_EQUAL((int)sourcePool.getReadySources(tmpSources).size(), 2); - EXPECT_EQUAL((int)sourcePool.getSuspendedSources(tmpSources).size(), 1); + EXPECT_EQUAL(sourcePool.getReadySources().size(), 2u); + EXPECT_EQUAL(sourcePool.getSuspendedSources().size(), 1u); // should not get the same source now, since original source is // suspended, while the rest are OK diff --git a/config/src/vespa/config/frt/frtconnectionpool.cpp b/config/src/vespa/config/frt/frtconnectionpool.cpp index 1aff50f9041..00ed9334537 100644 --- a/config/src/vespa/config/frt/frtconnectionpool.cpp +++ b/config/src/vespa/config/frt/frtconnectionpool.cpp @@ -66,20 +66,18 @@ FRTConnectionPool::getCurrent() FRTConnection * FRTConnectionPool::getNextRoundRobin() { - std::vector readySources; - getReadySources(readySources); - std::vector suspendedSources; - getSuspendedSources(suspendedSources); + auto ready = getReadySources(); + auto suspended = getSuspendedSources(); FRTConnection* nextFRTConnection = nullptr; - if (!readySources.empty()) { - int sel = _selectIdx % (int)readySources.size(); + if ( ! ready.empty()) { + int sel = _selectIdx % (int)ready.size(); _selectIdx = sel + 1; - nextFRTConnection = readySources[sel]; - } else if (!suspendedSources.empty()) { - int sel = _selectIdx % (int)suspendedSources.size(); + nextFRTConnection = ready[sel]; + } else if ( ! suspended.empty()) { + int sel = _selectIdx % (int)suspended.size(); _selectIdx = sel + 1; - nextFRTConnection = suspendedSources[sel]; + nextFRTConnection = suspended[sel]; } return nextFRTConnection; } @@ -87,28 +85,26 @@ FRTConnectionPool::getNextRoundRobin() FRTConnection * FRTConnectionPool::getNextHashBased() { - std::vector readySources; - getReadySources(readySources); - std::vector suspendedSources; - getSuspendedSources(suspendedSources); + auto ready = getReadySources(); + auto suspended = getSuspendedSources(); FRTConnection* nextFRTConnection = nullptr; - if (!readySources.empty()) { - int sel = std::abs(hashCode(_hostname) % (int)readySources.size()); - nextFRTConnection = readySources[sel]; - } else { - int sel = std::abs(hashCode(_hostname) % (int)suspendedSources.size()); - nextFRTConnection = suspendedSources[sel]; + if ( ! ready.empty()) { + int sel = std::abs(hashCode(_hostname) % (int)ready.size()); + nextFRTConnection = ready[sel]; + } else if ( ! suspended.empty() ){ + int sel = std::abs(hashCode(_hostname) % (int)suspended.size()); + nextFRTConnection = suspended[sel]; } return nextFRTConnection; } -const std::vector & -FRTConnectionPool::getReadySources(std::vector & readySources) const +std::vector +FRTConnectionPool::getReadySources() const { - readySources.clear(); + std::vector readySources; for (const auto & entry : _connections) { FRTConnection* source = entry.second.get(); int64_t tnow = FRTConnection::milliSecsSinceEpoch(); @@ -120,10 +116,10 @@ FRTConnectionPool::getReadySources(std::vector & readySources) c return readySources; } -const std::vector & -FRTConnectionPool::getSuspendedSources(std::vector & suspendedSources) const +std::vector +FRTConnectionPool::getSuspendedSources() const { - suspendedSources.clear(); + std::vector suspendedSources; for (const auto & entry : _connections) { FRTConnection* source = entry.second.get(); int64_t tnow = FRTConnection::milliSecsSinceEpoch(); diff --git a/config/src/vespa/config/frt/frtconnectionpool.h b/config/src/vespa/config/frt/frtconnectionpool.h index 0b30e723272..dc758249771 100644 --- a/config/src/vespa/config/frt/frtconnectionpool.h +++ b/config/src/vespa/config/frt/frtconnectionpool.h @@ -109,14 +109,14 @@ public: * * @return list of FRTConnection pointers */ - const std::vector & getReadySources(std::vector & readySources) const; + std::vector getReadySources() const; /** * Gets list of sources that are suspended. * * @param suspendedSources is list of FRTConnection pointers */ - const std::vector & getSuspendedSources(std::vector & suspendedSources) const; + std::vector getSuspendedSources() const; /** * Implementation of the Java hashCode function for the String class. diff --git a/config/src/vespa/config/frt/frtsource.cpp b/config/src/vespa/config/frt/frtsource.cpp index 94884e19b3f..4bb8a8a3611 100644 --- a/config/src/vespa/config/frt/frtsource.cpp +++ b/config/src/vespa/config/frt/frtsource.cpp @@ -51,6 +51,9 @@ FRTSource::getConfig() int64_t serverTimeout = _agent->getTimeout(); double clientTimeout = (serverTimeout / 1000.0) + 5.0; // The additional 5 seconds is the time allowed for the server to respond after serverTimeout has elapsed. Connection * connection = _connectionFactory->getCurrent(); + if (connection == nullptr) { + LOG(warning, "No connection available - bad config ?"); + } const ConfigState & state(_agent->getConfigState()); // LOG(debug, "invoking request with md5 %s, gen %" PRId64 ", servertimeout(%" PRId64 "), client(%f)", state.md5.c_str(), state.generation, serverTimeout, clientTimeout); -- cgit v1.2.3