Copilot commented on code in PR #13634:
URL: https://github.com/apache/trafficserver/pull/13634#discussion_r3932263252
##########
src/records/RecCore.cc:
##########
@@ -582,11 +583,34 @@ RecLookupMatchingRecords(unsigned rec_type, const char
*match, void (*callback)(
return REC_ERR_FAIL;
}
+ // The pattern comes from the admin_lookup_records RPC and is run against
every record name,
+ // and that RPC is served inline on a single thread, so an unbounded pattern
stalls it: 19s per
+ // request across a stock server's 1328 names, against 123ms at this limit.
+ //
+ // Raising the limit costs run time linearly, while the longest name a
pattern can still resolve
+ // grows only as its square root, since proving no match costs about L^2/2
steps. 50,000 resolves
+ // names up to ~310 characters, 4.7x the 66 character longest a stock build
registers. The 1750
+ // used elsewhere in the tree reaches only ~59 and cannot resolve what ships.
+ static constexpr uint32_t REC_REGEX_MATCH_LIMIT = 50000;
+
+ RegexMatchContext match_context;
+ match_context.set_match_limit(REC_REGEX_MATCH_LIMIT);
+ RegexMatches matches;
+
+ // Exhausting the limit is the only failure this bound provokes, and it is
not a verdict, so count
+ // those names and report once per lookup instead of silently dropping
records from the caller's
+ // results. Any other negative return keeps the pre-existing
treat-as-no-match behavior.
+ unsigned indeterminate = 0;
+
if ((rec_type & (RECT_PROCESS | RECT_NODE | RECT_PLUGIN))) {
// First find the new metrics, this is a bit of a hack, because we still
use the old
// librecords callback with a "pseudo" record.
for (auto &&[name, type, val] : ts::Metrics::instance()) {
- if (regex.exec(name.data())) {
+ int const rc = regex.exec(name.data(), matches, 0, &match_context);
+
Review Comment:
`ts::Metrics::instance()` yields `std::string_view` names; passing
`name.data()` forces an extra `strlen()` and loses the known length. Pass
`name` directly to `Regex::exec()` to avoid the redundant scan and handle any
embedded NULs correctly.
This issue also appears on line 655 of the same file.
--
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]
