Fixing a recoverable journal should not result in the zone being written

when dns_journal_rollforward returned ISC_R_RECOVERABLE the distintion
between 'up to date' and 'success' was lost, as a consequence
zone_needdump() was called writing out the zone file when it shouldn't
have been.   This change restores that distintion.  Adjust system
test to reflect visible changes.

(cherry picked from commit ec7a9af381708b341258b2399c901a3df0a6fd56)

diff --git a/bin/tests/system/journal/tests.sh b/bin/tests/system/journal/tests.sh
index 824c3723a26e43d8619134cc9cf32992b81adc5a..94df34387e3a3e2b0c9705ed4db34bbd7980055d 100644 (file)
--- a/bin/tests/system/journal/tests.sh
+++ b/bin/tests/system/journal/tests.sh
@@ -90,7 +90,7 @@ ret=0
 dig_with_opts -t soa ixfr > dig.out.test$n
 grep 'status: NOERROR' dig.out.test$n > /dev/null || ret=1
 grep '2012010902' dig.out.test$n > /dev/null || ret=1
-grep 'zone ixfr/IN: journal rollforward completed successfully: recoverable' ns1/named.run > /dev/null || ret=1
+grep 'zone ixfr/IN: journal rollforward completed successfully: up to date' ns1/named.run > /dev/null || ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -107,7 +107,9 @@ ret=0
 dig_with_opts -t soa hdr1d1d2d1d2 > dig.out.test$n
 grep 'status: NOERROR' dig.out.test$n > /dev/null || ret=1
 grep '2012010905' dig.out.test$n > /dev/null || ret=1
-grep 'zone hdr1d1d2d1d2/IN: journal rollforward completed successfully: recoverable' ns1/named.run > /dev/null || ret=1
+grep 'zone hdr1d1d2d1d2/IN: journal rollforward completed successfully: success' ns1/named.run > /dev/null || ret=1
+grep 'zone_journal_compact: zone hdr1d1d2d1d2/IN: repair full journal' ns1/named.run > /dev/null || ret=1
+grep 'hdr1d1d2d1d2/IN: dns_journal_compact: success' ns1/named.run > /dev/null || ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -127,7 +129,10 @@ ret=0
 dig_with_opts -t soa hdr1d2d1d2d1 > dig.out.test$n
 grep 'status: NOERROR' dig.out.test$n > /dev/null || ret=1
 grep '2012010905' dig.out.test$n > /dev/null || ret=1
-grep 'zone hdr1d2d1d2d1/IN: journal rollforward completed successfully: recoverable' ns1/named.run > /dev/null || ret=1
+grep 'zone hdr1d2d1d2d1/IN: journal rollforward completed successfully: success' ns1/named.run > /dev/null || ret=1
+grep 'zone hdr1d2d1d2d1/IN: retried using old journal format' ns1/named.run > /dev/null || ret=1
+grep 'zone_journal_compact: zone hdr1d2d1d2d1/IN: repair full journal' ns1/named.run > /dev/null || ret=1
+grep 'zone hdr1d2d1d2d1/IN: dns_journal_compact: success' ns1/named.run > /dev/null || ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -175,10 +180,15 @@ status=`expr $status + $ret`
 n=`expr $n + 1`
 echo_i "check max-journal-size works after journal update ($n)"
 ret=0
-# a dump should have been triggered by repairing the journal,
-# which would have resulted in the journal already being
-# compacted.
-[ $(wc -c < ns1/maxjournal.db.jnl) -lt 4000 ] || ret=1
+# journal was repaired, it should still be big
+[ $(wc -c < ns1/maxjournal.db.jnl) -gt 12000 ] || ret=1
+# the zone hasn't been dumped yet, so 'rndc sync' should work without
+# needing a zone update first.
+rndc_with_opts 10.53.0.1 sync maxjournal
+check_size() (
+    [ $(wc -c < ns1/maxjournal.db.jnl) -lt 4000 ]
+)
+retry_quiet 10 check_size || ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 

diff --git a/lib/dns/include/dns/journal.h b/lib/dns/include/dns/journal.h
index be6d72ba43f6adcb42790a60bf18a8669d5a775a..aad1667bf87fcdecc007f5e4029f2fca827091c1 100644 (file)
--- a/lib/dns/include/dns/journal.h
+++ b/lib/dns/include/dns/journal.h
@@ -249,7 +249,7 @@ dns_journal_current_rr(dns_journal_t *j, dns_name_t **name, uint32_t *ttl,
 
 isc_result_t
 dns_journal_rollforward(isc_mem_t *mctx, dns_db_t *db, unsigned int options,
-                       const char *filename);
+                       const char *filename, bool *recovered);
 /*%<
  * Roll forward (play back) the journal file "filename" into the
  * database "db".  This should be called when the server starts
@@ -260,6 +260,8 @@ dns_journal_rollforward(isc_mem_t *mctx, dns_db_t *db, unsigned int options,
  *\li  'db' is a valid database which does not have a version
  *           open for writing.
  *\li   'filename' is the name of the journal file belonging to 'db'.
+ *\li  'recovered' is a optional pointer to a boolean that returns
+ *     whether a recoverable error was detected.
  *
  * Returns:
  *\li  DNS_R_NOJOURNAL when journal does not exist.
@@ -268,8 +270,6 @@ dns_journal_rollforward(isc_mem_t *mctx, dns_db_t *db, unsigned int options,
  *\li  DNS_R_UPTODATE when the database was already up to date.
  *\li  ISC_R_SUCCESS journal has been applied successfully to the
  *      database without any issues.
- *\li  DNS_R_RECOVERABLE if successful or up to date, but the journal
- *      was found to contain at least one outdated transaction header.
  *
  *     others
  */

diff --git a/lib/dns/journal.c b/lib/dns/journal.c
index b9318e8de6c58f6b9bd358cba2b5bbddfbf3f36b..2245122513ce10609af01a5902a9503e2633fbbf 100644 (file)
--- a/lib/dns/journal.c
+++ b/lib/dns/journal.c
@@ -1599,7 +1599,7 @@ failure:
 
 isc_result_t
 dns_journal_rollforward(isc_mem_t *mctx, dns_db_t *db, unsigned int options,
-                       const char *filename) {
+                       const char *filename, bool *recovered) {
        dns_journal_t *j = NULL;
        isc_result_t result;
 
@@ -1621,12 +1621,11 @@ dns_journal_rollforward(isc_mem_t *mctx, dns_db_t *db, unsigned int options,
        }
 
        result = roll_forward(j, db, options);
-       if ((result == ISC_R_SUCCESS || result == DNS_R_UPTODATE) &&
-           j->recovered) {
-               result = DNS_R_RECOVERABLE;
-       }
 
 failure:
+       if (recovered != NULL) {
+               *recovered = j->recovered;
+       }
        dns_journal_destroy(&j);
        return (result);
 }

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 01bc1c04a2f51eaa2d28201a2fbc8250a08cbc49..b55629082fbdaff9ab3f4469421d3e57d552d10c 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -4734,10 +4734,10 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
                        options = 0;
                }
                result = dns_journal_rollforward(zone->mctx, db, options,
-                                                zone->journal);
-               if (result != ISC_R_SUCCESS && result != DNS_R_RECOVERABLE &&
-                   result != ISC_R_NOTFOUND && result != DNS_R_UPTODATE &&
-                   result != DNS_R_NOJOURNAL && result != ISC_R_RANGE)
+                                                zone->journal, &fixjournal);
+               if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND &&
+                   result != DNS_R_UPTODATE && result != DNS_R_NOJOURNAL &&
+                   result != ISC_R_RANGE)
                {
                        dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD,
                                      ISC_LOG_ERROR,
@@ -4758,12 +4758,11 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
                              dns_result_totext(result));
                if (result == ISC_R_SUCCESS) {
                        needdump = true;
-               } else if (result == DNS_R_RECOVERABLE) {
+               }
+               if (fixjournal) {
                        dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD,
                                      ISC_LOG_ERROR,
                                      "retried using old journal format");
-                       needdump = true;
-                       fixjournal = true;
                }
        }
 
@@ -5087,12 +5086,12 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
 
        result = ISC_R_SUCCESS;
 
+       if (fixjournal) {
+               DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_FIXJOURNAL);
+               zone_journal_compact(zone, zone->db, 0);
+       }
        if (needdump) {
-               if (fixjournal) {
-                       DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_FIXJOURNAL);
-                       zone_journal_compact(zone, zone->db, 0);
-                       zone_needdump(zone, 0);
-               } else if (zone->type == dns_zone_key) {
+               if (zone->type == dns_zone_key) {
                        zone_needdump(zone, 30);
                } else {
                        zone_needdump(zone, DNS_DUMP_DELAY);