From 31a64cfb6558944440e735eb891251c0acbf8192 Mon Sep 17 00:00:00 2001 From: Mytherin Date: Thu, 15 May 2025 22:03:46 +0200 Subject: [PATCH] Apply logger patch --- duckdb | 2 +- extension/httpfs/create_secret_functions.cpp | 2 +- extension/httpfs/httpfs.cpp | 29 +++++++++++++++----- extension/httpfs/httpfs_client.cpp | 7 ----- extension/httpfs/include/httpfs_client.hpp | 6 ++-- extension/httpfs/s3fs.cpp | 8 ++++-- test/sql/secret/secret_refresh.test | 6 ++-- test/sql/secret/secret_refresh_attach.test | 2 +- 8 files changed, 38 insertions(+), 24 deletions(-) diff --git a/duckdb b/duckdb index 3c0d4669..44d08560 160000 --- a/duckdb +++ b/duckdb @@ -1 +1 @@ -Subproject commit 3c0d466943444de3cfb5fcf2db08e5c4a6eb6c5e +Subproject commit 44d0856021c69743661adaea0006725fdbf7db29 diff --git a/extension/httpfs/create_secret_functions.cpp b/extension/httpfs/create_secret_functions.cpp index d59c8af1..7929cdb4 100644 --- a/extension/httpfs/create_secret_functions.cpp +++ b/extension/httpfs/create_secret_functions.cpp @@ -158,7 +158,7 @@ bool CreateS3SecretFunctions::TryRefreshS3Secret(ClientContext &context, const S try { auto res = secret_manager.CreateSecret(context, refresh_input); auto &new_secret = dynamic_cast(*res->secret); - DUCKDB_LOG_INFO(context, "httpfs.SecretRefresh", "Successfully refreshed secret: %s, new key_id: %s", + DUCKDB_LOG_INFO(context, "Successfully refreshed secret: %s, new key_id: %s", secret_to_refresh.secret->GetName(), new_secret.TryGetValue("key_id").ToString()); return true; } catch (std::exception &ex) { diff --git a/extension/httpfs/httpfs.cpp b/extension/httpfs/httpfs.cpp index b1115629..34bcc5d1 100644 --- a/extension/httpfs/httpfs.cpp +++ b/extension/httpfs/httpfs.cpp @@ -8,7 +8,7 @@ #include "duckdb/common/thread.hpp" #include "duckdb/common/types/hash.hpp" #include "duckdb/function/scalar/strftime_format.hpp" -#include "duckdb/logging/http_logger.hpp" +#include "duckdb/logging/file_system_logger.hpp" #include "duckdb/main/client_context.hpp" #include "duckdb/main/database.hpp" #include "duckdb/main/secret/secret_manager.hpp" @@ -32,7 +32,8 @@ shared_ptr HTTPFSUtil::GetHTTPUtil(optional_ptr opener) { return make_shared_ptr(); } -unique_ptr HTTPFSUtil::InitializeParameters(optional_ptr opener, optional_ptr info) { +unique_ptr HTTPFSUtil::InitializeParameters(optional_ptr opener, + optional_ptr info) { auto result = make_uniq(*this); result->Initialize(opener); @@ -277,8 +278,8 @@ void TimestampToTimeT(timestamp_t timestamp, time_t &result) { HTTPFileHandle::HTTPFileHandle(FileSystem &fs, const OpenFileInfo &file, FileOpenFlags flags, unique_ptr params_p) - : FileHandle(fs, file.path, flags), params(std::move(params_p)), http_params(params->Cast()), flags(flags), length(0), - buffer_available(0), buffer_idx(0), file_offset(0), buffer_start(0), buffer_end(0) { + : FileHandle(fs, file.path, flags), params(std::move(params_p)), http_params(params->Cast()), + flags(flags), length(0), buffer_available(0), buffer_idx(0), file_offset(0), buffer_start(0), buffer_end(0) { // check if the handle has extended properties that can be set directly in the handle // if we have these properties we don't need to do a head request to obtain them later if (file.extended_info) { @@ -342,6 +343,9 @@ unique_ptr HTTPFileSystem::OpenFileExtended(const OpenFileInfo &file auto handle = CreateHandle(file, flags, opener); handle->Initialize(opener); + + DUCKDB_LOG_FILE_SYSTEM_OPEN((*handle)); + return std::move(handle); } @@ -356,6 +360,8 @@ void HTTPFileSystem::Read(FileHandle &handle, void *buffer, int64_t nr_bytes, id throw InternalException("Cached file not initialized properly"); } memcpy(buffer, hfh.cached_file_handle->GetData() + location, nr_bytes); + DUCKDB_LOG_FILE_SYSTEM_READ(handle, nr_bytes, location); + hfh.file_offset = location + nr_bytes; return; } @@ -366,17 +372,19 @@ void HTTPFileSystem::Read(FileHandle &handle, void *buffer, int64_t nr_bytes, id bool skip_buffer = hfh.flags.DirectIO() || hfh.flags.RequireParallelAccess(); if (skip_buffer && to_read > 0) { GetRangeRequest(hfh, hfh.path, {}, location, (char *)buffer, to_read); - + DUCKDB_LOG_FILE_SYSTEM_READ(handle, nr_bytes, location); // Update handle status within critical section for parallel access. if (hfh.flags.RequireParallelAccess()) { std::lock_guard lck(hfh.mu); hfh.buffer_available = 0; hfh.buffer_idx = 0; + hfh.file_offset = location + nr_bytes; return; } hfh.buffer_available = 0; hfh.buffer_idx = 0; + hfh.file_offset = location + nr_bytes; return; } @@ -423,6 +431,8 @@ void HTTPFileSystem::Read(FileHandle &handle, void *buffer, int64_t nr_bytes, id } } } + hfh.file_offset = location + nr_bytes; + DUCKDB_LOG_FILE_SYSTEM_READ(handle, nr_bytes, location); } int64_t HTTPFileSystem::Read(FileHandle &handle, void *buffer, int64_t nr_bytes) { @@ -430,7 +440,6 @@ int64_t HTTPFileSystem::Read(FileHandle &handle, void *buffer, int64_t nr_bytes) idx_t max_read = hfh.length - hfh.file_offset; nr_bytes = MinValue(max_read, nr_bytes); Read(handle, buffer, nr_bytes, hfh.file_offset); - hfh.file_offset += nr_bytes; return nr_bytes; } @@ -642,6 +651,10 @@ void HTTPFileHandle::Initialize(optional_ptr opener) { http_params.state = make_shared_ptr(); } + if (opener) { + TryAddLogger(*opener); + } + auto current_cache = TryGetMetadataCache(opener, hfs); bool should_write_cache = false; @@ -711,5 +724,7 @@ void HTTPFileHandle::StoreClient(unique_ptr client) { client_cache.StoreClient(std::move(client)); } -HTTPFileHandle::~HTTPFileHandle() = default; +HTTPFileHandle::~HTTPFileHandle() { + DUCKDB_LOG_FILE_SYSTEM_CLOSE((*this)); +}; } // namespace duckdb diff --git a/extension/httpfs/httpfs_client.cpp b/extension/httpfs/httpfs_client.cpp index bf87668e..7a779ef9 100644 --- a/extension/httpfs/httpfs_client.cpp +++ b/extension/httpfs/httpfs_client.cpp @@ -1,6 +1,5 @@ #include "httpfs_client.hpp" #include "http_state.hpp" -#include "duckdb/logging/http_logger.hpp" #define CPPHTTPLIB_OPENSSL_SUPPORT #include "httplib.hpp" @@ -21,9 +20,6 @@ class HTTPFSClient : public HTTPClient { client->set_read_timeout(http_params.timeout, http_params.timeout_usec); client->set_connection_timeout(http_params.timeout, http_params.timeout_usec); client->set_decompress(false); - if (http_params.logger) { - SetLogger(*http_params.logger); - } if (!http_params.bearer_token.empty()) { client->set_bearer_token_auth(http_params.bearer_token.c_str()); } @@ -38,9 +34,6 @@ class HTTPFSClient : public HTTPClient { state = http_params.state; } - void SetLogger(HTTPLogger &logger) { - client->set_logger(logger.GetLogger()); - } unique_ptr Get(GetRequestInfo &info) override { if (state) { state->get_count++; diff --git a/extension/httpfs/include/httpfs_client.hpp b/extension/httpfs/include/httpfs_client.hpp index 1a2be770..1d7620cf 100644 --- a/extension/httpfs/include/httpfs_client.hpp +++ b/extension/httpfs/include/httpfs_client.hpp @@ -9,7 +9,8 @@ struct FileOpenerInfo; class HTTPState; struct HTTPFSParams : public HTTPParams { - HTTPFSParams(HTTPUtil &http_util) : HTTPParams(http_util) {} + HTTPFSParams(HTTPUtil &http_util) : HTTPParams(http_util) { + } static constexpr bool DEFAULT_ENABLE_SERVER_CERT_VERIFICATION = false; static constexpr uint64_t DEFAULT_HF_MAX_PER_PAGE = 0; @@ -25,7 +26,8 @@ struct HTTPFSParams : public HTTPParams { class HTTPFSUtil : public HTTPUtil { public: - unique_ptr InitializeParameters(optional_ptr opener, optional_ptr info) override; + unique_ptr InitializeParameters(optional_ptr opener, + optional_ptr info) override; unique_ptr InitializeClient(HTTPParams &http_params, const string &proto_host_port) override; static unordered_map ParseGetParameters(const string &text); diff --git a/extension/httpfs/s3fs.cpp b/extension/httpfs/s3fs.cpp index ce0710fc..3e5c5c29 100644 --- a/extension/httpfs/s3fs.cpp +++ b/extension/httpfs/s3fs.cpp @@ -4,6 +4,8 @@ #include "duckdb.hpp" #ifndef DUCKDB_AMALGAMATION #include "duckdb/common/exception/http_exception.hpp" +#include "duckdb/logging/log_type.hpp" +#include "duckdb/logging/file_system_logger.hpp" #include "duckdb/common/helper.hpp" #include "duckdb/common/thread.hpp" #include "duckdb/common/types/timestamp.hpp" @@ -838,6 +840,8 @@ void S3FileSystem::Write(FileHandle &handle, void *buffer, int64_t nr_bytes, idx s3fh.file_offset += bytes_to_write; bytes_written += bytes_to_write; } + + DUCKDB_LOG_FILE_SYSTEM_WRITE(handle, bytes_written, s3fh.file_offset - bytes_written); } static bool Match(vector::const_iterator key, vector::const_iterator key_end, @@ -918,8 +922,8 @@ vector S3FileSystem::Glob(const string &glob_pattern, FileOpener * string common_prefix_continuation_token; do { auto prefix_res = - AWSListObjectV2::Request(prefix_path, *http_params, s3_auth_params, common_prefix_continuation_token, - HTTPState::TryGetState(opener).get()); + AWSListObjectV2::Request(prefix_path, *http_params, s3_auth_params, + common_prefix_continuation_token, HTTPState::TryGetState(opener).get()); AWSListObjectV2::ParseFileList(prefix_res, s3_keys); auto more_prefixes = AWSListObjectV2::ParseCommonPrefix(prefix_res); common_prefixes.insert(common_prefixes.end(), more_prefixes.begin(), more_prefixes.end()); diff --git a/test/sql/secret/secret_refresh.test b/test/sql/secret/secret_refresh.test index e1cc7ed4..85c8738a 100644 --- a/test/sql/secret/secret_refresh.test +++ b/test/sql/secret/secret_refresh.test @@ -61,7 +61,7 @@ statement ok FROM "s3://test-bucket/test-file.parquet" query I -SELECT message[0:46] FROM duckdb_logs WHERE type='httpfs.SecretRefresh' +SELECT message[0:46] FROM duckdb_logs WHERE message like '%Successfully refreshed secret%' ---- Successfully refreshed secret: s1, new key_id: @@ -84,7 +84,7 @@ FROM "s3://test-bucket/test-file.parquet" HTTP 403 query I -SELECT message[0:46] FROM duckdb_logs WHERE type='httpfs.SecretRefresh' +SELECT message[0:46] FROM duckdb_logs WHERE message like '%Successfully refreshed secret%' ---- Successfully refreshed secret: s1, new key_id: @@ -125,5 +125,5 @@ HTTP 403 # -> log empty query II -SELECT log_level, message FROM duckdb_logs WHERE type='httpfs.SecretRefresh' +SELECT log_level, message FROM duckdb_logs WHERE message like '%Successfully refreshed secret%' ---- diff --git a/test/sql/secret/secret_refresh_attach.test b/test/sql/secret/secret_refresh_attach.test index a62ed157..c20881d9 100644 --- a/test/sql/secret/secret_refresh_attach.test +++ b/test/sql/secret/secret_refresh_attach.test @@ -45,6 +45,6 @@ ATTACH 's3://test-bucket/presigned/attach.db' AS db (READONLY 1); # Secret refresh has been triggered query II -SELECT log_level, message FROM duckdb_logs WHERE type='httpfs.SecretRefresh' +SELECT log_level, message FROM duckdb_logs WHERE message like '%Successfully refreshed secret%' ---- INFO Successfully refreshed secret: uhuh_this_mah_sh, new key_id: all the girls \ No newline at end of file