Make record sorting in `pdnsutil zone list` optional.

The memory and CPU usage on zones with millions of records would become
a serious concern, and of course we can't really know the number of
records beforehand.

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>

diff --git a/docs/manpages/pdnsutil.1.rst b/docs/manpages/pdnsutil.1.rst
index 6d9efe6bc75ef1b5b52914e1aedc5a95c33ba00d..d8a8bc69dc4ac565a5d353dd3153d8611ef0d29b 100644 (file)
--- a/docs/manpages/pdnsutil.1.rst
+++ b/docs/manpages/pdnsutil.1.rst
@@ -22,6 +22,7 @@ Options
 -v, --verbose           Be more verbose
 -f, --force             Force an action
 -q, --quiet             Be quiet
+-s, --sort              Sort output when applicable
 --config-name <NAME>    Virtual configuration name
 --config-dir <DIR>      Location of pdns.conf. Default is /etc/powerdns.
 
@@ -217,7 +218,9 @@ zone increase-serial *ZONE*
 
 zone list *ZONE*
 
-    Show all records for *ZONE*.
+    Show all records for *ZONE*. Passing --sort or -s will sort the records
+    according to their name, but may require a lot of memory and processor
+    time on huge zones.
 
 zone list-all *KIND*
 

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index ef3aab064987a0bbb2db7e7058909ac73db5d07f..7bd3edfdf2d03f2521658d2c8d36803115a5b6d8 100644 (file)
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -61,9 +61,11 @@ po::variables_map g_vm;
 
 string g_programname="pdns";
 
+// Global option flag values
 namespace {
   bool g_force;
   bool g_quiet;
+  bool g_sort;
   bool g_verbose;
 }
 
@@ -1780,21 +1782,39 @@ static int listZone(const ZoneName &zone) {
     return EXIT_FAILURE;
   }
 
-  std::vector<DNSRecord> records;
-  DNSResourceRecord rr;
+  std::ostream::sync_with_stdio(false);
+  cout<<"$ORIGIN ."<<endl;
 
-  di.backend->list(zone, di.id);
-  while(di.backend->get(rr)) {
-    if(rr.qtype.getCode() != QType::ENT) {
-      records.emplace_back(DNSRecord(rr));
+  // Sorting zone output has the advantage of getting the database records as
+  // fast as possible, which - on backends where fetching records involve a
+  // transaction, such as LMDB - reduces contention on the database. But on
+  // the other hands, for huge zones, this can make the memory usage of
+  // pdnsutil grow to insanely unreasonable levels... so you get to choose
+  // your poison.
+  DNSResourceRecord drr;
+  if (g_sort) {
+    std::vector<DNSRecord> records;
+
+    di.backend->list(zone, di.id);
+    while(di.backend->get(drr)) {
+      if(drr.qtype.getCode() != QType::ENT) {
+        records.emplace_back(DNSRecord(drr));
+      }
+    }
+    sort(records.begin(), records.end(), DNSRecord::prettyCompare);
+    for (const auto& rec : records) {
+      std::cout << formatRecord(rec) << std::endl;
     }
   }
-  sort(records.begin(), records.end(), DNSRecord::prettyCompare);
-  cout<<"$ORIGIN ."<<endl;
-  std::ostream::sync_with_stdio(false);
-  for (const auto& rec : records) {
-    std::cout << formatRecord(rec) << std::endl;
+  else {
+    di.backend->list(zone, di.id);
+    while(di.backend->get(drr)) {
+      if(drr.qtype.getCode() != QType::ENT) {
+        std::cout << formatRecord(DNSRecord(drr)) << std::endl;
+      }
+    }
   }
+
   cout.flush();
   return EXIT_SUCCESS;
 }
@@ -5994,6 +6014,7 @@ try
     ("verbose,v", "be verbose")
     ("force,f", "force an action")
     ("quiet,q", "be quiet")
+    ("sort,s", "sort output when applicable")
     ("config-name", po::value<string>()->default_value(""), "virtual configuration name")
     ("config-dir", po::value<string>()->default_value(SYSCONFDIR), "location of pdns.conf")
     ("no-colors", "do not use colors in output")
@@ -6016,6 +6037,7 @@ try
 
   g_force = g_vm.count("force") != 0;
   g_quiet = g_vm.count("quiet") != 0;
+  g_sort = g_vm.count("sort") != 0;
   g_verbose = g_vm.count("verbose") != 0;
 
   if (g_vm.count("version") != 0) {

diff --git a/regression-tests.nobackend/gsqlite3-corrupted-record/command b/regression-tests.nobackend/gsqlite3-corrupted-record/command
index 330a0acd794ea0e7d0e4fb98557e6ebce200b27f..585c0f65711f8fca2c17e63aec0b083977c6687c 100755 (executable)
--- a/regression-tests.nobackend/gsqlite3-corrupted-record/command
+++ b/regression-tests.nobackend/gsqlite3-corrupted-record/command
@@ -26,7 +26,7 @@ echo "UPDATE records SET name='this-name-turns-out-to-be-an-invalid-dns-name-bec
 echo "UPDATE records SET ordername='..' WHERE name='never.bug.less';" | sqlite3 pdns.sqlite3 2>&1
 
 # Check that pdnsutil zone list doesn't show the invalid records
-$PDNSUTIL $ARGS zone list bug.less
+$PDNSUTIL -s $ARGS zone list bug.less
 
 # Check that pdnsutil zone check reports the invalid records
 $PDNSUTIL $ARGS zone check bug.less

diff --git a/regression-tests.nobackend/lmdb-schema-upgrade/command b/regression-tests.nobackend/lmdb-schema-upgrade/command
index 03790eec9e601cba8e316a7b1a90602ca056aa96..4f9802325289972a31bc3214af52465810bc6276 100755 (executable)
--- a/regression-tests.nobackend/lmdb-schema-upgrade/command
+++ b/regression-tests.nobackend/lmdb-schema-upgrade/command
@@ -24,7 +24,7 @@ EOF
   for zone in $(grep 'zone ' "${rootPath}/../../regression-tests/named.conf"  | cut -f2 -d\" | grep -v '^nztest.com$')
   do
     if [ "$zone" != "." ]; then
-      $PDNSUTIL -q --config-dir="${workdir}" --config-name=lmdb list-zone $zone
+      $PDNSUTIL -q --config-dir="${workdir}" --config-name=lmdb --sort list-zone $zone
     fi
   done
   rm -r $workdir

diff --git a/regression-tests/mysqldiff b/regression-tests/mysqldiff
index c6da66f41423e2e18feb7ebd4427164764ae1aae..5910194a18d86e2c8cf7ff86f7342c616e426ca2 100755 (executable)
--- a/regression-tests/mysqldiff
+++ b/regression-tests/mysqldiff
@@ -52,7 +52,7 @@ lmdb)
     # Maybe we should add a pdnsutil backend-cmd to retrieve these results in
     # same format as the mysql query...
     $PDNSUTIL --config-dir=. --config-name=$backend \
-        list-zone $zonewithvariant | grep -vwF SOA \
+        --sort zone list $zonewithvariant | grep -vwF SOA \
         > ${testsdir}/${testname}/$step
     ;;
 esac

diff --git a/regression-tests/tests/pdnsutil-zone-handling/command b/regression-tests/tests/pdnsutil-zone-handling/command
index 010fc2be7e51da2d6d7195577c91c2a3df92d21d..9228d055ace7c80fd0565003ae6bee18026a4b0e 100755 (executable)
--- a/regression-tests/tests/pdnsutil-zone-handling/command
+++ b/regression-tests/tests/pdnsutil-zone-handling/command
@@ -6,7 +6,7 @@
 # --enable-verbose-logging removed.
 
 pdnsutil_wrapper() {
-$PDNSUTIL --config-dir=. --config-name=$backend "$@" 2>&1 | egrep -v 'destructor'
+$PDNSUTIL --config-dir=. --config-name=$backend --sort "$@" 2>&1 | egrep -v 'destructor'
 }
 
 ZONE=bug.less