Repository navigation
Conversation
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.
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.
| { | ||
| if (code >= 0 && code <= END) | ||
| return code; | ||
| for (size_t index = 0; index < antalya_entries.size(); ++index) |
There was a problem hiding this comment.
Why 'for' here and in other places?
antalya_entries has codes from ANTALYA_ERROR_CODE_BASE +1 to ANTALYA_ERROR_CODE_BASE + antalya_entries.size(), we can just check the range.
There was a problem hiding this comment.
They are contiguous today, but contiguity is not an invariant: IDs may have gaps and must never be reused. The lookup therefore checks registered entries rather than assuming code minus base equals the storage index.
We can add a compile-time check for them being contiguous if you think it's worth it. But I don't expect us to have too many items
There was a problem hiding this comment.
Upstream also has gaps, but check is if (code >= 0 && code <= END), and it's ok from my point of view.
There was a problem hiding this comment.
Upstream uses values[END + 1], while ours uses sparse array. The last id in the array is not guaranteed to be tied to the size of the Antalya error codes array.
However I guess we could use the same pattern here, which would also allow us to use that check
There was a problem hiding this comment.
Why we need gaps in Antalya errors? The only one reason to have gaps is predefined ranges for different types of errors. The only one reason to have predefined ranges is need to process range at once, if (error_code >= RANGE_BEGIN && error_code < RANGE_END)`. Do we really need this?
There was a problem hiding this comment.
We don't have the gaps now, but that doesn't mean we won't have them later. If some feature that carries a custom error code is going to be removed, we probably won't be sticking to that error code and can remove it. If we reuse the code from it, it might introduce similar problem that led to this PR in the first place.
This branch has not been deployed
3 participants
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
UInt16bounds. Preserve the upstreamvaluesAPI and name-table construction to reduce forward-porting overhead.Document the numeric migration and add lookup, accounting, log-column, and distributed-forwarding regression tests.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Separate Antalya error codes from upstream ones to prevent any collisions in the future
Documentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: