From 29738f0cb62166aef4dd914ea1382f33af4be4c4 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 10 Jul 2024 08:30:33 +0300 Subject: [PATCH 1/6] api: Move snitch API registration next to snitch itself Once sharded is started, it can register its handlers Signed-off-by: Pavel Emelyanov --- main.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/main.cc b/main.cc index 38f20dafcd..69bc484cea 100644 --- a/main.cc +++ b/main.cc @@ -931,6 +931,11 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl // #293 - do not stop anything (unless snitch.on_all(start) fails) stop_snitch->cancel(); + api::set_server_snitch(ctx, snitch).get(); + auto stop_snitch_api = defer_verbose_shutdown("snitch API", [&ctx] { + api::unset_server_snitch(ctx).get(); + }); + if (auto opt_public_address = snitch.local()->get_public_address()) { // Use the Public IP as broadcast_address to other nodes // and the broadcast_rpc_address (for client CQL connections). @@ -1665,10 +1670,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl }); }).get(); - api::set_server_snitch(ctx, snitch).get(); - auto stop_snitch_api = defer_verbose_shutdown("snitch API", [&ctx] { - api::unset_server_snitch(ctx).get(); - }); api::set_server_column_family(ctx, sys_ks).get(); auto stop_cf_api = defer_verbose_shutdown("column family API", [&ctx] { api::unset_server_column_family(ctx).get(); From 10566256fdd7372130208b825286a9aec2370068 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 22 Jul 2024 12:14:08 +0300 Subject: [PATCH 2/6] api: Do not return zero local host-id The local host id is read from local token metadata and returned to the caller as string. The t.m. itself starts with default-constructed host id vlaue which is updated later. However, even such "unset" host id value can be rendered as string without errors. This makes the correct work of the API endpoint depend on the initialization sequence which may (spoilter: it will) change in the future. Signed-off-by: Pavel Emelyanov --- api/token_metadata.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/token_metadata.cc b/api/token_metadata.cc index f20a8a2107..f6f9ddcd63 100644 --- a/api/token_metadata.cc +++ b/api/token_metadata.cc @@ -21,6 +21,9 @@ using namespace json; void set_token_metadata(http_context& ctx, routes& r, sharded& tm) { ss::local_hostid.set(r, [&tm](std::unique_ptr req) { auto id = tm.local().get()->get_my_id(); + if (!bool(id)) { + throw not_found_exception("local host ID is not yet set"); + } return make_ready_future(id.to_sstring()); }); From 6ae09cc6bfc91bb7ebbb05bbaf665222caa8a571 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 10 Jul 2024 08:34:38 +0300 Subject: [PATCH 3/6] api: Register token-metadata API next to token-metadata itsels Right now API registration happens quite late because it waits storage service to register its "function" first. This can be done beforeheand and the t.m. API can be moved to where it should be. Signed-off-by: Pavel Emelyanov --- api/api.cc | 4 +++- main.cc | 13 ++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/api/api.cc b/api/api.cc index dfbe9eee32..e92cd14591 100644 --- a/api/api.cc +++ b/api/api.cc @@ -73,6 +73,8 @@ future<> set_server_init(http_context& ctx) { set_error_injection(ctx, r); rb->register_function(r, "storage_proxy", "The storage proxy API"); + rb->register_function(r, "storage_service", + "The storage service API"); }); } @@ -115,7 +117,7 @@ future<> unset_thrift_controller(http_context& ctx) { } future<> set_server_storage_service(http_context& ctx, sharded& ss, service::raft_group0_client& group0_client) { - return register_api(ctx, "storage_service", "The storage service API", [&ss, &group0_client] (http_context& ctx, routes& r) { + return ctx.http_server.set_routes([&ctx, &ss, &group0_client] (routes& r) { set_storage_service(ctx, r, ss, group0_client); }); } diff --git a/main.cc b/main.cc index 69bc484cea..f65519a09d 100644 --- a/main.cc +++ b/main.cc @@ -985,6 +985,12 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl // token_metadata.stop().get(); //}); + api::set_server_token_metadata(ctx, token_metadata).get(); + auto stop_tokens_api = defer_verbose_shutdown("token metadata API", [&ctx] { + api::unset_server_token_metadata(ctx).get(); + }); + + supervisor::notify("starting effective_replication_map factory"); erm_factory.start().get(); auto stop_erm_factory = deferred_stop(erm_factory); @@ -1492,13 +1498,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl tablet_allocator.stop().get(); }); - // FIXME -- this can happen next to token_metadata start, but it needs "storage_service" - // API register, so it comes that late for now - api::set_server_token_metadata(ctx, token_metadata).get(); - auto stop_tokens_api = defer_verbose_shutdown("token metadata API", [&ctx] { - api::unset_server_token_metadata(ctx).get(); - }); - supervisor::notify("starting mapreduce service"); mapreduce_service.start(std::ref(messaging), std::ref(proxy), std::ref(db), std::ref(token_metadata), std::ref(stop_signal.as_sharded_abort_source())).get(); auto stop_mapreduce_service_handlers = defer_verbose_shutdown("mapreduce service", [&mapreduce_service] { From e1eb48f9c291a8dc365391d8844509f955432fa3 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 10 Jul 2024 08:36:32 +0300 Subject: [PATCH 4/6] api: Move storage API few steps above The sequence currently is sharded.start() sharded.invoke_on_all(start_remote) api::set_server_storage_service() The last two steps can be safely swapped to keep storage service API next to its service. Signed-off-by: Pavel Emelyanov --- main.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/main.cc b/main.cc index f65519a09d..f9e39a4056 100644 --- a/main.cc +++ b/main.cc @@ -1538,6 +1538,11 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl ss.stop().get(); }); + api::set_server_storage_service(ctx, ss, group0_client).get(); + auto stop_ss_api = defer_verbose_shutdown("storage service API", [&ctx] { + api::unset_server_storage_service(ctx).get(); + }); + supervisor::notify("initializing query processor remote part"); // TODO: do this together with proxy.start_remote(...) qp.invoke_on_all(&cql3::query_processor::start_remote, std::ref(mm), std::ref(mapreduce_service), @@ -1546,11 +1551,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl qp.invoke_on_all(&cql3::query_processor::stop_remote).get(); }); - api::set_server_storage_service(ctx, ss, group0_client).get(); - auto stop_ss_api = defer_verbose_shutdown("storage service API", [&ctx] { - api::unset_server_storage_service(ctx).get(); - }); - supervisor::notify("initializing virtual tables"); smp::invoke_on_all([&] { return db::initialize_virtual_tables(db, ss, gossiper, raft_gr, sys_ks, *cfg); From 61fb0ad9964c8ab2ecbe112ccaafa21b8b48f72a Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 10 Jul 2024 08:39:43 +0300 Subject: [PATCH 5/6] main: Don't ignore set_cache_service() future The call itself seem to be in wrong place -- there's no "cache service" also the API uses database and snapshot_ctl to work on. So it deserves more cleanup, but at least don't throw the returned future<> away. Signed-off-by: Pavel Emelyanov --- main.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main.cc b/main.cc index f9e39a4056..140565fe78 100644 --- a/main.cc +++ b/main.cc @@ -1828,8 +1828,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl api::unset_server_tasks_compaction_module(ctx).get(); }); - //FIXME: discarded future - (void)api::set_server_cache(ctx); + api::set_server_cache(ctx).get(); if (cfg->maintenance_mode()) { startlog.info("entering maintenance mode."); From 456dbc122b8aea6ab08dfbf91c6d3857d7cd75cb Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 10 Jul 2024 12:52:06 +0300 Subject: [PATCH 6/6] api: Unset cache_service endpoints on stop They currently stay registered long after the dependent services get stopped. There's a need for batch unsetting (scylladb/seastar#1620), so currently only this explicit listing :( Signed-off-by: Pavel Emelyanov --- api/api.cc | 4 ++++ api/api_init.hh | 1 + api/cache_service.cc | 45 ++++++++++++++++++++++++++++++++++++++++++++ api/cache_service.hh | 1 + main.cc | 3 +++ 5 files changed, 54 insertions(+) diff --git a/api/api.cc b/api/api.cc index e92cd14591..1cfa638636 100644 --- a/api/api.cc +++ b/api/api.cc @@ -258,6 +258,10 @@ future<> set_server_cache(http_context& ctx) { "The cache service API", set_cache_service); } +future<> unset_server_cache(http_context& ctx) { + return ctx.http_server.set_routes([&ctx] (routes& r) { unset_cache_service(ctx, r); }); +} + future<> set_hinted_handoff(http_context& ctx, sharded& proxy) { return register_api(ctx, "hinted_handoff", "The hinted handoff API", [&proxy] (http_context& ctx, routes& r) { diff --git a/api/api_init.hh b/api/api_init.hh index e0fc19d618..b9abb30d50 100644 --- a/api/api_init.hh +++ b/api/api_init.hh @@ -119,6 +119,7 @@ future<> unset_server_stream_manager(http_context& ctx); future<> set_hinted_handoff(http_context& ctx, sharded& p); future<> unset_hinted_handoff(http_context& ctx); future<> set_server_cache(http_context& ctx); +future<> unset_server_cache(http_context& ctx); future<> set_server_compaction_manager(http_context& ctx); future<> set_server_done(http_context& ctx); future<> set_server_task_manager(http_context& ctx, sharded& tm, lw_shared_ptr cfg); diff --git a/api/cache_service.cc b/api/cache_service.cc index 425704a9cb..fa16e5e95e 100644 --- a/api/cache_service.cc +++ b/api/cache_service.cc @@ -320,5 +320,50 @@ void set_cache_service(http_context& ctx, routes& r) { }); } +void unset_cache_service(http_context& ctx, routes& r) { + cs::get_row_cache_save_period_in_seconds.unset(r); + cs::set_row_cache_save_period_in_seconds.unset(r); + cs::get_key_cache_save_period_in_seconds.unset(r); + cs::set_key_cache_save_period_in_seconds.unset(r); + cs::get_counter_cache_save_period_in_seconds.unset(r); + cs::set_counter_cache_save_period_in_seconds.unset(r); + cs::get_row_cache_keys_to_save.unset(r); + cs::set_row_cache_keys_to_save.unset(r); + cs::get_key_cache_keys_to_save.unset(r); + cs::set_key_cache_keys_to_save.unset(r); + cs::get_counter_cache_keys_to_save.unset(r); + cs::set_counter_cache_keys_to_save.unset(r); + cs::invalidate_key_cache.unset(r); + cs::invalidate_counter_cache.unset(r); + cs::set_row_cache_capacity_in_mb.unset(r); + cs::set_key_cache_capacity_in_mb.unset(r); + cs::set_counter_cache_capacity_in_mb.unset(r); + cs::save_caches.unset(r); + cs::get_key_capacity.unset(r); + cs::get_key_hits.unset(r); + cs::get_key_requests.unset(r); + cs::get_key_hit_rate.unset(r); + cs::get_key_hits_moving_avrage.unset(r); + cs::get_key_requests_moving_avrage.unset(r); + cs::get_key_size.unset(r); + cs::get_key_entries.unset(r); + cs::get_row_capacity.unset(r); + cs::get_row_hits.unset(r); + cs::get_row_requests.unset(r); + cs::get_row_hit_rate.unset(r); + cs::get_row_hits_moving_avrage.unset(r); + cs::get_row_requests_moving_avrage.unset(r); + cs::get_row_size.unset(r); + cs::get_row_entries.unset(r); + cs::get_counter_capacity.unset(r); + cs::get_counter_hits.unset(r); + cs::get_counter_requests.unset(r); + cs::get_counter_hit_rate.unset(r); + cs::get_counter_hits_moving_avrage.unset(r); + cs::get_counter_requests_moving_avrage.unset(r); + cs::get_counter_size.unset(r); + cs::get_counter_entries.unset(r); +} + } diff --git a/api/cache_service.hh b/api/cache_service.hh index 0165ec3e0d..ac438af3bc 100644 --- a/api/cache_service.hh +++ b/api/cache_service.hh @@ -16,5 +16,6 @@ namespace api { struct http_context; void set_cache_service(http_context& ctx, seastar::httpd::routes& r); +void unset_cache_service(http_context& ctx, seastar::httpd::routes& r); } diff --git a/main.cc b/main.cc index 140565fe78..75ea339aa2 100644 --- a/main.cc +++ b/main.cc @@ -1829,6 +1829,9 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl }); api::set_server_cache(ctx).get(); + auto stop_cache_api = defer_verbose_shutdown("cache API", [&ctx] { + api::unset_server_cache(ctx).get(); + }); if (cfg->maintenance_mode()) { startlog.info("entering maintenance mode.");