From: Jason Dillaman Date: Thu, 12 Nov 2020 18:03:12 +0000 (-0500) Subject: librbd/migration: move read processing into HttpClient X-Git-Tag: v16.1.0~479^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=fef41bf131212c097d41773c2b12f0fb9830b5d2;p=ceph.git librbd/migration: move read processing into HttpClient This will allow the logic to be re-used between the standard HTTP stream and the future S3 stream. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/migration/HttpClient.cc b/src/librbd/migration/HttpClient.cc index 2d0e556f76fa..8ae55ba94ccc 100644 --- a/src/librbd/migration/HttpClient.cc +++ b/src/librbd/migration/HttpClient.cc @@ -6,7 +6,10 @@ #include "common/errno.h" #include "librbd/AsioEngine.h" #include "librbd/ImageCtx.h" +#include "librbd/Utils.h" #include "librbd/asio/Utils.h" +#include "librbd/io/AioCompletion.h" +#include "librbd/io/ReadResult.h" #include "librbd/migration/Utils.h" #include #include @@ -14,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -216,8 +218,7 @@ private: std::deque> m_receive_queue; boost::beast::flat_buffer m_buffer; - std::optional> m_header_parser; + std::optional> m_header_parser; std::optional> m_parser; D& derived() { @@ -455,7 +456,7 @@ private: return; } - boost::beast::http::response response; + Response response; if (work->header_only()) { m_parser.emplace(std::move(*m_header_parser)); } @@ -534,8 +535,7 @@ private: ceph_assert(false); } - void complete_work(std::shared_ptr work, int r, - boost::beast::http::response&& response) { + void complete_work(std::shared_ptr work, int r, Response&& response) { auto cct = m_http_client->m_cct; ldout(cct, 20) << "work=" << work.get() << ", r=" << r << dendl; @@ -744,8 +744,8 @@ private: template HttpClient::HttpClient(I* image_ctx, const std::string& url) - : m_cct(image_ctx->cct), m_asio_engine(image_ctx->asio_engine), m_url(url), - m_strand(*m_asio_engine), + : m_cct(image_ctx->cct), m_image_ctx(image_ctx), + m_asio_engine(image_ctx->asio_engine), m_url(url), m_strand(*m_asio_engine), m_ssl_context(boost::asio::ssl::context::sslv23_client) { m_ssl_context.set_default_verify_paths(); } @@ -776,20 +776,18 @@ template void HttpClient::get_size(uint64_t* size, Context* on_finish) { ldout(m_cct, 10) << dendl; - boost::beast::http::request req; + boost::beast::http::request req; req.method(boost::beast::http::verb::head); issue( - std::move(req), [this, size, on_finish] - (int r, boost::beast::http::response&& response) { + std::move(req), [this, size, on_finish](int r, Response&& response) { handle_get_size(r, std::move(response), size, on_finish); }); } template -void HttpClient::handle_get_size( - int r, boost::beast::http::response&& response, - uint64_t* size, Context* on_finish) { +void HttpClient::handle_get_size(int r, Response&& response, uint64_t* size, + Context* on_finish) { ldout(m_cct, 10) << "r=" << r << dendl; if (r < 0) { @@ -825,6 +823,76 @@ void HttpClient::handle_get_size( on_finish->complete(0); } +template +void HttpClient::read(io::Extents&& byte_extents, bufferlist* data, + Context* on_finish) { + ldout(m_cct, 20) << dendl; + + auto aio_comp = io::AioCompletion::create_and_start( + on_finish, librbd::util::get_image_ctx(m_image_ctx), io::AIO_TYPE_READ); + aio_comp->set_request_count(byte_extents.size()); + + // utilize ReadResult to assemble multiple byte extents into a single bl + // since boost::beast doesn't support multipart responses out-of-the-box + io::ReadResult read_result{data}; + aio_comp->read_result = std::move(read_result); + aio_comp->read_result.set_image_extents(byte_extents); + + // issue a range get request for each extent + uint64_t buffer_offset = 0; + for (auto [byte_offset, byte_length] : byte_extents) { + auto ctx = new io::ReadResult::C_ImageReadRequest( + aio_comp, buffer_offset, {{byte_offset, byte_length}}); + buffer_offset += byte_length; + + Request req; + req.method(boost::beast::http::verb::get); + + std::stringstream range; + ceph_assert(byte_length > 0); + range << "bytes=" << byte_offset << "-" << (byte_offset + byte_length - 1); + req.set(boost::beast::http::field::range, range.str()); + + issue( + std::move(req), + [this, byte_offset=byte_offset, byte_length=byte_length, ctx] + (int r, Response&& response) { + handle_read(r, std::move(response), byte_offset, byte_length, &ctx->bl, + ctx); + }); + } +} + +template +void HttpClient::handle_read(int r, Response&& response, + uint64_t byte_offset, uint64_t byte_length, + bufferlist* data, Context* on_finish) { + ldout(m_cct, 20) << "bytes=" << byte_offset << "~" << byte_length << ", " + << "r=" << r << dendl; + + if (r < 0) { + lderr(m_cct) << "failed to read requested byte range: " + << cpp_strerror(r) << dendl; + on_finish->complete(r); + return; + } else if (response.result() != boost::beast::http::status::partial_content) { + lderr(m_cct) << "failed to retrieve requested byte range: HTTP " + << response.result() << dendl; + on_finish->complete(-EIO); + return; + } else if (byte_length != response.body().size()) { + lderr(m_cct) << "unexpected short range read: " + << "wanted=" << byte_length << ", " + << "received=" << response.body().size() << dendl; + on_finish->complete(-EINVAL); + return; + } + + data->clear(); + data->append(response.body()); + on_finish->complete(data->length()); +} + template void HttpClient::issue(std::shared_ptr&& work) { boost::asio::post(m_strand, [this, work=std::move(work)]() mutable { diff --git a/src/librbd/migration/HttpClient.h b/src/librbd/migration/HttpClient.h index d2266c73b984..79ff2c3eeb97 100644 --- a/src/librbd/migration/HttpClient.h +++ b/src/librbd/migration/HttpClient.h @@ -6,12 +6,14 @@ #include "include/common_fwd.h" #include "include/int_types.h" +#include "librbd/io/Types.h" #include "librbd/migration/Types.h" #include #include #include #include #include +#include #include #include #include @@ -32,6 +34,11 @@ namespace migration { template class HttpClient { public: + using EmptyBody = boost::beast::http::empty_body; + using StringBody = boost::beast::http::string_body; + using Request = boost::beast::http::request; + using Response = boost::beast::http::response; + static HttpClient* create(ImageCtxT* image_ctx, const std::string& url) { return new HttpClient(image_ctx, url); } @@ -45,12 +52,13 @@ public: void get_size(uint64_t* size, Context* on_finish); + void read(io::Extents&& byte_extents, bufferlist* data, + Context* on_finish); + void set_ignore_self_signed_cert(bool ignore) { m_ignore_self_signed_cert = ignore; } - using StringBody = boost::beast::http::string_body; - template void issue(boost::beast::http::request&& request, Completion&& completion) { @@ -73,8 +81,7 @@ public: return (request.method() == boost::beast::http::verb::head); } - void complete( - int r, boost::beast::http::response&& response) override { + void complete(int r, Response&& response) override { completion(r, std::move(response)); } @@ -123,8 +130,7 @@ private: virtual ~Work() {} virtual bool need_eof() const = 0; virtual bool header_only() const = 0; - virtual void complete( - int r, boost::beast::http::response&&) = 0; + virtual void complete(int r, Response&&) = 0; virtual void operator()( HttpSessionInterface* http_session, boost::beast::tcp_stream& stream) = 0; @@ -139,6 +145,7 @@ private: struct SslHttpSession; CephContext* m_cct; + ImageCtxT* m_image_ctx; std::shared_ptr m_asio_engine; std::string m_url; @@ -159,9 +166,11 @@ private: BOOST_BEAST_VERSION_STRING); } - void handle_get_size( - int r, boost::beast::http::response&& response, - uint64_t* size, Context* on_finish); + void handle_get_size(int r, Response&& response, uint64_t* size, + Context* on_finish); + + void handle_read(int r, Response&& response, uint64_t byte_offset, + uint64_t byte_length, bufferlist* data, Context* on_finish); void issue(std::shared_ptr&& work); diff --git a/src/librbd/migration/HttpStream.cc b/src/librbd/migration/HttpStream.cc index 14c19af09f65..7b8f91a325cb 100644 --- a/src/librbd/migration/HttpStream.cc +++ b/src/librbd/migration/HttpStream.cc @@ -6,10 +6,7 @@ #include "common/errno.h" #include "librbd/AsioEngine.h" #include "librbd/ImageCtx.h" -#include "librbd/Utils.h" #include "librbd/asio/Utils.h" -#include "librbd/io/AioCompletion.h" -#include "librbd/io/ReadResult.h" #include "librbd/migration/HttpClient.h" #include @@ -74,72 +71,9 @@ void HttpStream::get_size(uint64_t* size, Context* on_finish) { template void HttpStream::read(io::Extents&& byte_extents, bufferlist* data, Context* on_finish) { - using HttpRequest = boost::beast::http::request< - boost::beast::http::empty_body>; - - ldout(m_cct, 20) << dendl; - - auto aio_comp = io::AioCompletion::create_and_start( - on_finish, util::get_image_ctx(m_image_ctx), io::AIO_TYPE_READ); - aio_comp->set_request_count(byte_extents.size()); - - // utilize ReadResult to assemble multiple byte extents into a single bl - // since boost::beast doesn't support multipart responses out-of-the-box - io::ReadResult read_result{data}; - aio_comp->read_result = std::move(read_result); - aio_comp->read_result.set_image_extents(byte_extents); - - // issue a range get request for each extent - uint64_t buffer_offset = 0; - for (auto [byte_offset, byte_length] : byte_extents) { - auto ctx = new io::ReadResult::C_ImageReadRequest( - aio_comp, buffer_offset, {{byte_offset, byte_length}}); - buffer_offset += byte_length; - - HttpRequest req; - req.method(boost::beast::http::verb::get); - - std::stringstream range; - ceph_assert(byte_length > 0); - range << "bytes=" << byte_offset << "-" << (byte_offset + byte_length - 1); - req.set(boost::beast::http::field::range, range.str()); - - m_http_client->issue(std::move(req), - [this, byte_offset=byte_offset, byte_length=byte_length, ctx](int r, HttpResponse&& response) { - handle_read(r, std::move(response), byte_offset, byte_length, &ctx->bl, - ctx); - }); - } -} - -template -void HttpStream::handle_read(int r, HttpResponse&& response, - uint64_t byte_offset, uint64_t byte_length, - bufferlist* data, Context* on_finish) { - ldout(m_cct, 20) << "bytes=" << byte_offset << "~" << byte_length << ", " - << "r=" << r << dendl; - - if (r < 0) { - lderr(m_cct) << "failed to read requested byte range: " - << cpp_strerror(r) << dendl; - on_finish->complete(r); - return; - } else if (response.result() != boost::beast::http::status::partial_content) { - lderr(m_cct) << "failed to retrieve requested byte range: HTTP " - << response.result() << dendl; - on_finish->complete(-EIO); - return; - } else if (byte_length != response.body().size()) { - lderr(m_cct) << "unexpected short range read: " - << "wanted=" << byte_length << ", " - << "received=" << response.body().size() << dendl; - on_finish->complete(-EINVAL); - return; - } + ldout(m_cct, 20) << "byte_extents=" << byte_extents << dendl; - data->clear(); - data->append(response.body()); - on_finish->complete(data->length()); + m_http_client->read(std::move(byte_extents), data, on_finish); } } // namespace migration diff --git a/src/librbd/migration/HttpStream.h b/src/librbd/migration/HttpStream.h index eba28864ee36..01a583714964 100644 --- a/src/librbd/migration/HttpStream.h +++ b/src/librbd/migration/HttpStream.h @@ -58,8 +58,6 @@ private: std::unique_ptr> m_http_client; - void handle_read(int r, HttpResponse&& response, uint64_t byte_offset, - uint64_t byte_length, bufferlist* data, Context* on_finish); }; } // namespace migration diff --git a/src/test/librbd/migration/test_mock_HttpClient.cc b/src/test/librbd/migration/test_mock_HttpClient.cc index c0df4dd7848b..0a52d71a4e41 100644 --- a/src/test/librbd/migration/test_mock_HttpClient.cc +++ b/src/test/librbd/migration/test_mock_HttpClient.cc @@ -21,6 +21,14 @@ struct MockTestImageCtx : public MockImageCtx { }; } // anonymous namespace + +namespace util { + +inline ImageCtx *get_image_ctx(MockTestImageCtx *image_ctx) { + return image_ctx->image_ctx; +} + +} // namespace util } // namespace librbd #include "librbd/migration/HttpClient.cc" @@ -810,5 +818,55 @@ TEST_F(TestMockMigrationHttpClient, GetSizeError) { ASSERT_EQ(0, ctx3.wait()); } +TEST_F(TestMockMigrationHttpClient, Read) { + MockTestImageCtx mock_test_image_ctx(*m_image_ctx); + MockHttpClient http_client(&mock_test_image_ctx, + get_local_url(URL_SCHEME_HTTP)); + + boost::asio::ip::tcp::socket socket(*m_image_ctx->asio_engine); + C_SaferCond on_connect_ctx; + client_accept(&socket, false, &on_connect_ctx); + + C_SaferCond ctx1; + http_client.open(&ctx1); + ASSERT_EQ(0, on_connect_ctx.wait()); + ASSERT_EQ(0, ctx1.wait()); + + bufferlist bl; + C_SaferCond ctx2; + http_client.read({{0, 128}, {256, 64}}, &bl, &ctx2); + + HttpRequest expected_req1; + expected_req1.method(boost::beast::http::verb::get); + expected_req1.set(boost::beast::http::field::range, "bytes=0-127"); + client_read_request(socket, expected_req1); + + HttpRequest expected_req2; + expected_req2.method(boost::beast::http::verb::get); + expected_req2.set(boost::beast::http::field::range, "bytes=256-319"); + client_read_request(socket, expected_req2); + + HttpResponse expected_res1; + expected_res1.result(boost::beast::http::status::partial_content); + expected_res1.body() = std::string(128, '1'); + client_write_response(socket, expected_res1); + + HttpResponse expected_res2; + expected_res2.result(boost::beast::http::status::partial_content); + expected_res2.body() = std::string(64, '2'); + client_write_response(socket, expected_res2); + + ASSERT_EQ(192, ctx2.wait()); + + bufferlist expect_bl; + expect_bl.append(std::string(128, '1')); + expect_bl.append(std::string(64, '2')); + ASSERT_EQ(expect_bl, bl); + + C_SaferCond ctx3; + http_client.close(&ctx3); + ASSERT_EQ(0, ctx3.wait()); +} + } // namespace migration } // namespace librbd diff --git a/src/test/librbd/migration/test_mock_HttpStream.cc b/src/test/librbd/migration/test_mock_HttpStream.cc index 84e32c3ac610..aff22b757e9d 100644 --- a/src/test/librbd/migration/test_mock_HttpStream.cc +++ b/src/test/librbd/migration/test_mock_HttpStream.cc @@ -35,30 +35,9 @@ struct HttpClient { MOCK_METHOD1(open, void(Context*)); MOCK_METHOD1(close, void(Context*)); MOCK_METHOD2(get_size, void(uint64_t*, Context*)); - - MOCK_METHOD3(issue, void(const boost::beast::http::request< - boost::beast::http::empty_body>&, - boost::beast::http::response< - boost::beast::http::string_body>*, - Context*)); - - template - void issue(boost::beast::http::request&& request, - Completion&& completion) { - struct ContextImpl : public Context { - boost::beast::http::response res; - Completion completion; - - ContextImpl(Completion&& completion) : completion(std::move(completion)) { - } - - void finish(int r) override { - completion(r, std::move(res)); - } - }; - - auto ctx = new ContextImpl(std::move(completion)); - issue(request, &ctx->res, ctx); + MOCK_METHOD3(do_read, void(const io::Extents&, bufferlist*, Context*)); + void read(io::Extents&& extents, bufferlist* bl, Context* ctx) { + do_read(extents, bl, ctx); } HttpClient() { @@ -69,14 +48,6 @@ struct HttpClient { HttpClient* HttpClient::s_instance = nullptr; } // namespace migration - -namespace util { - -inline ImageCtx *get_image_ctx(MockTestImageCtx *image_ctx) { - return image_ctx->image_ctx; -} - -} // namespace util } // namespace librbd #include "librbd/migration/HttpStream.cc" @@ -87,6 +58,7 @@ namespace migration { using ::testing::_; using ::testing::Invoke; using ::testing::InSequence; +using ::testing::WithArgs; class TestMockMigrationHttpStream : public TestMockFixture { public: @@ -120,29 +92,18 @@ public: })); } - void expect_read(MockHttpClient& mock_http_client, io::Extent byte_extent, - const std::string& data, int r) { - std::stringstream byte_range; - byte_range << byte_extent.first << "-" - << (byte_extent.first + byte_extent.second - 1); - - EXPECT_CALL(mock_http_client, issue(_, _, _)) - .WillOnce(Invoke( - [data, r, byte_range=byte_range.str()] - (const boost::beast::http::request< - boost::beast::http::empty_body>& request, - boost::beast::http::response< - boost::beast::http::string_body>* response, Context* ctx) { - ASSERT_EQ("bytes=" + byte_range, - request[boost::beast::http::field::range]); - - response->result(boost::beast::http::status::partial_content); - response->set(boost::beast::http::field::content_range, - "bytes " + byte_range + "/*"); - response->body() = data; - response->prepare_payload(); - ctx->complete(r); - })); + void expect_read(MockHttpClient& mock_http_client, io::Extents byte_extents, + const bufferlist& bl, int r) { + uint64_t len = 0; + for (auto [_, byte_len] : byte_extents) { + len += byte_len; + } + EXPECT_CALL(mock_http_client, do_read(byte_extents, _, _)) + .WillOnce(WithArgs<1, 2>(Invoke( + [len, bl, r](bufferlist* out_bl, Context* ctx) { + *out_bl = bl; + ctx->complete(r < 0 ? r : len); + }))); } json_spirit::mObject json_object; @@ -206,8 +167,9 @@ TEST_F(TestMockMigrationHttpStream, Read) { auto mock_http_client = new MockHttpClient(); expect_open(*mock_http_client, 0); - expect_read(*mock_http_client, {0, 128}, std::string(128, '1'), 0); - expect_read(*mock_http_client, {256, 64}, std::string(64, '2'), 0); + bufferlist expect_bl; + expect_bl.append(std::string(192, '1')); + expect_read(*mock_http_client, {{0, 128}, {256, 64}}, expect_bl, 0); expect_close(*mock_http_client, 0); @@ -221,10 +183,6 @@ TEST_F(TestMockMigrationHttpStream, Read) { bufferlist bl; mock_http_stream.read({{0, 128}, {256, 64}}, &bl, &ctx2); ASSERT_EQ(192, ctx2.wait()); - - bufferlist expect_bl; - expect_bl.append(std::string(128, '1')); - expect_bl.append(std::string(64, '2')); ASSERT_EQ(expect_bl, bl); C_SaferCond ctx3;