From 8d490d20dcb33b29ddac5d13192f424149a6680e Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 12:33:34 +0300 Subject: [PATCH 1/6] api: Use ctx.sp in set_storage_proxy() routes It's already used in many other places, few methods still stick to global proxy usage. Signed-off-by: Pavel Emelyanov --- api/storage_proxy.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/storage_proxy.cc b/api/storage_proxy.cc index 4eb0c67d9e..12c0b55d4d 100644 --- a/api/storage_proxy.cc +++ b/api/storage_proxy.cc @@ -191,36 +191,36 @@ void set_storage_proxy(http_context& ctx, routes& r, sharded(0); }); - sp::get_hinted_handoff_enabled.set(r, [](std::unique_ptr req) { - const auto& filter = service::get_storage_proxy().local().get_hints_host_filter(); + sp::get_hinted_handoff_enabled.set(r, [&ctx](std::unique_ptr req) { + const auto& filter = ctx.sp.local().get_hints_host_filter(); return make_ready_future(!filter.is_disabled_for_all()); }); - sp::set_hinted_handoff_enabled.set(r, [](std::unique_ptr req) { + sp::set_hinted_handoff_enabled.set(r, [&ctx](std::unique_ptr req) { auto enable = req->get_query_param("enable"); auto filter = (enable == "true" || enable == "1") ? db::hints::host_filter(db::hints::host_filter::enabled_for_all_tag {}) : db::hints::host_filter(db::hints::host_filter::disabled_for_all_tag {}); - return service::get_storage_proxy().invoke_on_all([filter = std::move(filter)] (service::storage_proxy& sp) { + return ctx.sp.invoke_on_all([filter = std::move(filter)] (service::storage_proxy& sp) { return sp.change_hints_host_filter(filter); }).then([] { return make_ready_future(json_void()); }); }); - sp::get_hinted_handoff_enabled_by_dc.set(r, [](std::unique_ptr req) { + sp::get_hinted_handoff_enabled_by_dc.set(r, [&ctx](std::unique_ptr req) { std::vector res; - const auto& filter = service::get_storage_proxy().local().get_hints_host_filter(); + const auto& filter = ctx.sp.local().get_hints_host_filter(); const auto& dcs = filter.get_dcs(); res.reserve(res.size()); std::copy(dcs.begin(), dcs.end(), std::back_inserter(res)); return make_ready_future(res); }); - sp::set_hinted_handoff_enabled_by_dc_list.set(r, [](std::unique_ptr req) { + sp::set_hinted_handoff_enabled_by_dc_list.set(r, [&ctx](std::unique_ptr req) { auto dcs = req->get_query_param("dcs"); auto filter = db::hints::host_filter::parse_from_dc_list(std::move(dcs)); - return service::get_storage_proxy().invoke_on_all([filter = std::move(filter)] (service::storage_proxy& sp) { + return ctx.sp.invoke_on_all([filter = std::move(filter)] (service::storage_proxy& sp) { return sp.change_hints_host_filter(filter); }).then([] { return make_ready_future(json_void()); From 21136058bdb81bb2f17346dc1be52c25f357c598 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 12:42:43 +0300 Subject: [PATCH 2/6] api,main: Unset storage_proxy API on stop So that the routes referencing and using ctx.sp don't step on a proxy that's going to be removed (not now, but some time later) fron under them on shutdown. Signed-off-by: Pavel Emelyanov --- api/api.cc | 4 +++ api/api_init.hh | 1 + api/storage_proxy.cc | 69 ++++++++++++++++++++++++++++++++++++++++++++ api/storage_proxy.hh | 1 + main.cc | 3 ++ 5 files changed, 78 insertions(+) diff --git a/api/api.cc b/api/api.cc index 4f4d5c9f20..c04fb9080f 100644 --- a/api/api.cc +++ b/api/api.cc @@ -194,6 +194,10 @@ future<> set_server_storage_proxy(http_context& ctx, sharded unset_server_storage_proxy(http_context& ctx) { + return ctx.http_server.set_routes([&ctx] (routes& r) { unset_storage_proxy(ctx, r); }); +} + future<> set_server_stream_manager(http_context& ctx, sharded& sm) { return register_api(ctx, "stream_manager", "The stream manager API", [&sm] (http_context& ctx, routes& r) { diff --git a/api/api_init.hh b/api/api_init.hh index 8318307ae0..c3951ef483 100644 --- a/api/api_init.hh +++ b/api/api_init.hh @@ -107,6 +107,7 @@ future<> unset_server_load_sstable(http_context& ctx); future<> set_server_messaging_service(http_context& ctx, sharded& ms); future<> unset_server_messaging_service(http_context& ctx); future<> set_server_storage_proxy(http_context& ctx, sharded& ss); +future<> unset_server_storage_proxy(http_context& ctx); future<> set_server_stream_manager(http_context& ctx, sharded& sm); future<> unset_server_stream_manager(http_context& ctx); future<> set_hinted_handoff(http_context& ctx, sharded& g); diff --git a/api/storage_proxy.cc b/api/storage_proxy.cc index 12c0b55d4d..7a22a52978 100644 --- a/api/storage_proxy.cc +++ b/api/storage_proxy.cc @@ -518,4 +518,73 @@ void set_storage_proxy(http_context& ctx, routes& r, sharded& ss); +void unset_storage_proxy(http_context& ctx, httpd::routes& r); } diff --git a/main.cc b/main.cc index e74445f444..0935a5e5a7 100644 --- a/main.cc +++ b/main.cc @@ -1318,6 +1318,9 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl api::unset_server_snitch(ctx).get(); }); api::set_server_storage_proxy(ctx, ss).get(); + auto stop_sp_api = defer_verbose_shutdown("storage proxy API", [&ctx] { + api::unset_server_storage_proxy(ctx).get(); + }); api::set_server_load_sstable(ctx, sys_ks).get(); auto stop_cf_api = defer_verbose_shutdown("column family API", [&ctx] { api::unset_server_load_sstable(ctx).get(); From ece731301c54e704f3cebd0c1637174d8e36fc86 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 12:44:37 +0300 Subject: [PATCH 3/6] api: Use ctx.sp in storage service handler Similarly to previous patch, but from another routes group. The storage service API calls mainly use storage service, but one place needs proxy to call recalculate_schema_version() with Signed-off-by: Pavel Emelyanov --- api/storage_service.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/storage_service.cc b/api/storage_service.cc index 6d721798b0..0a01e404db 100644 --- a/api/storage_service.cc +++ b/api/storage_service.cc @@ -1023,12 +1023,11 @@ void set_storage_service(http_context& ctx, routes& r, sharded(res); }); - ss::reset_local_schema.set(r, [&sys_ks](std::unique_ptr req) { + ss::reset_local_schema.set(r, [&ctx, &sys_ks](std::unique_ptr req) { // FIXME: We should truncate schema tables if more than one node in the cluster. - auto& sp = service::get_storage_proxy(); - auto& fs = sp.local().features(); + auto& fs = ctx.sp.local().features(); apilog.info("reset_local_schema"); - return db::schema_tables::recalculate_schema_version(sys_ks, sp, fs).then([] { + return db::schema_tables::recalculate_schema_version(sys_ks, ctx.sp, fs).then([] { return make_ready_future(json_void()); }); }); From 257814f4431bdf69290f4cf55cd381e9d8362e96 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 12:47:27 +0300 Subject: [PATCH 4/6] view: Coroutinuze view_builder::view_build_statuses() Easier to patch it this way further. Indentation is deliberately left broken until next patch. Signed-off-by: Pavel Emelyanov --- db/view/view.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/db/view/view.cc b/db/view/view.cc index b9525c70fd..c5547d81f8 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -2094,7 +2094,7 @@ future<> view_builder::calculate_shard_build_step(view_builder_init_state& vbi) future> view_builder::view_build_statuses(sstring keyspace, sstring view_name) const { - return _sys_dist_ks.view_status(std::move(keyspace), std::move(view_name)).then([] (std::unordered_map status) { + std::unordered_map status = co_await _sys_dist_ks.view_status(std::move(keyspace), std::move(view_name)); std::unordered_map status_map; const auto& topo = service::get_local_storage_proxy().get_token_metadata_ptr()->get_topology(); topo.for_each_node([&] (const locator::node *node) { @@ -2102,8 +2102,7 @@ view_builder::view_build_statuses(sstring keyspace, sstring view_name) const { auto s = it != status.end() ? std::move(it->second) : "UNKNOWN"; status_map.emplace(node->endpoint().to_sstring(), std::move(s)); }); - return status_map; - }); + co_return status_map; } future<> view_builder::add_new_view(view_ptr view, build_step& step) { From 403463d7ebcdeec1dab8f6dd5f8da53ef1be32f7 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 12:47:37 +0300 Subject: [PATCH 5/6] view: Indentation fix after previous patch Signed-off-by: Pavel Emelyanov --- db/view/view.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/db/view/view.cc b/db/view/view.cc index c5547d81f8..f9fd2b253f 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -2095,14 +2095,14 @@ future<> view_builder::calculate_shard_build_step(view_builder_init_state& vbi) future> view_builder::view_build_statuses(sstring keyspace, sstring view_name) const { std::unordered_map status = co_await _sys_dist_ks.view_status(std::move(keyspace), std::move(view_name)); - std::unordered_map status_map; - const auto& topo = service::get_local_storage_proxy().get_token_metadata_ptr()->get_topology(); - topo.for_each_node([&] (const locator::node *node) { - auto it = status.find(node->host_id()); - auto s = it != status.end() ? std::move(it->second) : "UNKNOWN"; - status_map.emplace(node->endpoint().to_sstring(), std::move(s)); - }); - co_return status_map; + std::unordered_map status_map; + const auto& topo = service::get_local_storage_proxy().get_token_metadata_ptr()->get_topology(); + topo.for_each_node([&] (const locator::node *node) { + auto it = status.find(node->host_id()); + auto s = it != status.end() ? std::move(it->second) : "UNKNOWN"; + status_map.emplace(node->endpoint().to_sstring(), std::move(s)); + }); + co_return status_map; } future<> view_builder::add_new_view(view_ptr view, build_step& step) { From bda2aea5be4f84ecee8d4bd579645c1556b79d2d Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 12:52:21 +0300 Subject: [PATCH 6/6] view: Get topology via database tokens The view_builder::view_build_statuses() needs topology to walk its nodes. Now it gets one from global proxy via its token metadata, but database also has tokens and view_builder has reference to database. Signed-off-by: Pavel Emelyanov --- db/view/view.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/view/view.cc b/db/view/view.cc index f9fd2b253f..f7586fcf5c 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -2096,7 +2096,7 @@ future> view_builder::view_build_statuses(sstring keyspace, sstring view_name) const { std::unordered_map status = co_await _sys_dist_ks.view_status(std::move(keyspace), std::move(view_name)); std::unordered_map status_map; - const auto& topo = service::get_local_storage_proxy().get_token_metadata_ptr()->get_topology(); + const auto& topo = _db.get_token_metadata().get_topology(); topo.for_each_node([&] (const locator::node *node) { auto it = status.find(node->host_id()); auto s = it != status.end() ? std::move(it->second) : "UNKNOWN";