brbzull0 opened a new issue, #13710:
URL: https://github.com/apache/trafficserver/issues/13710
### Impact
`traffic_ctl hostdb status <hostname>` returns partition objects with no
`records` key
for every partition that holds entries but no match for the filter.
`proxy.config.hostdb.partitions`
defaults to `64`, so on a populated HostDB a single-hostname query returns
the one real
match alongside up to 63 bare `{"id": N}` stubs.
A consumer iterating `partitions[].records` raises `KeyError` (or reads
`undefined`) on
those stubs, and has to special-case a key that the unfiltered response
always carries.
The output is also internally inconsistent: a partition holding zero entries
is dropped
from the array entirely, while a partition whose entries are all filtered
out appears as
a contentless stub. Two ways of having no records, two different shapes.
Default configuration, no plugin required — any operator who passes a
hostname argument
hits it. There is no workaround beyond defensive parsing on the consumer
side. The
unfiltered `traffic_ctl hostdb status` is unaffected.
```
Version: master @ 6cc6a2d150b5631d6b3d7aa82d52e687a2e250ff
Since: 6b10f87f1a4e5b4d699eeefca239a721f6a3ff2e, "Add HOSTNAME option to
traffic_ctl hostdb status (#12858)", 2026-02-06 — first released
in 10.2.0
Config: proxy.config.hostdb.partitions (default 64)
```
### Proof
`partition_node["records"]` is created only as a side effect of `push_back`,
inside the
record loop. A partition survives the `empty()` check at :96, gets its `id`
at :101, and
is pushed at :111 whether or not any record passed the hostname filter at
:105.
```cpp
// src/mgmt/rpc/handlers/hostdb/HostDB.cc:96-111
if (partition_entries.empty()) {
continue;
}
Node partition_node;
partition_node["id"] = i;
for (RefCountCacheHashEntry *entry : partition_entries) {
HostDBRecord *record = static_cast<HostDBRecord
*>(entry->item.get());
if (!hostname.empty() && record->name_view().find(hostname) ==
std::string_view::npos) {
continue;
}
partition_node["records"].push_back(*record);
}
partitions.push_back(partition_node);
```
https://github.com/apache/trafficserver/blob/6cc6a2d150b5631d6b3d7aa82d52e687a2e250ff/src/mgmt/rpc/handlers/hostdb/HostDB.cc#L96-L111
Emitting both shapes against the vendored yaml-cpp, for three partitions
whose entries
are all filtered out:
Current:
```json
{"partitions": [{"id": "0"}, {"id": "1"}, {"id": "2"}]}
```
Expected:
```json
{"partitions": [{"id": "0", "records": []}, {"id": "1", "records": []},
{"id": "2", "records": []}]}
```
The omission is schema-legal only by omission:
`src/mgmt/rpc/schema/hostdb_status_schema.json`
declares `records` as `"type": "array"` but carries no `required` array
anywhere in the
document, so a partition object with only `id` validates.
This is the same invariant #13609 established one level up. That PR changed
`partitions`
itself to be constructed as a sequence — `Node
partitions{YAML::NodeType::Sequence}` at
HostDB.cc:85 — and its test asserts `partitions=[]`
(`tests/gold_tests/traffic_ctl/traffic_ctl_json_null.test.py:58`), on the
principle that a
container node which may stay empty has to be constructed as one. `records`
is that same
principle one level deeper and was missed.
To be precise about scope: this is not a regression from #13609. The key is
absent, not
emitted as `~`. The behaviour dates from #12858.
Found by inspection of the merged tree and by emitting both shapes against
the bundled
yaml-cpp. Not reproduced against a running ATS with a populated HostDB, so
the stub count
of "up to 63" is derived from the default partition count rather than
measured.
### Proposed change
Construct `records` as an empty sequence before the loop, so a partition
that contributes
nothing still carries the key:
```diff
--- a/src/mgmt/rpc/handlers/hostdb/HostDB.cc
+++ b/src/mgmt/rpc/handlers/hostdb/HostDB.cc
@@ -98,7 +98,10 @@
}
Node partition_node;
- partition_node["id"] = i;
+ partition_node["id"] = i;
+ // Always a sequence, so a partition whose entries are all filtered
out by `hostname`
+ // emits `"records": []` rather than dropping the key.
+ partition_node["records"] = Node{YAML::NodeType::Sequence};
for (RefCountCacheHashEntry *entry : partition_entries) {
HostDBRecord *record = static_cast<HostDBRecord
*>(entry->item.get());
```
The alternative is to drop fully-filtered partitions from the array
entirely, matching the
`empty()` case at :96. That produces terser output, but it makes the
response shape depend
on the filter in a second way and diverges from the invariant #13609 just
established for
`partitions`. Constructing the sequence is the smaller and more consistent
change.
No compatibility concern: consumers that already tolerate the missing key
keep working, and
the JSON schema needs no edit since `records` is already typed as an array.
Still to do: a filtered-query assertion in `traffic_ctl_json_null.test.py`
alongside the
existing `partitions=[]` case. That needs a populated HostDB, which the
current test
deliberately avoids, so it may belong in a separate autest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
