dnsdist: Rule for basing decisions on outstanding queries in a pool

[phonedph1]

Short description

QPS sometimes doesn't matter, but outstanding queries/backend pressure does. We have been doing this for years in Lua directly with great success.

Not sure where this could be added to test code.

Basic usage:

> addAction(PoolOutstandingRule("", 1), PoolAction("spillover"))
<spam a bunch of stuff>
> showRules()
#   Name                             Matches Rule                                                     Action
0                                         18 pool '' outstanding > 1                                  to pool spillover

Checklist

I have:

• read the CONTRIBUTING.md document
• compiled this code
• tested this code
• included documentation (including possible behaviour changes)
• documented the code
• added or modified regression test(s)
• added or modified unit test(s)

[rgacogne]

A couple nits but the PR looks good, thanks! It would likely not be easy to test in the regression tests since it would require raising the outstanding queries count of the backend in some way. Unit testing in test-dnsdistrules_cc.cc might not be too hard to implement, though.

[phonedph1]

Apart from needing to copy getDQ here, would this be suitable?

--- a/pdns/dnsdistdist/test-dnsdistrules_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdistrules_cc.cc
@@ -7,6 +7,23 @@
 
 #include "dnsdist-rules.hh"
 
+static DNSQuestion getDQ(const DNSName* providedName = nullptr)
+{
+  static const DNSName qname("powerdns.com.");
+  static const ComboAddress lc("127.0.0.1:53");
+  static const ComboAddress rem("192.0.2.1:42");
+  static struct timespec queryRealTime;
+  static PacketBuffer packet(sizeof(dnsheader));
+
+  uint16_t qtype = QType::A;
+  uint16_t qclass = QClass::IN;
+  auto proto = dnsdist::Protocol::DoUDP;
+  gettime(&queryRealTime, true);
+
+  DNSQuestion dq(providedName ? providedName : &qname, qtype, qclass, &lc, &rem, packet, proto, &queryRealTime);
+  return dq;
+}
+
 BOOST_AUTO_TEST_SUITE(dnsdistluarules_cc)
 
 BOOST_AUTO_TEST_CASE(test_MaxQPSIPRule) {
@@ -101,5 +118,32 @@ BOOST_AUTO_TEST_CASE(test_MaxQPSIPRule) {
   BOOST_CHECK_EQUAL(scanned, 0U);
 }
 
+BOOST_AUTO_TEST_CASE(test_poolOutstandingRule) {
+  auto dq = getDQ();
+
+  ServerPool sp{};
+  auto ds1 = std::make_shared<DownstreamState>(ComboAddress("192.0.2.1:53"));
+  auto ds2 = std::make_shared<DownstreamState>(ComboAddress("192.0.2.2:53"));
+
+  /* increase the outstanding count of both */
+  ds1->outstanding = 420;
+  ds2->outstanding = 30;
+
+  sp.addServer(ds1);
+  sp.addServer(ds2);
+
+  BOOST_CHECK_EQUAL(sp.poolLoad(), 420+30);
+
+  auto localPool = g_pools.getCopy();
+  addServerToPool(localPool, "test", ds1);
+  addServerToPool(localPool, "test", ds2);
+  g_pools.setState(localPool);
+
+  PoolOutstandingRule pOR1("test", 10);
+  BOOST_CHECK_EQUAL(pOR1.matches(&dq), true);
+
+  PoolOutstandingRule pOR2("test", 1000);
+  BOOST_CHECK_EQUAL(pOR2.matches(&dq), false);
+}
 
 BOOST_AUTO_TEST_SUITE_END()

[rgacogne]

Apart from needing to copy getDQ here, would this be suitable?

That looks wonderful, and don't worry too much about copying getDQ, that's fine by me.