Skip to content

hostdb: status omits "records" for a partition whose entries are all filtered out #13710

Description

@brbzull0

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.

// 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);

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);

Emitting both shapes against the vendored yaml-cpp, for three partitions whose entries
are all filtered out:

Current:

{"partitions": [{"id": "0"}, {"id": "1"}, {"id": "2"}]}

Expected:

{"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:

--- 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions