diff options
author | Arne H Juul <arnej@yahooinc.com> | 2021-09-09 14:04:28 +0000 |
---|---|---|
committer | Arne H Juul <arnej@yahooinc.com> | 2021-09-09 14:14:29 +0000 |
commit | b88b16941da5d27754199ba2b03e75aa37a977c1 (patch) | |
tree | b20d3953eafbb9ad31685fcfb8e10e9667c65fd2 /slobrok | |
parent | bb42ff1449aec38ff7dffc43eb9370b635c3eef4 (diff) |
handle addLocal for down services better
* if we already have a registration in progress, hook the second
one to the first so they get the same answer at the same time
* if we think the service is down, restart its monitoring to
avoid getting stale information, and add the inflight hook so we get an
answer when the new ping returns.
Diffstat (limited to 'slobrok')
-rw-r--r-- | slobrok/src/tests/local_rpc_monitor_map/local_rpc_monitor_map_test.cpp | 21 | ||||
-rw-r--r-- | slobrok/src/vespa/slobrok/server/local_rpc_monitor_map.cpp | 37 |
2 files changed, 53 insertions, 5 deletions
diff --git a/slobrok/src/tests/local_rpc_monitor_map/local_rpc_monitor_map_test.cpp b/slobrok/src/tests/local_rpc_monitor_map/local_rpc_monitor_map_test.cpp index 40f8baa32a9..a69a77d3640 100644 --- a/slobrok/src/tests/local_rpc_monitor_map/local_rpc_monitor_map_test.cpp +++ b/slobrok/src/tests/local_rpc_monitor_map/local_rpc_monitor_map_test.cpp @@ -240,16 +240,33 @@ TEST_F(LocalRpcMonitorMapTest, local_add_already_up) { ASSERT_TRUE(handler_deleted); } -TEST_F(LocalRpcMonitorMapTest, local_add_already_down) { +TEST_F(LocalRpcMonitorMapTest, local_add_unknown_comes_up) { std::unique_ptr<OkState> state; bool handler_deleted; add_mapping(mapping, false); map.addLocal(mapping, std::make_unique<MyAddLocalHandler>(state, handler_deleted)); - monitor_log.expect({}); + monitor_log.expect({MonitorCall::stop(mapping), MonitorCall::start(mapping, true)}); map_log.expect({}); + ASSERT_FALSE(state); + map.up(mapping); ASSERT_TRUE(state); EXPECT_TRUE(state->ok()); ASSERT_TRUE(handler_deleted); + map_log.expect({MapCall::add(mapping)}); +} + +TEST_F(LocalRpcMonitorMapTest, local_add_unknown_goes_down) { + std::unique_ptr<OkState> state; + bool handler_deleted; + add_mapping(mapping, false); + map.addLocal(mapping, std::make_unique<MyAddLocalHandler>(state, handler_deleted)); + monitor_log.expect({MonitorCall::stop(mapping), MonitorCall::start(mapping, true)}); + map_log.expect({}); + ASSERT_FALSE(state); + map.down(mapping); + ASSERT_TRUE(state); + EXPECT_FALSE(state->ok()); + ASSERT_TRUE(handler_deleted); } TEST_F(LocalRpcMonitorMapTest, local_add_conflict) { diff --git a/slobrok/src/vespa/slobrok/server/local_rpc_monitor_map.cpp b/slobrok/src/vespa/slobrok/server/local_rpc_monitor_map.cpp index 79f796dc435..47f604a08ec 100644 --- a/slobrok/src/vespa/slobrok/server/local_rpc_monitor_map.cpp +++ b/slobrok/src/vespa/slobrok/server/local_rpc_monitor_map.cpp @@ -10,6 +10,26 @@ namespace slobrok { #pragma GCC diagnostic ignored "-Winline" +namespace { + +struct ChainedAddLocalCompletionHandler : LocalRpcMonitorMap::AddLocalCompletionHandler { + std::unique_ptr<AddLocalCompletionHandler> first; + std::unique_ptr<AddLocalCompletionHandler> second; + + ChainedAddLocalCompletionHandler(std::unique_ptr<AddLocalCompletionHandler> f, + std::unique_ptr<AddLocalCompletionHandler> s) + : first(std::move(f)), second(std::move(s)) + {} + + void doneHandler(OkState result) override { + first->doneHandler(result); + second->doneHandler(result); + } + ~ChainedAddLocalCompletionHandler() override {} +}; + +} + void LocalRpcMonitorMap::DelayedTasks::PerformTask() { std::vector<Event> todo; std::swap(todo, _queue); @@ -89,12 +109,23 @@ void LocalRpcMonitorMap::addLocal(const ServiceMapping &mapping, mapping.name.c_str(), mapping.spec.c_str()); auto old = _map.find(mapping.name); if (old != _map.end()) { - const PerService & exists = old->second; + PerService & exists = old->second; if (exists.spec == mapping.spec) { LOG(debug, "added mapping %s->%s was already present", mapping.name.c_str(), mapping.spec.c_str()); - // clear exists.ignoreFirstOk ? - inflight->doneHandler(OkState(0, "already registered")); + if (exists.up) { + inflight->doneHandler(OkState(0, "already registered")); + } else if (exists.inflight) { + auto newInflight = std::make_unique<ChainedAddLocalCompletionHandler>( + std::move(exists.inflight), + std::move(inflight)); + exists.inflight = std::move(newInflight); + } else { + _mappingMonitor->stop(mapping); + exists.inflight = std::move(inflight); + exists.ignoreFirstOk = false; + _mappingMonitor->start(mapping, true); + } return; } LOG(warning, "tried addLocal for mapping %s->%s, but already had conflicting mapping %s->%s", |