From acae3cc22348073ad5de9bac0682b0d8f89820de Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Tue, 5 Jul 2022 11:19:57 +0300 Subject: [PATCH] treewide: stop use of deprecated coroutine::make_exception Convert most use sites from `co_return coroutine::make_exception` to `co_await coroutine::return_exception{,_ptr}` where possible. In cases this is done in a catch clause, convert to `co_return coroutine::exception`, generating an exception_ptr if needed. Signed-off-by: Benny Halevy Closes #10972 --- alternator/auth.cc | 6 +++--- alternator/executor.cc | 6 +++--- cql3/statements/alter_type_statement.cc | 3 ++- cql3/statements/create_type_statement.cc | 2 +- db/commitlog/commitlog.cc | 2 +- db/schema_tables.cc | 11 ++++++----- lang/wasm.cc | 8 ++++---- service/migration_manager.cc | 18 +++++++++++------- service/storage_proxy.cc | 10 +++++----- 9 files changed, 36 insertions(+), 30 deletions(-) diff --git a/alternator/auth.cc b/alternator/auth.cc index cbcf153a17..195f8ed2ef 100644 --- a/alternator/auth.cc +++ b/alternator/auth.cc @@ -129,7 +129,7 @@ future get_key_from_roles(service::storage_proxy& proxy, std::strin std::vector bounds{query::clustering_range::make_open_ended_both_sides()}; const column_definition* salted_hash_col = schema->get_column_definition(bytes("salted_hash")); if (!salted_hash_col) { - co_return coroutine::make_exception(api_error::unrecognized_client(format("Credentials cannot be fetched for: {}", username))); + co_await coroutine::return_exception(api_error::unrecognized_client(format("Credentials cannot be fetched for: {}", username))); } auto selection = cql3::selection::selection::for_columns(schema, {salted_hash_col}); auto partition_slice = query::partition_slice(std::move(bounds), {}, query::column_id_vector{salted_hash_col->id}, selection->get_query_options()); @@ -145,11 +145,11 @@ future get_key_from_roles(service::storage_proxy& proxy, std::strin auto result_set = builder.build(); if (result_set->empty()) { - co_return coroutine::make_exception(api_error::unrecognized_client(format("User not found: {}", username))); + co_await coroutine::return_exception(api_error::unrecognized_client(format("User not found: {}", username))); } const bytes_opt& salted_hash = result_set->rows().front().front(); // We only asked for 1 row and 1 column if (!salted_hash) { - co_return coroutine::make_exception(api_error::unrecognized_client(format("No password found for user: {}", username))); + co_await coroutine::return_exception(api_error::unrecognized_client(format("No password found for user: {}", username))); } co_return value_cast(utf8_type->deserialize(*salted_hash)); } diff --git a/alternator/executor.cc b/alternator/executor.cc index b0032565af..f1cbc49e20 100644 --- a/alternator/executor.cc +++ b/alternator/executor.cc @@ -1127,7 +1127,7 @@ future executor::update_table(client_state& clien for (auto& s : unsupported) { if (rjson::find(request, s)) { - co_return coroutine::make_exception(api_error::validation(s + " not supported")); + co_await coroutine::return_exception(api_error::validation(s + " not supported")); } } @@ -1148,7 +1148,7 @@ future executor::update_table(client_state& clien // the ugly but harmless conversion to string_view here is because // Seastar's sstring is missing a find(std::string_view) :-() if (std::string_view(tab->cf_name()).find(INTERNAL_TABLE_PREFIX) == 0) { - co_return coroutine::make_exception(api_error::validation(format("Prefix {} is reserved for accessing internal tables", INTERNAL_TABLE_PREFIX))); + co_await coroutine::return_exception(api_error::validation(format("Prefix {} is reserved for accessing internal tables", INTERNAL_TABLE_PREFIX))); } schema_builder builder(tab); @@ -3320,7 +3320,7 @@ future executor::batch_get_item(client_state& cli } elogger.trace("Unprocessed keys: {}", response["UnprocessedKeys"]); if (!some_succeeded && eptr) { - co_return coroutine::make_exception(std::move(eptr)); + co_await coroutine::return_exception_ptr(std::move(eptr)); } if (is_big(response)) { co_return make_streamed(std::move(response)); diff --git a/cql3/statements/alter_type_statement.cc b/cql3/statements/alter_type_statement.cc index 3ed0924a8b..876925610c 100644 --- a/cql3/statements/alter_type_statement.cc +++ b/cql3/statements/alter_type_statement.cc @@ -117,7 +117,8 @@ alter_type_statement::prepare_schema_mutations(query_processor& qp, api::timesta co_return std::make_pair(std::move(ret), std::move(m)); } catch(data_dictionary::no_such_keyspace& e) { - co_return coroutine::make_exception(exceptions::invalid_request_exception(format("Cannot alter type in unknown keyspace {}", keyspace()))); + auto&& ex = std::make_exception_ptr(exceptions::invalid_request_exception(format("Cannot alter type in unknown keyspace {}", keyspace()))); + co_return coroutine::exception(std::move(ex)); } } diff --git a/cql3/statements/create_type_statement.cc b/cql3/statements/create_type_statement.cc index 263d5aec53..e1000b36ff 100644 --- a/cql3/statements/create_type_statement.cc +++ b/cql3/statements/create_type_statement.cc @@ -134,7 +134,7 @@ future, std::vector< _name.get_string_type_name()); } else { if (!_if_not_exists) { - co_return coroutine::make_exception(exceptions::invalid_request_exception(format("A user type of name {} already exists", _name.to_string()))); + co_await coroutine::return_exception(exceptions::invalid_request_exception(format("A user type of name {} already exists", _name.to_string()))); } } } catch (data_dictionary::no_such_keyspace& e) { diff --git a/db/commitlog/commitlog.cc b/db/commitlog/commitlog.cc index 88915a9f00..3fbfb5731f 100644 --- a/db/commitlog/commitlog.cc +++ b/db/commitlog/commitlog.cc @@ -1796,7 +1796,7 @@ future db::commitlog::segment_manager: gate::holder g(_gate); if (_shutdown) { - co_return coroutine::make_exception(std::runtime_error("Commitlog has been shut down. Cannot add data")); + co_await coroutine::return_exception(std::runtime_error("Commitlog has been shut down. Cannot add data")); } ++_new_counter; diff --git a/db/schema_tables.cc b/db/schema_tables.cc index 30a1051f35..6af780461f 100644 --- a/db/schema_tables.cc +++ b/db/schema_tables.cc @@ -816,7 +816,8 @@ future query_partition_mutation(service::storage_proxy& proxy, } else if (partitions.size() == 1) { co_return partitions[0].mut().unfreeze(s); } else { - co_return coroutine::make_exception(std::invalid_argument("Results must have at most one partition")); + auto&& ex = std::make_exception_ptr(std::invalid_argument("Results must have at most one partition")); + co_return coroutine::exception(std::move(ex)); } } @@ -1108,7 +1109,7 @@ future> extract_scylla_specific_keyspace_info(d if (proxy.local().features().cluster_schema_features().contains()) { auto&& rs = partition.second; if (rs->empty()) { - co_return coroutine::make_exception(std::runtime_error("query result has no rows")); + co_await coroutine::return_exception(std::runtime_error("query result has no rows")); } auto&& row = rs->row(0); auto keyspace_name = row.get_nonnull("keyspace_name"); @@ -2447,7 +2448,7 @@ future create_table_from_name(distributed& p auto qn = qualified_name(keyspace, table); auto sm = co_await read_table_mutations(proxy, qn, tables()); if (!sm.live()) { - co_return coroutine::make_exception(std::runtime_error(format("{}:{} not found in the schema definitions keyspace.", qn.keyspace_name, qn.table_name))); + co_await coroutine::return_exception(std::runtime_error(format("{}:{} not found in the schema definitions keyspace.", qn.keyspace_name, qn.table_name))); } co_return create_table_from_mutations(proxy, std::move(sm)); } @@ -2945,7 +2946,7 @@ static future create_view_from_table_row(distributed("keyspace_name"), row.get_nonnull("view_name")); schema_mutations sm = co_await read_table_mutations(proxy, qn, views()); if (!sm.live()) { - co_return coroutine::make_exception(std::runtime_error(format("{}:{} not found in the view definitions keyspace.", qn.keyspace_name, qn.table_name))); + co_await coroutine::return_exception(std::runtime_error(format("{}:{} not found in the view definitions keyspace.", qn.keyspace_name, qn.table_name))); } co_return create_view_from_mutations(proxy, std::move(sm)); } @@ -3307,7 +3308,7 @@ future get_column_mapping(utils::UUID table_id, table_schema_ver // If we don't have a stored column_mapping for an obsolete schema version // then it means it's way too old and been cleaned up already. // Fail the whole learn stage in this case. - co_return coroutine::make_exception(std::runtime_error( + co_await coroutine::return_exception(std::runtime_error( format("Failed to look up column mapping for schema version {}", version))); } diff --git a/lang/wasm.cc b/lang/wasm.cc index 72e7739d5f..84bb5d3642 100644 --- a/lang/wasm.cc +++ b/lang/wasm.cc @@ -305,7 +305,7 @@ seastar::future run_script(context& ctx, const std::vector // Replenish the store with initial amount of fuel auto added = store.context().add_fuel(ctx.engine_ptr->initial_fuel_amount()); if (!added) { - co_return coroutine::make_exception(wasm::exception(added.err().message())); + co_await coroutine::return_exception(wasm::exception(added.err().message())); } auto [instance, func] = create_instance_and_func(ctx, store); std::vector argv; @@ -319,7 +319,7 @@ seastar::future run_script(context& ctx, const std::vector } else if (param) { visit(type, init_arg_visitor{param, argv, store, instance}); } else { - co_return coroutine::make_exception(wasm::exception(format("Function {} cannot be called on null values", ctx.function_name))); + co_await coroutine::return_exception(wasm::exception(format("Function {} cannot be called on null values", ctx.function_name))); } } uint64_t fuel_before = *store.context().fuel_consumed(); @@ -330,11 +330,11 @@ seastar::future run_script(context& ctx, const std::vector wasm_logger.debug("Consumed {} fuel units", consumed); if (!result) { - co_return coroutine::make_exception(wasm::exception("Calling wasm function failed: " + result.err().message())); + co_await coroutine::return_exception(wasm::exception("Calling wasm function failed: " + result.err().message())); } std::vector result_vec = std::move(result).unwrap(); if (result_vec.size() != 1) { - co_return coroutine::make_exception(wasm::exception(format("Unexpected number of returned values: {} (expected: 1)", result_vec.size()))); + co_await coroutine::return_exception(wasm::exception(format("Unexpected number of returned values: {} (expected: 1)", result_vec.size()))); } // TODO: ABI for return values is experimental and subject to change in the future. diff --git a/service/migration_manager.cc b/service/migration_manager.cc index ba57c1d58a..31d77eb0b4 100644 --- a/service/migration_manager.cc +++ b/service/migration_manager.cc @@ -695,8 +695,9 @@ future> migration_manager::prepare_column_family_update_an }); co_return co_await include_keyspace(*keyspace, std::move(mutations)); } catch (const replica::no_such_column_family& e) { - co_return coroutine::make_exception(exceptions::configuration_exception(format("Cannot update non existing table '{}' in keyspace '{}'.", + auto&& ex = std::make_exception_ptr(exceptions::configuration_exception(format("Cannot update non existing table '{}' in keyspace '{}'.", cfm->cf_name(), cfm->ks_name()))); + co_return coroutine::exception(std::move(ex)); } } @@ -762,7 +763,7 @@ future> migration_manager::prepare_column_family_drop_anno auto& old_cfm = db.find_column_family(ks_name, cf_name); auto& schema = old_cfm.schema(); if (schema->is_view()) { - co_return coroutine::make_exception(exceptions::invalid_request_exception("Cannot use DROP TABLE on Materialized View")); + co_await coroutine::return_exception(exceptions::invalid_request_exception("Cannot use DROP TABLE on Materialized View")); } auto keyspace = db.find_keyspace(ks_name).metadata(); @@ -774,7 +775,7 @@ future> migration_manager::prepare_column_family_drop_anno auto explicit_view_names = views | boost::adaptors::filtered([&old_cfm](const view_ptr& v) { return !old_cfm.get_index_manager().is_index(v); }) | boost::adaptors::transformed([](const view_ptr& v) { return v->cf_name(); }); - co_return coroutine::make_exception(exceptions::invalid_request_exception(format("Cannot drop table when materialized views still depend on it ({}.{{{}}})", + co_await coroutine::return_exception(exceptions::invalid_request_exception(format("Cannot drop table when materialized views still depend on it ({}.{{{}}})", schema->ks_name(), ::join(", ", explicit_view_names)))); } mlogger.info("Drop table '{}.{}'", schema->ks_name(), schema->cf_name()); @@ -800,7 +801,8 @@ future> migration_manager::prepare_column_family_drop_anno }); co_return co_await include_keyspace(*keyspace, std::move(mutations)); } catch (const replica::no_such_column_family& e) { - co_return coroutine::make_exception(exceptions::configuration_exception(format("Cannot drop non existing table '{}' in keyspace '{}'.", cf_name, ks_name))); + auto&& ex = std::make_exception_ptr(exceptions::configuration_exception(format("Cannot drop non existing table '{}' in keyspace '{}'.", cf_name, ks_name))); + co_return coroutine::exception(std::move(ex)); } } @@ -827,7 +829,8 @@ future> migration_manager::prepare_new_view_announcement(v auto mutations = db::schema_tables::make_create_view_mutations(keyspace, std::move(view), ts); co_return co_await include_keyspace(*keyspace, std::move(mutations)); } catch (const replica::no_such_keyspace& e) { - co_return coroutine::make_exception(exceptions::configuration_exception(format("Cannot add view '{}' to non existing keyspace '{}'.", view->cf_name(), view->ks_name()))); + auto&& ex = std::make_exception_ptr(exceptions::configuration_exception(format("Cannot add view '{}' to non existing keyspace '{}'.", view->cf_name(), view->ks_name()))); + co_return coroutine::exception(std::move(ex)); } } @@ -840,7 +843,7 @@ future> migration_manager::prepare_view_update_announcemen auto&& keyspace = db.find_keyspace(view->ks_name()).metadata(); auto& old_view = keyspace->cf_meta_data().at(view->cf_name()); if (!old_view->is_view()) { - co_return coroutine::make_exception(exceptions::invalid_request_exception("Cannot use ALTER MATERIALIZED VIEW on Table")); + co_await coroutine::return_exception(exceptions::invalid_request_exception("Cannot use ALTER MATERIALIZED VIEW on Table")); } #if 0 oldCfm.validateCompatility(cfm); @@ -849,8 +852,9 @@ future> migration_manager::prepare_view_update_announcemen auto mutations = db::schema_tables::make_update_view_mutations(keyspace, view_ptr(old_view), std::move(view), ts, true); co_return co_await include_keyspace(*keyspace, std::move(mutations)); } catch (const std::out_of_range& e) { - co_return coroutine::make_exception(exceptions::configuration_exception(format("Cannot update non existing materialized view '{}' in keyspace '{}'.", + auto&& ex = std::make_exception_ptr(exceptions::configuration_exception(format("Cannot update non existing materialized view '{}' in keyspace '{}'.", view->cf_name(), view->ks_name()))); + co_return coroutine::exception(std::move(ex)); } } diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index 4a41ce1624..8f3683caa5 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -850,7 +850,7 @@ static future<> sleep_approx_50ms() { future paxos_response_handler::begin_and_repair_paxos(client_state& cs, unsigned& contentions, bool is_write) { if (!_proxy->features().lwt) { - co_return coroutine::make_exception(std::runtime_error("The cluster does not support Paxos. Upgrade all the nodes to the version with LWT support.")); + co_await coroutine::return_exception(std::runtime_error("The cluster does not support Paxos. Upgrade all the nodes to the version with LWT support.")); } api::timestamp_type min_timestamp_micros_to_use = 0; @@ -858,7 +858,7 @@ paxos_response_handler::begin_and_repair_paxos(client_state& cs, unsigned& conte while(true) { if (storage_proxy::clock_type::now() > _cas_timeout) { - co_return coroutine::make_exception( + co_await coroutine::return_exception( mutation_write_timeout_exception(_schema->ks_name(), _schema->cf_name(), _cl_for_paxos, 0, _required_participants, db::write_type::CAS) ); } @@ -915,7 +915,7 @@ paxos_response_handler::begin_and_repair_paxos(client_state& cs, unsigned& conte } catch (mutation_write_timeout_exception& e) { e.type = db::write_type::CAS; // we're still doing preparation for the paxos rounds, so we want to use the CAS (see CASSANDRA-8672) - co_return coroutine::make_exception(std::move(e)); + co_return coroutine::exception(std::make_exception_ptr(e)); } } else { paxos::paxos_state::logger.debug("CAS[{}] Some replicas have already promised a higher ballot than ours; aborting", _id); @@ -4288,7 +4288,7 @@ storage_proxy::query_singular(lw_shared_ptr cmd, const auto tmptr = get_token_metadata_ptr(); for (auto&& pr: partition_ranges) { if (!pr.is_singular()) { - co_return coroutine::make_exception(std::runtime_error("mixed singular and non singular range are not supported")); + co_await coroutine::return_exception(std::runtime_error("mixed singular and non singular range are not supported")); } auto token_range = dht::token_range::make_singular(pr.start()->value().token()); @@ -4838,7 +4838,7 @@ future storage_proxy::cas(schema_ptr schema, shared_ptr reque db::validate_for_cas_learn(cl_for_learn, schema->ks_name()); if (cas_shard(*schema, partition_ranges[0].start()->value().as_decorated_key().token()) != this_shard_id()) { - co_return coroutine::make_exception(std::logic_error("storage_proxy::cas called on a wrong shard")); + co_await coroutine::return_exception(std::logic_error("storage_proxy::cas called on a wrong shard")); } // In case a nullptr is passed to this function (i.e. the caller isn't interested in