From 29c32ed8319ac671122b47e260e3eb1b4931f6aa Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Fri, 4 Mar 2022 15:21:52 +0100 Subject: [PATCH] Refactor code --- src/IO/ParallelReadBuffer.cpp | 2 +- src/IO/ReadWriteBufferFromHTTP.h | 13 +++++++---- src/Storages/StorageURL.cpp | 39 ++++++++++++++++++++------------ 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/IO/ParallelReadBuffer.cpp b/src/IO/ParallelReadBuffer.cpp index b5a6a4d18bd..87beed32d4a 100644 --- a/src/IO/ParallelReadBuffer.cpp +++ b/src/IO/ParallelReadBuffer.cpp @@ -82,7 +82,7 @@ void ParallelReadBuffer::processor() } /// Start processing - readerThreadFunction(worker); + readerThreadFunction(std::move(worker)); } } diff --git a/src/IO/ReadWriteBufferFromHTTP.h b/src/IO/ReadWriteBufferFromHTTP.h index f81c664bb24..a379aee0b81 100644 --- a/src/IO/ReadWriteBufferFromHTTP.h +++ b/src/IO/ReadWriteBufferFromHTTP.h @@ -162,7 +162,7 @@ namespace detail range_header_value = fmt::format("bytes={}-{}", getOffset(), *read_range.end); else range_header_value = fmt::format("bytes={}-", getOffset()); - LOG_ERROR(log, "Adding header: Range: {}", range_header_value); + LOG_TEST(log, "Adding header: Range: {}", range_header_value); request.set("Range", range_header_value); } @@ -430,7 +430,8 @@ namespace detail if (next_callback) next_callback(count()); - if (read_range.end && getOffset() > read_range.end.value()) { + if (read_range.end && getOffset() > read_range.end.value()) + { assert(getOffset() == read_range.end.value() + 1); return false; } @@ -620,6 +621,7 @@ public: using Range = std::pair; + // return upper exclusive range of values, i.e. [from_range, to_range> std::optional nextRange() { if (from_range >= total_size) @@ -627,14 +629,14 @@ public: return std::nullopt; } - auto to_range = from_range + range_step - 1; + auto to_range = from_range + range_step; if (to_range >= total_size) { to_range = total_size - 1; } Range range{from_range, to_range}; - from_range = to_range + 1; + from_range = to_range; return std::move(range); } @@ -738,7 +740,8 @@ public: buffer_size, settings, http_header_entries, - ReadWriteBufferFromHTTP::Range{next_range->first, next_range->second}, + // HTTP Range has inclusive bounds, i.e. [from, to] + ReadWriteBufferFromHTTP::Range{next_range->first, next_range->second - 1}, remote_host_filter, delay_initialization, use_external_buffer, diff --git a/src/Storages/StorageURL.cpp b/src/Storages/StorageURL.cpp index 879b83fc1cb..a3efb419acb 100644 --- a/src/Storages/StorageURL.cpp +++ b/src/Storages/StorageURL.cpp @@ -259,25 +259,34 @@ namespace try { - auto http_session = makeHTTPSession(request_uri, timeouts); - auto request = Poco::Net::HTTPRequest(Poco::Net::HTTPRequest::HTTP_HEAD, request_uri.getPathAndQuery(), Poco::Net::HTTPRequest::HTTP_1_1); - request.setHost(request_uri.getHost()); // use original, not resolved host name in header - request.set("Range", "bytes=0-"); - http_session->sendRequest(request); - Poco::Net::HTTPResponse res; - receiveResponse(*http_session, request, res, true); - bool supports_ranges = res.has("Accept-Ranges") && res.get("Accept-Ranges") == "bytes"; - if (supports_ranges) { - LOG_ERROR(&Poco::Logger::get(__PRETTY_FUNCTION__), "Ranges supported"); - } else { - LOG_ERROR(&Poco::Logger::get(__PRETTY_FUNCTION__), "Ranges not supported"); + bool supports_ranges = false; + std::optional content_length; + try + { + auto http_session = makeHTTPSession(request_uri, timeouts); + auto request = Poco::Net::HTTPRequest(Poco::Net::HTTPRequest::HTTP_HEAD, request_uri.getPathAndQuery(), Poco::Net::HTTPRequest::HTTP_1_1); + request.setHost(request_uri.getHost()); // use original, not resolved host name in header + // to check if Range header is supported, we need to send a request with it set + request.set("Range", "bytes=0-"); + http_session->sendRequest(request); + Poco::Net::HTTPResponse res; + receiveResponse(*http_session, request, res, true); + supports_ranges = res.has("Accept-Ranges") && res.get("Accept-Ranges") == "bytes"; + LOG_TRACE(&Poco::Logger::get("StorageURLSource"), fmt::runtime(supports_ranges ? "HTTP Range is supported" : "HTTP Range is not supported")); + + if (res.hasContentLength()) + { + content_length.emplace(res.getContentLength()); + } + } catch (...) + { + LOG_TRACE(&Poco::Logger::get(__PRETTY_FUNCTION__), "HEAD request failed. HTTP Range cannot be used."); } - //if (!supports_ranges) - if (supports_ranges && res.hasContentLength()) + if (supports_ranges && content_length) { auto read_buffer_factory = std::make_unique( - res.getContentLength(), + *content_length, 10 * 1024 * 1024, request_uri, http_method,