From acf5264bcdd74aba6c1dd4644a95641b5b02ac05 Mon Sep 17 00:00:00 2001 From: Alberto Miranda Date: Wed, 23 Nov 2022 19:54:47 +0100 Subject: [PATCH] API: Refactor ADM_remove_pfs_storage() --- examples/cxx/ADM_remove_pfs_storage.cpp | 21 +++++++-------- src/common/net/proto/rpc_types.h | 11 ++++++-- src/lib/admire.cpp | 26 +++++------------- src/lib/admire.hpp | 4 +-- src/lib/c_wrapper.cpp | 6 ++--- src/lib/detail/impl.cpp | 35 +++++++++++++++++++++++++ src/lib/detail/impl.hpp | 3 +++ src/scord/rpc_handlers.cpp | 23 +++++++++++++--- 8 files changed, 87 insertions(+), 42 deletions(-) diff --git a/examples/cxx/ADM_remove_pfs_storage.cpp b/examples/cxx/ADM_remove_pfs_storage.cpp index 11e887df..d03833a4 100644 --- a/examples/cxx/ADM_remove_pfs_storage.cpp +++ b/examples/cxx/ADM_remove_pfs_storage.cpp @@ -37,24 +37,23 @@ main(int argc, char* argv[]) { admire::server server{"tcp", argv[1]}; - ADM_pfs_storage_t pfs_storage{}; - ADM_return_t ret = ADM_SUCCESS; + std::string pfs_name = "gpfs_scratch"; + std::string pfs_mount = "/gpfs/scratch"; try { - ret = admire::remove_pfs_storage(server, pfs_storage); + const auto pfs_storage = admire::register_pfs_storage( + server, pfs_name, admire::pfs_storage::type::gpfs, + admire::pfs_storage::ctx{pfs_mount}); + + admire::remove_pfs_storage(server, pfs_storage); } catch(const std::exception& e) { - fmt::print(stderr, "FATAL: ADM_remove_pfs_storage() failed: {}\n", + fmt::print(stderr, + "FATAL: ADM_register_pfs_storage() or " + "ADM_remove_pfs_storage() failed: {}\n", e.what()); exit(EXIT_FAILURE); } - if(ret != ADM_SUCCESS) { - fmt::print(stdout, - "ADM_remove_pfs_storage() remote procedure not completed " - "successfully\n"); - exit(EXIT_FAILURE); - } - fmt::print(stdout, "ADM_remove_pfs_storage() remote procedure completed " "successfully\n"); } diff --git a/src/common/net/proto/rpc_types.h b/src/common/net/proto/rpc_types.h index 952e0df1..cf9b5187 100644 --- a/src/common/net/proto/rpc_types.h +++ b/src/common/net/proto/rpc_types.h @@ -416,9 +416,16 @@ MERCURY_GEN_PROC( ); /// ADM_remove_pfs_storage -MERCURY_GEN_PROC(ADM_remove_pfs_storage_in_t, ((int32_t) (reqs))) +MERCURY_GEN_PROC( + ADM_remove_pfs_storage_in_t, + ((hg_uint64_t) (server_id)) +); -MERCURY_GEN_PROC(ADM_remove_pfs_storage_out_t, ((int32_t) (ret))) +MERCURY_GEN_PROC( + ADM_remove_pfs_storage_out_t, + ((hg_uint64_t) (op_id)) + ((hg_int32_t) (retval)) +); /// ADM_input MERCURY_GEN_PROC(ADM_input_in_t, diff --git a/src/lib/admire.cpp b/src/lib/admire.cpp index 8edaa4f8..84f54de7 100644 --- a/src/lib/admire.cpp +++ b/src/lib/admire.cpp @@ -322,29 +322,15 @@ update_pfs_storage(const server& srv, const pfs_storage& pfs_storage, } } -ADM_return_t -remove_pfs_storage(const server& srv, ADM_pfs_storage_t pfs_storage) { - (void) srv; - (void) pfs_storage; - - scord::network::rpc_client rpc_client{srv.protocol(), rpc_registration_cb}; - - auto endp = rpc_client.lookup(srv.address()); - - LOGGER_INFO("ADM_remove_pfs_storage(...)"); - - ADM_remove_pfs_storage_in_t in{}; - ADM_remove_pfs_storage_out_t out; +void +remove_pfs_storage(const server& srv, const pfs_storage& pfs_storage) { - const auto rpc = endp.call("ADM_remove_pfs_storage", &in, &out); + const auto ec = detail::remove_pfs_storage(srv, pfs_storage); - if(out.ret < 0) { - LOGGER_ERROR("ADM_remove_pfs_storage() = {}", out.ret); - return static_cast(out.ret); + if(!ec) { + throw std::runtime_error(fmt::format( + "ADM_remove_pfs_storage() error: {}", ec.message())); } - - LOGGER_INFO("ADM_remove_pfs_storage() = {}", ADM_SUCCESS); - return ADM_SUCCESS; } admire::transfer diff --git a/src/lib/admire.hpp b/src/lib/admire.hpp index 0b7a61a7..a7106a9d 100644 --- a/src/lib/admire.hpp +++ b/src/lib/admire.hpp @@ -81,8 +81,8 @@ void update_pfs_storage(const server& srv, const pfs_storage& pfs_storage, const admire::pfs_storage::ctx& pfs_storage_ctx); -ADM_return_t -remove_pfs_storage(const server& srv, ADM_pfs_storage_t pfs_storage); +void +remove_pfs_storage(const server& srv, const pfs_storage& pfs_storage); admire::transfer transfer_datasets(const server& srv, const job& job, diff --git a/src/lib/c_wrapper.cpp b/src/lib/c_wrapper.cpp index 01bf9112..c608ae47 100644 --- a/src/lib/c_wrapper.cpp +++ b/src/lib/c_wrapper.cpp @@ -162,10 +162,8 @@ ADM_update_pfs_storage(ADM_server_t server, ADM_pfs_storage_t pfs_storage, ADM_return_t ADM_remove_pfs_storage(ADM_server_t server, ADM_pfs_storage_t pfs_storage) { - - const admire::server srv{server}; - - return admire::remove_pfs_storage(srv, pfs_storage); + return admire::detail::remove_pfs_storage(admire::server{server}, + admire::pfs_storage{pfs_storage}); } ADM_return_t diff --git a/src/lib/detail/impl.cpp b/src/lib/detail/impl.cpp index 5f915abd..962f1f92 100644 --- a/src/lib/detail/impl.cpp +++ b/src/lib/detail/impl.cpp @@ -594,4 +594,39 @@ update_pfs_storage(const server& srv, const pfs_storage& pfs_storage, return admire::error_code::success; } +admire::error_code +remove_pfs_storage(const server& srv, const pfs_storage& pfs_storage) { + + scord::network::rpc_client rpc_client{srv.protocol(), rpc_registration_cb}; + + const auto rpc_id = ::api::remote_procedure::new_id(); + auto endp = rpc_client.lookup(srv.address()); + + LOGGER_INFO("rpc id: {} name: {} from: {} => " + "body: {{pfs_storage_id: {}}}", + rpc_id, std::quoted("ADM_"s + __FUNCTION__), + std::quoted(rpc_client.self_address()), pfs_storage.id()); + + ADM_remove_pfs_storage_in_t in{pfs_storage.id()}; + ADM_remove_pfs_storage_out_t out; + + const auto rpc = endp.call("ADM_remove_pfs_storage", &in, &out); + + if(const auto rv = admire::error_code{out.retval}; !rv) { + LOGGER_ERROR("rpc id: {} name: {} from: {} <= " + "body: {{retval: {}}} [op_id: {}]", + rpc_id, std::quoted("ADM_"s + __FUNCTION__), + std::quoted(rpc.origin()), rv, out.op_id); + return rv; + } + + LOGGER_INFO("rpc id: {} name: {} from: {} <= " + "body: {{retval: {}}} [op_id: {}]", + rpc_id, std::quoted("ADM_"s + __FUNCTION__), + std::quoted(rpc.origin()), admire::error_code::success, + out.op_id); + + return admire::error_code::success; +} + } // namespace admire::detail diff --git a/src/lib/detail/impl.hpp b/src/lib/detail/impl.hpp index bf595d15..52aafcd0 100644 --- a/src/lib/detail/impl.hpp +++ b/src/lib/detail/impl.hpp @@ -76,6 +76,9 @@ admire::error_code update_pfs_storage(const server& srv, const pfs_storage& pfs_storage, const admire::pfs_storage::ctx& pfs_storage_ctx); +admire::error_code +remove_pfs_storage(const server& srv, const pfs_storage& pfs_storage); + } // namespace admire::detail #endif // SCORD_ADMIRE_IMPL_HPP diff --git a/src/scord/rpc_handlers.cpp b/src/scord/rpc_handlers.cpp index 31b09b48..2c530742 100644 --- a/src/scord/rpc_handlers.cpp +++ b/src/scord/rpc_handlers.cpp @@ -675,11 +675,28 @@ ADM_remove_pfs_storage(hg_handle_t h) { ret = margo_get_input(h, &in); assert(ret == HG_SUCCESS); - out.ret = -1; + const auto rpc_id = remote_procedure::new_id(); + LOGGER_INFO("rpc id: {} name: {} from: {} => " + "body: {{pfs_storage_id: {}}}", + rpc_id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), + in.server_id); - LOGGER_INFO("ADM_remove_pfs_storage()"); - out.ret = 0; + auto& pfs_manager = scord::pfs_storage_manager::instance(); + admire::error_code ec = pfs_manager.remove(in.server_id); + + if(!ec) { + LOGGER_ERROR("rpc id: {} error_msg: \"Error removing pfs storage: {}\"", + rpc_id, in.server_id); + } + + out.op_id = rpc_id; + out.retval = ec; + + LOGGER_INFO("rpc id: {} name: {} to: {} <= " + "body: {{retval: {}}}", + rpc_id, std::quoted(__FUNCTION__), std::quoted(get_address(h)), + ec); ret = margo_respond(h, &out); assert(ret == HG_SUCCESS); -- GitLab