From 2f2b1791dec3fa7a5c9838a4f1d99a859ddedf5e Mon Sep 17 00:00:00 2001 From: Mikhail Koviazin Date: Sat, 3 Oct 2026 13:28:53 +0200 Subject: [PATCH 1/2] Separate Antalya error codes from upstream allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move Antalya-only errors to a separate append-only registry with wire codes 10001–10006. Preserve symbolic names and the existing exception packet layout; do not introduce legacy numeric aliases. Store vendor counters compactly and include them in `system.errors`, `system.error_log`, and Prometheus metrics. Skip unnamed accounting slots in the error log to avoid attributing unknown errors to the sentinel. Validate vendor IDs, names, upstream conflicts, and `UInt16` bounds. Preserve the upstream `values` API and name-table construction to reduce forward-porting overhead. Document the numeric migration and add lookup, accounting, log-column, and distributed-forwarding regression tests. --- ci/jobs/scripts/check_style/check_cpp.sh | 4 + docs/en/antalya/error_codes.md | 72 ++++++++ src/Common/AntalyaErrorCodes.h | 17 ++ src/Common/ErrorCodes.cpp | 147 +++++++++++----- src/Common/ErrorCodes.h | 14 +- src/Common/tests/gtest_error_codes.cpp | 157 ++++++++++++++++++ src/Interpreters/ErrorLog.cpp | 20 ++- src/Interpreters/ErrorLog.h | 2 +- src/Server/PrometheusMetricsWriter.cpp | 7 +- src/Storages/System/StorageSystemErrors.cpp | 11 +- .../test_antalya_error_codes/__init__.py | 0 .../configs/errors.xml | 16 ++ .../test_antalya_error_codes/test.py | 143 ++++++++++++++++ .../05055_antalya_error_codes.reference | 13 ++ .../0_stateless/05055_antalya_error_codes.sql | 13 ++ 15 files changed, 579 insertions(+), 57 deletions(-) create mode 100644 docs/en/antalya/error_codes.md create mode 100644 src/Common/AntalyaErrorCodes.h create mode 100644 src/Common/tests/gtest_error_codes.cpp create mode 100644 tests/integration/test_antalya_error_codes/__init__.py create mode 100644 tests/integration/test_antalya_error_codes/configs/errors.xml create mode 100644 tests/integration/test_antalya_error_codes/test.py create mode 100644 tests/queries/0_stateless/05055_antalya_error_codes.reference create mode 100644 tests/queries/0_stateless/05055_antalya_error_codes.sql diff --git a/ci/jobs/scripts/check_style/check_cpp.sh b/ci/jobs/scripts/check_style/check_cpp.sh index 121e5340afe5..b23391fe5a18 100755 --- a/ci/jobs/scripts/check_style/check_cpp.sh +++ b/ci/jobs/scripts/check_style/check_cpp.sh @@ -126,6 +126,10 @@ EXTERN_TYPES_EXCLUDES=( ErrorCodes::values ErrorCodes::values[i] ErrorCodes::getErrorCodeByName + ErrorCodes::size + ErrorCodes::getCode + ErrorCodes::getValue + ErrorCodes::ANTALYA_ERROR_CODE_BASE ErrorCodes::Value ) # Check unused/undefined/duplicate ErrorCodes, ProfileEvents, CurrentMetrics declarations. diff --git a/docs/en/antalya/error_codes.md b/docs/en/antalya/error_codes.md new file mode 100644 index 000000000000..21251f340b34 --- /dev/null +++ b/docs/en/antalya/error_codes.md @@ -0,0 +1,72 @@ +--- +description: 'Stable numeric identities for Antalya-specific errors and migration from legacy codes.' +sidebar_label: 'Error codes' +sidebar_position: 20 +slug: /antalya/error-codes +title: 'Antalya error codes' +doc_type: 'reference' +--- + +# Antalya error codes {#antalya-error-codes} + +Antalya-specific errors use a separate registry. Their numeric code is +`10000 + local_id`, where local IDs are positive, append-only, and never reused. +Upstream errors retain their existing numbers. Symbolic names remain in +`DB::ErrorCodes`; applications should prefer symbolic names over numeric codes +when possible. + +The high range is an Antalya convention, not a reservation recognized by +upstream ClickHouse. Build-time checks reject duplicate Antalya IDs or names, +overlapping numeric ranges, and names shared with upstream errors. Wire codes +must fit in `UInt16` because +`system.part_log.error` and `system.background_schedule_pool_log.error` use that +type. Local IDs therefore cannot exceed `55535`; this limit is enforced at build +time. Adding a new error requires a new local ID in `src/Common/AntalyaErrorCodes.h`. + +## Migration from legacy numbers {#migration-from-legacy-numbers} + +| Name | Legacy code | New code | +|---|---:|---:| +| `CATALOG_NAMESPACE_DISABLED` | 779 | 10001 | +| `PENDING_MUTATIONS_NOT_ALLOWED` | 1009 | 10002 | +| `EXPORT_PARTITION_ALREADY_EXPORTED` | 1010 | 10003 | +| `PARTITION_EXPORT_FAILED` | 1011 | 10004 | +| `CAS_WRITE_UNATTRIBUTED` | 1037 | 10005 | +| `CAS_DELETE_MARKER` | 1038 | 10006 | + +This is a numeric compatibility break. Update clients, monitoring rules, and +scripts that compare the legacy numbers. There are no legacy numeric aliases: +some legacy values identify different errors in upstream ClickHouse. Error +messages and symbolic names are unchanged. + +## Client and mixed-version behavior {#client-and-mixed-version-behavior} + +Exception packets retain their existing layout and signed 32-bit code field. +No protocol revision or peer negotiation is required. Antalya clients built +with this registry display the symbolic name. Older or upstream clients retain +the numeric code and message but may display an empty symbolic name. + +Distributed-query forwarding preserves the numeric identity, including codes +unknown to the intermediate server. However, preserving the code does not make +older Antalya servers recognize it in code-specific handling, such as partition +export conflict handling. Mixed-version feature behavior is not guaranteed; +upgrade participating servers together when relying on Antalya-specific error +handling. New servers likewise do not reinterpret legacy numbers as new errors. + +A shell exit status is only eight bits. Do not use `$?` to recover the full +numeric error code; inspect the client's error output instead. Query failure +continues to produce a nonzero exit status. + +## Accounting {#accounting} + +`system.errors`, `system.error_log`, and Prometheus error metrics include +registered Antalya errors. Counter storage grows with the number of registered +errors, not with the largest numeric code. Local and remote counters remain +separate. Unknown codes retain their identity in exceptions. Codes outside the +upstream array, unless registered by Antalya, share an unnamed out-of-range +accounting slot. Unassigned slots inside the upstream array keep their existing +accounting behavior. Unknown codes do not acquire a registered name or increment +an Antalya error's counters. The aggregated error log skips unnamed slots, as do +`system.errors` and Prometheus, rather than reporting the shared slot number as +an exception identity. `system.query_log.exception_code` retains the original +numeric code. diff --git a/src/Common/AntalyaErrorCodes.h b/src/Common/AntalyaErrorCodes.h new file mode 100644 index 000000000000..c879bf0fb7b6 --- /dev/null +++ b/src/Common/AntalyaErrorCodes.h @@ -0,0 +1,17 @@ +#pragma once + +/// Local IDs are append-only, never reused, and independent of upstream assignments. +/// Keep this registry ordered by local ID. Wire codes are `10000 + local_id`. +/// Codes must fit in `UInt16` to preserve their identity in part and background-pool logs. +#define APPLY_FOR_ANTALYA_ERROR_CODES(M) \ + M(1, CATALOG_NAMESPACE_DISABLED) \ + M(2, PENDING_MUTATIONS_NOT_ALLOWED) \ + M(3, EXPORT_PARTITION_ALREADY_EXPORTED) \ + M(4, PARTITION_EXPORT_FAILED) \ + M(5, CAS_WRITE_UNATTRIBUTED) \ + M(6, CAS_DELETE_MARKER) + +namespace DB::ErrorCodes +{ +inline constexpr int ANTALYA_ERROR_CODE_BASE = 10000; +} diff --git a/src/Common/ErrorCodes.cpp b/src/Common/ErrorCodes.cpp index cfdc94b31795..5269246461f8 100644 --- a/src/Common/ErrorCodes.cpp +++ b/src/Common/ErrorCodes.cpp @@ -1,7 +1,11 @@ #include +#include #include #include #include +#include +#include +#include /** Previously, these constants were located in one enum. * But in this case there is a problem: when you add a new constant, you need to recompile @@ -658,7 +662,6 @@ M(776, RESOURCE_LIMIT_EXCEEDED) \ M(777, MEMORY_RESERVATION_KILLED) \ M(778, MEMORY_RESERVATION_FAILED) \ - M(779, CATALOG_NAMESPACE_DISABLED) \ \ M(900, DISTRIBUTED_CACHE_ERROR) \ M(901, CANNOT_USE_DISTRIBUTED_CACHE) \ @@ -676,19 +679,6 @@ M(1006, INVALID_CURSOR_LOOKUP) \ M(1007, ILLEGAL_STREAM) \ M(1008, TEMPORARY_DATA_NOT_IN_CACHE) \ - M(1009, PENDING_MUTATIONS_NOT_ALLOWED) \ - /* 1010 and 1011 predate the fork's error-code range policy stated below, and are kept as-is \ - * rather than renumbered: they currently collide with upstream ClickHouse's own 1010 \ - * (UNIQUE_KEY_DENSE_INDEX_UNREADABLE) and 1011 (HANDLER_ALREADY_EXISTS). */ \ - M(1010, EXPORT_PARTITION_ALREADY_EXPORTED) \ - M(1011, PARTITION_EXPORT_FAILED) \ - /* 1012 and 1013 are intentionally skipped: they collide with upstream ClickHouse's \ - * HANDLER_DOESNT_EXIST and AMBIGUOUS_HANDLER. Fork-specific error codes live in the 1030-1099 \ - * range, chosen to sit well above upstream's maximum error code (1017 at the time this range \ - * was reserved) so upstream can keep adding codes below it without colliding with the fork's. \ - * A new fork error code goes in this range, not below 1030. CAS codes occupy 1037-1038. */ \ - M(1037, CAS_WRITE_UNATTRIBUTED) \ - M(1038, CAS_DELETE_MARKER) \ /* See END */ #ifdef APPLY_FOR_EXTERNAL_ERROR_CODES @@ -705,7 +695,21 @@ namespace ErrorCodes APPLY_FOR_ERROR_CODES(M) #undef M - constexpr ErrorCode END = 1038; +#define M(ID, NAME) extern const ErrorCode NAME = ANTALYA_ERROR_CODE_BASE + ID; + APPLY_FOR_ANTALYA_ERROR_CODES(M) +#undef M + + constexpr ErrorCode getUpstreamEnd() + { + ErrorCode maximum = 0; +#define M(VALUE, NAME) maximum = std::max(maximum, ErrorCode{VALUE}); + APPLY_FOR_ERROR_CODES(M) +#undef M + return maximum + 1; + } + + constexpr ErrorCode END = getUpstreamEnd(); + static_assert(END < ANTALYA_ERROR_CODE_BASE, "Upstream range overlaps the Antalya range"); ErrorPairHolder values[END + 1]{}; struct ErrorCodesNames @@ -719,52 +723,121 @@ namespace ErrorCodes } } static error_codes_names; + namespace + { + struct Entry + { + ErrorCode code; + std::string_view name; + }; + + constexpr auto antalya_entries = std::to_array({ +#define M(ID, NAME) Entry{ANTALYA_ERROR_CODE_BASE + ID, #NAME}, + APPLY_FOR_ANTALYA_ERROR_CODES(M) +#undef M + }); + + constexpr bool validateAntalyaIds() + { + Int64 previous_id = 0; +#define M(ID, NAME) \ + if (Int64{ID} <= previous_id || Int64{ID} > std::numeric_limits::max() - Int64{ANTALYA_ERROR_CODE_BASE}) \ + return false; \ + previous_id = ID; + APPLY_FOR_ANTALYA_ERROR_CODES(M) +#undef M + return true; + } + static_assert(validateAntalyaIds(), "Antalya IDs must be positive, ordered, unique, and fit UInt16 log columns"); + + constexpr bool validateAntalyaNames() + { + for (size_t index = 0; index < antalya_entries.size(); ++index) + { + const auto name = antalya_entries[index].name; + for (size_t previous = 0; previous < index; ++previous) + { + if (name == antalya_entries[previous].name) + return false; + } +#define M(VALUE, NAME) if (name == std::string_view(#NAME)) return false; + APPLY_FOR_ERROR_CODES(M) +#undef M + } + return true; + } + static_assert(validateAntalyaNames(), "Antalya error names must be unique and distinct from upstream names"); + + std::array antalya_values; + + size_t getIndex(ErrorCode code) + { + if (code >= 0 && code <= END) + return code; + for (size_t index = 0; index < antalya_entries.size(); ++index) + { + if (antalya_entries[index].code == code) + return END + 1 + index; + } + /// Preserve out-of-range accounting without attributing it to a registered error. + return END; + } + } + std::string_view getName(ErrorCode error_code) { - if (error_code < 0 || error_code > END) - return std::string_view(); - return error_codes_names.names[error_code]; + if (error_code >= 0 && error_code <= END) + return error_codes_names.names[error_code]; + for (const auto & entry : antalya_entries) + { + if (entry.code == error_code) + return entry.name; + } + return {}; } ErrorCode getErrorCodeByName(std::string_view error_name) { - for (int i = 0, end = ErrorCodes::end(); i < end; ++i) + for (size_t index = 0; index < size(); ++index) { - std::string_view name = ErrorCodes::getName(i); + const auto code = getCode(index); + std::string_view name = getName(code); if (name.empty()) continue; if (name == error_name) - return i; + return code; } throw Exception(NO_SUCH_ERROR_CODE, "No error code with name: '{}'", error_name); } ErrorCode end() { return END + 1; } - size_t increment(ErrorCode error_code, bool remote, const std::string & message, const std::string & format_string, const FramePointers & trace) + size_t size() { return end() + antalya_entries.size(); } + + ErrorCode getCode(size_t index) { - if (error_code < 0 || error_code >= end()) - { - /// For everything outside the range, use END. - /// (end() is the pointer pass the end, while END is the last value that has an element in values array). - error_code = end() - 1; - } + if (index < static_cast(end())) + return static_cast(index); + return antalya_entries.at(index - end()).code; + } - return values[error_code].increment(remote, message, format_string, trace); + ErrorPairHolder & getValue(size_t index) + { + if (index < static_cast(end())) + return values[index]; + return antalya_values.at(index - end()); } - void extendedMessage(ErrorCode error_code, bool remote, size_t error_index, const std::string & message) + size_t increment(ErrorCode error_code, bool remote, const std::string & message, const std::string & format_string, const FramePointers & trace) { - if (error_code < 0 || error_code >= end()) - { - /// For everything outside the range, use END. - /// (end() is the pointer pass the end, while END is the last value that has an element in values array). - error_code = end() - 1; - } + return getValue(getIndex(error_code)).increment(remote, message, format_string, trace); + } - values[error_code].extendedMessage(remote, error_index, message); + void extendedMessage(ErrorCode error_code, bool remote, size_t error_index, const std::string & message) + { + getValue(getIndex(error_code)).extendedMessage(remote, error_index, message); } size_t ErrorPairHolder::increment(bool remote, const std::string & message, const std::string & format_string, const FramePointers & trace) diff --git a/src/Common/ErrorCodes.h b/src/Common/ErrorCodes.h index fe75b8d72c66..3c1a3c7a7697 100644 --- a/src/Common/ErrorCodes.h +++ b/src/Common/ErrorCodes.h @@ -16,7 +16,7 @@ namespace DB namespace ErrorCodes { - /// ErrorCode identifier (index in array). + /// Numeric error identity, including the code transmitted to peers; not a storage index. using ErrorCode = int; using Value = size_t; using FramePointers = std::vector; @@ -65,12 +65,20 @@ namespace ErrorCodes std::mutex mutex; }; - /// ErrorCode identifier -> current value of error_code. + /// Upstream numeric code -> counters. Antalya codes must not index this array. extern ErrorPairHolder values[]; - /// Get index just after last error_code identifier. + /// Get the upstream array boundary, including the unnamed out-of-range accounting slot. ErrorCode end(); + /// Number of counter slots, including upstream holes and the unnamed accounting sentinel. + /// Antalya entries occupy compact slots after the upstream array. + size_t size(); + /// Translate a storage index into the actual error code. The index must be less than `size`. + ErrorCode getCode(size_t index); + /// Access counters by storage index, not by numeric error identity. + ErrorPairHolder & getValue(size_t index); + /// Increments the counter of errors for a specified error code, and remembers some information about the last error. /// The function returns the index of the passed error among other errors with the same code and the same `remote` flag /// since the program startup. diff --git a/src/Common/tests/gtest_error_codes.cpp b/src/Common/tests/gtest_error_codes.cpp new file mode 100644 index 000000000000..05b864dd0c4b --- /dev/null +++ b/src/Common/tests/gtest_error_codes.cpp @@ -0,0 +1,157 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace DB::ErrorCodes +{ +extern const int CATALOG_NAMESPACE_DISABLED; +extern const int CAS_DELETE_MARKER; +extern const int BAD_ARGUMENTS; +} + +namespace +{ +struct ExpectedError +{ + int code; + std::string_view name; +}; + +constexpr auto expected_antalya_errors = std::to_array({ +#define M(ID, NAME) ExpectedError{DB::ErrorCodes::ANTALYA_ERROR_CODE_BASE + ID, #NAME}, + APPLY_FOR_ANTALYA_ERROR_CODES(M) +#undef M +}); +} + +TEST(ErrorCodes, AntalyaIdentity) +{ + EXPECT_EQ(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED, 10001); + EXPECT_EQ(DB::ErrorCodes::CAS_DELETE_MARKER, 10006); + for (const auto & entry : expected_antalya_errors) + { + EXPECT_EQ(DB::ErrorCodes::getName(entry.code), entry.name); + EXPECT_EQ(DB::ErrorCodes::getErrorCodeByName(entry.name), entry.code); + } + EXPECT_EQ(DB::ErrorCodes::getErrorCodeByName("BAD_ARGUMENTS"), DB::ErrorCodes::BAD_ARGUMENTS); + for (int code : {-1, 10000, 10007, std::numeric_limits::max()}) + EXPECT_TRUE(DB::ErrorCodes::getName(code).empty()); + EXPECT_THROW(DB::ErrorCodes::getErrorCodeByName("NOT_A_REGISTERED_ERROR"), DB::Exception); +} + +TEST(ErrorCodes, ExceptionWireIdentity) +{ + for (int code : {10001, 10006, 10007, -1, std::numeric_limits::max()}) + { + auto original = DB::Exception::createDeprecated("wire identity", code); + DB::WriteBufferFromOwnString output; + DB::writeException(original, output, false); + DB::ReadBufferFromString input(output.str()); + auto decoded = DB::readException(input, "", true); + EXPECT_EQ(decoded.code(), code); + EXPECT_NE(decoded.message().find("wire identity"), std::string::npos); + } +} + +TEST(ErrorCodes, CompactEnumerationAndCounters) +{ + ASSERT_EQ(DB::ErrorCodes::size(), DB::ErrorCodes::end() + expected_antalya_errors.size()); + EXPECT_LT(DB::ErrorCodes::size(), DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED); + EXPECT_TRUE(DB::ErrorCodes::getName(DB::ErrorCodes::end() - 1).empty()); + EXPECT_THROW(DB::ErrorCodes::getCode(DB::ErrorCodes::size()), std::out_of_range); + EXPECT_THROW(DB::ErrorCodes::getValue(DB::ErrorCodes::size()), std::out_of_range); + + for (size_t slot = 0; slot < DB::ErrorCodes::size(); ++slot) + { + const auto code = DB::ErrorCodes::getCode(slot); + const auto name = DB::ErrorCodes::getName(code); + if (!name.empty()) + EXPECT_EQ(DB::ErrorCodes::getErrorCodeByName(name), code); + if (slot < static_cast(DB::ErrorCodes::end())) + EXPECT_EQ(code, slot); + else + { + const auto & expected = expected_antalya_errors.at(slot - DB::ErrorCodes::end()); + EXPECT_EQ(code, expected.code); + EXPECT_EQ(name, expected.name); + } + + if (name.empty()) + continue; + for (bool remote : {false, true}) + { + const auto before = DB::ErrorCodes::getValue(slot).get(); + const auto index = DB::ErrorCodes::increment(code, remote, "slot test", "slot test", {}); + DB::ErrorCodes::extendedMessage(code, remote, index, "slot test extended"); + const auto after = DB::ErrorCodes::getValue(slot).get(); + EXPECT_EQ(after.local.count, before.local.count + !remote); + EXPECT_EQ(after.remote.count, before.remote.count + remote); + EXPECT_EQ((remote ? after.remote : after.local).message, "slot test extended"); + } + } +} + +TEST(ErrorCodes, UnknownAccountingDoesNotPolluteAntalya) +{ + const auto vendor_slot = DB::ErrorCodes::size() - 1; + const auto before = DB::ErrorCodes::getValue(vendor_slot).get(); + const auto sentinel = DB::ErrorCodes::end() - 1; + for (int code : {-1, DB::ErrorCodes::end(), 10000, 10007, std::numeric_limits::max()}) + { + const auto count = DB::ErrorCodes::getValue(sentinel).get().local.count; + const auto index = DB::ErrorCodes::increment(code, false, "unknown", "unknown", {}); + DB::ErrorCodes::extendedMessage(code, false, index, "unknown extended"); + const auto value = DB::ErrorCodes::getValue(sentinel).get().local; + EXPECT_EQ(value.count, count + 1); + EXPECT_EQ(value.message, "unknown extended"); + } + for (int code : {779, DB::ErrorCodes::end() - 2}) + { + ASSERT_GE(code, 0); + ASSERT_LT(code, DB::ErrorCodes::end()); + const auto ordinary_before = DB::ErrorCodes::getValue(code).get().local.count; + DB::ErrorCodes::increment(code, false, "ordinary", "ordinary", {}); + EXPECT_EQ(DB::ErrorCodes::getValue(code).get().local.count, ordinary_before + 1); + } + + const auto after = DB::ErrorCodes::getValue(vendor_slot).get(); + EXPECT_EQ(before.local.count, after.local.count); + EXPECT_EQ(before.remote.count, after.remote.count); +} + +TEST(ErrorCodes, AntalyaCodesSurviveNarrowLogColumns) +{ + auto check_log = [](auto element, int code) + { + element.error = static_cast(code); + DB::MutableColumns columns; + size_t error_column = 0; + for (const auto & column : element.getColumnsDescription().getAllPhysical()) + { + if (column.name == "error") + error_column = columns.size(); + columns.push_back(column.type->createColumn()); + } + element.appendToBlock(columns); + EXPECT_EQ(columns[error_column]->getUInt(0), code); + EXPECT_EQ(DB::ErrorCodes::getName(static_cast(columns[error_column]->getUInt(0))), DB::ErrorCodes::getName(code)); + }; + for (size_t slot = DB::ErrorCodes::end(); slot < DB::ErrorCodes::size(); ++slot) + { + const auto code = DB::ErrorCodes::getCode(slot); + check_log(DB::PartLogElement{}, code); + check_log(DB::BackgroundSchedulePoolLogElement{}, code); + } +} diff --git a/src/Interpreters/ErrorLog.cpp b/src/Interpreters/ErrorLog.cpp index b89ad69fcc71..0b572c97d9e4 100644 --- a/src/Interpreters/ErrorLog.cpp +++ b/src/Interpreters/ErrorLog.cpp @@ -127,15 +127,19 @@ void ErrorLog::stepFunction(TimePoint current_time) auto event_time = std::chrono::system_clock::to_time_t(current_time); - for (ErrorCodes::ErrorCode code = 0, end = ErrorCodes::end(); code < end; ++code) + for (size_t index = 0, size = ErrorCodes::size(); index < size; ++index) { - const auto & error = ErrorCodes::values[code].get(); - if (error.local.count != previous_values.at(code).local) + const auto code = ErrorCodes::getCode(index); + if (ErrorCodes::getName(code).empty()) + continue; + const auto error = ErrorCodes::getValue(index).get(); + auto & previous = previous_values.at(index); + if (error.local.count != previous.local) { ErrorLogElement local_elem { .event_time=event_time, .code=code, - .value=error.local.count - previous_values.at(code).local, + .value=error.local.count - previous.local, .remote=false, .last_error_time=(error.local.error_time_ms / 1000), .last_error_message=error.local.message, @@ -143,14 +147,14 @@ void ErrorLog::stepFunction(TimePoint current_time) .last_error_trace=error.local.trace }; this->add(std::move(local_elem)); - previous_values[code].local = error.local.count; + previous.local = error.local.count; } - if (error.remote.count != previous_values.at(code).remote) + if (error.remote.count != previous.remote) { ErrorLogElement remote_elem { .event_time=event_time, .code=code, - .value=error.remote.count - previous_values.at(code).remote, + .value=error.remote.count - previous.remote, .remote=true, .last_error_time=(error.remote.error_time_ms / 1000), .last_error_message=error.remote.message, @@ -158,7 +162,7 @@ void ErrorLog::stepFunction(TimePoint current_time) .last_error_trace=error.remote.trace }; add(std::move(remote_elem)); - previous_values[code].remote = error.remote.count; + previous.remote = error.remote.count; } } } diff --git a/src/Interpreters/ErrorLog.h b/src/Interpreters/ErrorLog.h index 4c317a376313..4fcacf668d9e 100644 --- a/src/Interpreters/ErrorLog.h +++ b/src/Interpreters/ErrorLog.h @@ -43,7 +43,7 @@ class ErrorLog : public PeriodicLog UInt64 remote = 0; }; /// stepFunction and flushBufferToLog may be executed concurrently, hence the mutex - std::vector previous_values TSA_GUARDED_BY(previous_values_mutex) = std::vector(ErrorCodes::end()); + std::vector previous_values TSA_GUARDED_BY(previous_values_mutex) = std::vector(ErrorCodes::size()); mutable std::mutex previous_values_mutex; }; diff --git a/src/Server/PrometheusMetricsWriter.cpp b/src/Server/PrometheusMetricsWriter.cpp index 8ce7036ba018..af30adf0001a 100644 --- a/src/Server/PrometheusMetricsWriter.cpp +++ b/src/Server/PrometheusMetricsWriter.cpp @@ -162,14 +162,15 @@ void PrometheusMetricsWriter::writeErrors(WriteBuffer & wb) const { size_t total_count = 0; - for (size_t i = 0, end = ErrorCodes::end(); i < end; ++i) + for (size_t i = 0, size = ErrorCodes::size(); i < size; ++i) { - const auto & error = ErrorCodes::values[i].get(); - std::string_view name = ErrorCodes::getName(static_cast(i)); + std::string_view name = ErrorCodes::getName(ErrorCodes::getCode(i)); if (name.empty()) continue; + const auto error = ErrorCodes::getValue(i).get(); + std::string key{error_metrics_prefix + toString(name)}; std::string help = fmt::format("The number of {} errors since last server restart", name); diff --git a/src/Storages/System/StorageSystemErrors.cpp b/src/Storages/System/StorageSystemErrors.cpp index e02340239ee2..90b2a9056c29 100644 --- a/src/Storages/System/StorageSystemErrors.cpp +++ b/src/Storages/System/StorageSystemErrors.cpp @@ -58,16 +58,17 @@ void StorageSystemErrors::fillData(MutableColumns & res_columns, ContextPtr cont } }; - for (size_t i = 0, end = ErrorCodes::end(); i < end; ++i) + for (size_t i = 0, size = ErrorCodes::size(); i < size; ++i) { - const auto & error = ErrorCodes::values[i].get(); - std::string_view name = ErrorCodes::getName(static_cast(i)); + const auto code = ErrorCodes::getCode(i); + std::string_view name = ErrorCodes::getName(code); if (name.empty()) continue; - add_row(name, i, error.local, /* remote= */ false); - add_row(name, i, error.remote, /* remote= */ true); + const auto error = ErrorCodes::getValue(i).get(); + add_row(name, code, error.local, /* remote= */ false); + add_row(name, code, error.remote, /* remote= */ true); } } diff --git a/tests/integration/test_antalya_error_codes/__init__.py b/tests/integration/test_antalya_error_codes/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/integration/test_antalya_error_codes/configs/errors.xml b/tests/integration/test_antalya_error_codes/configs/errors.xml new file mode 100644 index 000000000000..b1cffa725e14 --- /dev/null +++ b/tests/integration/test_antalya_error_codes/configs/errors.xml @@ -0,0 +1,16 @@ + + + system + error_log
+ 100 + 100 + + + /metrics + 8001 + false + false + false + true + + diff --git a/tests/integration/test_antalya_error_codes/test.py b/tests/integration/test_antalya_error_codes/test.py new file mode 100644 index 000000000000..7dadfc34d2d0 --- /dev/null +++ b/tests/integration/test_antalya_error_codes/test.py @@ -0,0 +1,143 @@ +import re + +import pytest +import requests + +from helpers.cluster import ClickHouseCluster + + +cluster = ClickHouseCluster(__file__) +origin = cluster.add_instance("origin", main_configs=["configs/errors.xml"]) +relay = cluster.add_instance("relay", main_configs=["configs/errors.xml"]) +edge = cluster.add_instance("edge", main_configs=["configs/errors.xml"]) + +ERRORS = [ + (10001, "CATALOG_NAMESPACE_DISABLED"), + (10002, "PENDING_MUTATIONS_NOT_ALLOWED"), + (10003, "EXPORT_PARTITION_ALREADY_EXPORTED"), + (10004, "PARTITION_EXPORT_FAILED"), + (10005, "CAS_WRITE_UNATTRIBUTED"), + (10006, "CAS_DELETE_MARKER"), +] +SETTINGS = {"allow_custom_error_code_in_throwif": 1} + + +@pytest.fixture(scope="module", autouse=True) +def started_cluster(): + try: + cluster.start() + yield + finally: + cluster.shutdown() + + +@pytest.mark.parametrize("code", [-1, 779, 1009, 10000, 10007, 2147483647]) +def test_unknown_code_is_not_logged_as_sentinel(code): + message = f"unknown code {code} must not become sentinel" + error = origin.query_and_get_error( + f"SELECT throwIf(1, '{message}', toInt32({code}))", settings=SETTINGS + ) + assert f"Code: {code}." in error + origin.query("SYSTEM FLUSH LOGS") + assert ( + origin.query( + "SELECT count() FROM system.error_log " + f"WHERE position(last_error_message, '{message}') > 0" + ) + == "0\n" + ) + assert ( + origin.query( + "SELECT count() FROM system.query_log WHERE type IN " + "('ExceptionBeforeStart', 'ExceptionWhileProcessing') " + f"AND exception_code = {code} AND position(exception, '{message}') > 0" + ) + == "1\n" + ) + + +@pytest.mark.parametrize("code,name", ERRORS) +def test_local_observability(code, name): + message = f"Antalya error {code}" + error = origin.query_and_get_error( + f"SELECT throwIf(1, '{message}', toInt32({code}))", settings=SETTINGS + ) + assert f"Code: {code}." in error + assert name in error + assert message in error + assert ( + origin.query( + f"SELECT count() FROM system.errors WHERE code = {code} " + f"AND name = '{name}' AND remote = 0 AND value > 0 " + f"AND position(last_error_message, '{message}') > 0" + ) + == "1\n" + ) + + origin.query("SYSTEM FLUSH LOGS") + assert ( + int( + origin.query( + f"SELECT count() FROM system.error_log WHERE code = {code} " + f"AND remote = 0 AND value > 0 AND position(last_error_message, '{message}') > 0" + ) + ) + > 0 + ) + + response = requests.get(f"http://{origin.ip_address}:8001/metrics", timeout=10) + response.raise_for_status() + metric = re.search(rf"^ClickHouseErrorMetric_{name} (\d+)$", response.text, re.MULTILINE) + assert metric is not None + assert int(metric.group(1)) > 0 + + +@pytest.mark.parametrize("code,name", [ERRORS[0], ERRORS[-1], (10007, "")]) +def test_distributed_rethrow(code, name): + table = f"vendor_error_{code}" + message = f"Antalya distributed error {code}" + try: + origin.query( + f"CREATE VIEW {table} AS SELECT throwIf(1, '{message}', toInt32({code})) AS value", + settings=SETTINGS, + ) + relay.query( + f"CREATE VIEW {table} AS SELECT * FROM remote('origin', default, {table})" + ) + error = edge.query_and_get_error( + f"SELECT * FROM remote('relay', default, {table})", settings=SETTINGS + ) + assert f"Code: {code}." in error + assert message in error + if name: + assert name in error + for node in (relay, edge): + assert ( + int( + node.query( + f"SELECT sum(value) FROM system.errors WHERE code = {code} " + f"AND name = '{name}' AND remote = 1" + ) + ) + > 0 + ) + node.query("SYSTEM FLUSH LOGS") + assert ( + int( + node.query( + f"SELECT count() FROM system.error_log WHERE code = {code} AND remote = 1" + ) + ) + > 0 + ) + else: + assert ( + edge.query( + f"SELECT count() FROM system.errors WHERE code = {code} " + "SETTINGS system_events_show_zero_values = 1" + ) + == "0\n" + ) + finally: + relay.query(f"DROP VIEW IF EXISTS {table}") + origin.query(f"DROP VIEW IF EXISTS {table}") diff --git a/tests/queries/0_stateless/05055_antalya_error_codes.reference b/tests/queries/0_stateless/05055_antalya_error_codes.reference new file mode 100644 index 000000000000..fe581e47b293 --- /dev/null +++ b/tests/queries/0_stateless/05055_antalya_error_codes.reference @@ -0,0 +1,13 @@ +CATALOG_NAMESPACE_DISABLED 10001 +PENDING_MUTATIONS_NOT_ALLOWED 10002 +EXPORT_PARTITION_ALREADY_EXPORTED 10003 +PARTITION_EXPORT_FAILED 10004 +CAS_WRITE_UNATTRIBUTED 10005 +CAS_DELETE_MARKER 10006 +CATALOG_NAMESPACE_DISABLED +PENDING_MUTATIONS_NOT_ALLOWED +EXPORT_PARTITION_ALREADY_EXPORTED +PARTITION_EXPORT_FAILED +CAS_WRITE_UNATTRIBUTED +CAS_DELETE_MARKER + diff --git a/tests/queries/0_stateless/05055_antalya_error_codes.sql b/tests/queries/0_stateless/05055_antalya_error_codes.sql new file mode 100644 index 000000000000..7fb6f06e1080 --- /dev/null +++ b/tests/queries/0_stateless/05055_antalya_error_codes.sql @@ -0,0 +1,13 @@ +SELECT name, code FROM system.errors +WHERE name IN ('CATALOG_NAMESPACE_DISABLED', 'PENDING_MUTATIONS_NOT_ALLOWED', + 'EXPORT_PARTITION_ALREADY_EXPORTED', 'PARTITION_EXPORT_FAILED', + 'CAS_WRITE_UNATTRIBUTED', 'CAS_DELETE_MARKER') AND remote = 0 +ORDER BY code SETTINGS system_events_show_zero_values = 1; + +SELECT errorCodeToName(number) FROM numbers(10001, 6); +SELECT errorCodeToName(toInt32(-1)), errorCodeToName(10000), errorCodeToName(10007), errorCodeToName(2147483647); + +SET allow_custom_error_code_in_throwif = 1; +SELECT throwIf(1, 'Antalya namespace regression', toInt32(10001)); -- { serverError CATALOG_NAMESPACE_DISABLED } +SELECT throwIf(1, 'Antalya namespace regression', toInt32(10006)); -- { serverError CAS_DELETE_MARKER } +SELECT throwIf(1, 'Unknown Antalya code', toInt32(10007)); -- { serverError 10007 } From ff06afcb1760ff59b2b556b84d1cfa36eb045869 Mon Sep 17 00:00:00 2001 From: Mikhail Koviazin Date: Sat, 3 Oct 2026 14:03:42 +0200 Subject: [PATCH 2/2] Preserve upstream gap codes in the error log Skip only the shared out-of-range accounting slot in `system.error_log`, rather than every unnamed slot. Upstream gaps retain their actual numeric identities and must continue to be logged. Add regression coverage for codes 779, 780, and 899, retain sentinel suppression tests, and correct the accounting documentation. --- docs/en/antalya/error_codes.md | 10 ++++++---- src/Interpreters/ErrorLog.cpp | 2 +- .../test_antalya_error_codes/test.py | 20 ++++++++++++++++++- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/docs/en/antalya/error_codes.md b/docs/en/antalya/error_codes.md index 21251f340b34..92d32e66a811 100644 --- a/docs/en/antalya/error_codes.md +++ b/docs/en/antalya/error_codes.md @@ -66,7 +66,9 @@ separate. Unknown codes retain their identity in exceptions. Codes outside the upstream array, unless registered by Antalya, share an unnamed out-of-range accounting slot. Unassigned slots inside the upstream array keep their existing accounting behavior. Unknown codes do not acquire a registered name or increment -an Antalya error's counters. The aggregated error log skips unnamed slots, as do -`system.errors` and Prometheus, rather than reporting the shared slot number as -an exception identity. `system.query_log.exception_code` retains the original -numeric code. +an Antalya error's counters. The aggregated error log skips only the shared +out-of-range accounting slot, rather than reporting its slot number as an +exception identity. Errors in unassigned slots inside the upstream array remain +logged under their original numeric codes, even without a symbolic name. +`system.errors` and Prometheus omit unnamed entries. +`system.query_log.exception_code` retains the original numeric code. diff --git a/src/Interpreters/ErrorLog.cpp b/src/Interpreters/ErrorLog.cpp index 0b572c97d9e4..82627190f43a 100644 --- a/src/Interpreters/ErrorLog.cpp +++ b/src/Interpreters/ErrorLog.cpp @@ -130,7 +130,7 @@ void ErrorLog::stepFunction(TimePoint current_time) for (size_t index = 0, size = ErrorCodes::size(); index < size; ++index) { const auto code = ErrorCodes::getCode(index); - if (ErrorCodes::getName(code).empty()) + if (index == static_cast(ErrorCodes::end() - 1)) continue; const auto error = ErrorCodes::getValue(index).get(); auto & previous = previous_values.at(index); diff --git a/tests/integration/test_antalya_error_codes/test.py b/tests/integration/test_antalya_error_codes/test.py index 7dadfc34d2d0..89012ce7f6f7 100644 --- a/tests/integration/test_antalya_error_codes/test.py +++ b/tests/integration/test_antalya_error_codes/test.py @@ -31,7 +31,7 @@ def started_cluster(): cluster.shutdown() -@pytest.mark.parametrize("code", [-1, 779, 1009, 10000, 10007, 2147483647]) +@pytest.mark.parametrize("code", [-1, 1009, 10000, 10007, 2147483647]) def test_unknown_code_is_not_logged_as_sentinel(code): message = f"unknown code {code} must not become sentinel" error = origin.query_and_get_error( @@ -56,6 +56,24 @@ def test_unknown_code_is_not_logged_as_sentinel(code): ) +@pytest.mark.parametrize("code", [779, 780, 899]) +def test_upstream_gap_code_is_logged_with_original_identity(code): + message = f"upstream gap code {code} must retain its identity" + assert origin.query(f"SELECT errorCodeToName({code})") == "\n" + error = origin.query_and_get_error( + f"SELECT throwIf(1, '{message}', toInt32({code}))", settings=SETTINGS + ) + assert f"Code: {code}." in error + origin.query("SYSTEM FLUSH LOGS") + assert ( + origin.query( + "SELECT DISTINCT code FROM system.error_log " + f"WHERE position(last_error_message, '{message}') > 0" + ) + == f"{code}\n" + ) + + @pytest.mark.parametrize("code,name", ERRORS) def test_local_observability(code, name): message = f"Antalya error {code}"