Refactor dns_journal_rollforward() to work over opened journal

Too much logic was cramped inside the dns_journal_rollforward() that
made it harder to follow.  The dns_journal_rollforward() was refactored
to work over already opened journal and some of the previous logic was
moved to new static zone_journal_rollforward() that separates the
journal "rollforward" logic from the "zone" logic.

(cherry picked from commit 55b942b4a02519f946c1613b97f6f45ff1bb4426)

diff --git a/bin/tests/system/journal/tests.sh b/bin/tests/system/journal/tests.sh
index 94df34387e3a3e2b0c9705ed4db34bbd7980055d..acb6ad08e1f1d346336c0d452991792d36c543f3 100644 (file)
--- a/bin/tests/system/journal/tests.sh
+++ b/bin/tests/system/journal/tests.sh
@@ -28,7 +28,7 @@ ret=0
 dig_with_opts changed soa > 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 changed/IN: retried using old journal format' ns1/named.run > /dev/null || ret=1
+grep 'zone changed/IN: journal rollforward completed successfully using old journal format' ns1/named.run > /dev/null || ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -62,7 +62,8 @@ ret=0
 dig_with_opts changed2 soa > 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 changed2/IN: retried using old journal format' ns1/named.run > /dev/null && ret=1
+grep 'zone changed2/IN: journal rollforward completed successfully: success' ns1/named.run > /dev/null || ret=1
+grep 'zone changed2/IN: journal rollforward completed successfully using old journal format' ns1/named.run > /dev/null && ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -72,7 +73,8 @@ ret=0
 dig_with_opts unchanged2 soa > dig.out.test$n
 grep 'status: NOERROR' dig.out.test$n > /dev/null || ret=1
 grep '2012010901' dig.out.test$n > /dev/null || ret=1
-grep 'zone unchanged2/IN: retried using old journal format' ns1/named.run > /dev/null && ret=1
+grep 'zone unchanged2/IN: journal rollforward completed successfully' ns1/named.run > /dev/null && ret=1
+grep 'zone unchanged2/IN: journal rollforward completed successfully using old journal format' ns1/named.run > /dev/null && ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -90,7 +92,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: up to date' ns1/named.run > /dev/null || ret=1
+grep 'zone ixfr/IN: journal rollforward completed successfully using old journal format: up to date' ns1/named.run > /dev/null || ret=1
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
@@ -107,7 +109,7 @@ 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: success' ns1/named.run > /dev/null || ret=1
+grep 'zone hdr1d1d2d1d2/IN: journal rollforward completed successfully using old journal format: 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"
@@ -129,8 +131,7 @@ 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: 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 hdr1d2d1d2d1/IN: journal rollforward completed successfully using old journal format: success' 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"

diff --git a/lib/dns/include/dns/journal.h b/lib/dns/include/dns/journal.h
index aad1667bf87fcdecc007f5e4029f2fca827091c1..c2d869702006172b1c7237dd9ceaf0d22c0670c1 100644 (file)
--- a/lib/dns/include/dns/journal.h
+++ b/lib/dns/include/dns/journal.h
@@ -187,6 +187,12 @@ dns_journal_empty(dns_journal_t *j);
  * Find out if a journal is empty.
  */
 
+bool
+dns_journal_recovered(dns_journal_t *j);
+/*<
+ * Find out if the journal could be opened using old journal format
+ */
+
 uint32_t
 dns_journal_first_serial(dns_journal_t *j);
 uint32_t
@@ -248,23 +254,18 @@ 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, bool *recovered);
+dns_journal_rollforward(dns_journal_t *j, dns_db_t *db, unsigned int options);
 /*%<
  * Roll forward (play back) the journal file "filename" into the
  * database "db".  This should be called when the server starts
  * after a shutdown or crash.
  *
  * Requires:
- *\li   'mctx' is a valid memory context.
+ *\li   'journal' is a valid journal
  *\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.
  *\li  ISC_R_NOTFOUND when current serial in not in journal.
  *\li  ISC_R_RANGE when current serial in not in journals range.
  *\li  DNS_R_UPTODATE when the database was already up to date.

diff --git a/lib/dns/journal.c b/lib/dns/journal.c
index 2245122513ce10609af01a5902a9503e2633fbbf..c4931ba10146cc1d22de4a8b6b2cda5f5ed9159d 100644 (file)
--- a/lib/dns/journal.c
+++ b/lib/dns/journal.c
@@ -1450,8 +1450,8 @@ dns_journal_destroy(dns_journal_t **journalp) {
 
 /* XXX Share code with incoming IXFR? */
 
-static isc_result_t
-roll_forward(dns_journal_t *j, dns_db_t *db, unsigned int options) {
+isc_result_t
+dns_journal_rollforward(dns_journal_t *j, dns_db_t *db, unsigned int options) {
        isc_buffer_t source; /* Transaction data from disk */
        isc_buffer_t target; /* Ditto after _fromwire check */
        uint32_t db_serial;  /* Database SOA serial */
@@ -1597,39 +1597,6 @@ failure:
        return (result);
 }
 
-isc_result_t
-dns_journal_rollforward(isc_mem_t *mctx, dns_db_t *db, unsigned int options,
-                       const char *filename, bool *recovered) {
-       dns_journal_t *j = NULL;
-       isc_result_t result;
-
-       REQUIRE(DNS_DB_VALID(db));
-       REQUIRE(filename != NULL);
-
-       result = dns_journal_open(mctx, filename, DNS_JOURNAL_READ, &j);
-       if (result == ISC_R_NOTFOUND) {
-               isc_log_write(JOURNAL_DEBUG_LOGARGS(3), "no journal file, but "
-                                                       "that's OK");
-               return (DNS_R_NOJOURNAL);
-       }
-       if (result != ISC_R_SUCCESS) {
-               return (result);
-       }
-
-       if (JOURNAL_EMPTY(&j->header)) {
-               CHECK(DNS_R_UPTODATE);
-       }
-
-       result = roll_forward(j, db, options);
-
-failure:
-       if (recovered != NULL) {
-               *recovered = j->recovered;
-       }
-       dns_journal_destroy(&j);
-       return (result);
-}
-
 isc_result_t
 dns_journal_print(isc_mem_t *mctx, uint32_t flags, const char *filename,
                  FILE *file) {
@@ -1787,6 +1754,11 @@ dns_journal_empty(dns_journal_t *j) {
        return (JOURNAL_EMPTY(&j->header));
 }
 
+bool
+dns_journal_recovered(dns_journal_t *j) {
+       return (j->recovered);
+}
+
 uint32_t
 dns_journal_first_serial(dns_journal_t *j) {
        return (j->header.begin.serial);

diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index 2e3260dcea2266c9c39901faf5fdcf942061de2f..46bfe243b543a0d9e431560ef6c0d4e04ca97348 100644 (file)
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -429,6 +429,7 @@ dns_journal_last_serial
 dns_journal_next_rr
 dns_journal_open
 dns_journal_print
+dns_journal_recovered
 dns_journal_rollforward
 dns_journal_set_sourceserial
 dns_journal_write_transaction

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index b55629082fbdaff9ab3f4469421d3e57d552d10c..8bc636d4a87ab7137b30e5a5c9f8d13e38cd32e5 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -893,6 +893,9 @@ static void
 setrl(isc_ratelimiter_t *rl, unsigned int *rate, unsigned int value);
 static void
 zone_journal_compact(dns_zone_t *zone, dns_db_t *db, uint32_t serial);
+static isc_result_t
+zone_journal_rollforward(dns_zone_t *zone, dns_db_t *db, bool *needdump,
+                        bool *fixjournal);
 
 #define ENTER zone_debuglog(zone, me, 1, "enter")
 
@@ -4637,7 +4640,6 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
        bool hasinclude = DNS_ZONE_FLAG(zone, DNS_ZONEFLG_HASINCLUDE);
        bool nomaster = false;
        bool had_db = false;
-       unsigned int options;
        dns_include_t *inc;
        bool is_dynamic = false;
 
@@ -4725,45 +4727,11 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
            !DNS_ZONE_OPTION(zone, DNS_ZONEOPT_NOMERGE) &&
            !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED))
        {
-               if (zone->type == dns_zone_master &&
-                   (inline_secure(zone) ||
-                    (zone->update_acl != NULL || zone->ssutable != NULL)))
-               {
-                       options = DNS_JOURNALOPT_RESIGN;
-               } else {
-                       options = 0;
-               }
-               result = dns_journal_rollforward(zone->mctx, db, options,
-                                                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,
-                                     "journal rollforward failed: %s",
-                                     dns_result_totext(result));
-                       goto cleanup;
-               }
-               if (result == ISC_R_NOTFOUND || result == ISC_R_RANGE) {
-                       dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD,
-                                     ISC_LOG_ERROR,
-                                     "journal rollforward failed: "
-                                     "journal out of sync with zone");
+               result = zone_journal_rollforward(zone, db, &needdump,
+                                                 &fixjournal);
+               if (result != ISC_R_SUCCESS) {
                        goto cleanup;
                }
-               dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_DEBUG(1),
-                             "journal rollforward completed "
-                             "successfully: %s",
-                             dns_result_totext(result));
-               if (result == ISC_R_SUCCESS) {
-                       needdump = true;
-               }
-               if (fixjournal) {
-                       dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD,
-                                     ISC_LOG_ERROR,
-                                     "retried using old journal format");
-               }
        }
 
        /*
@@ -11261,6 +11229,82 @@ unlock:
        UNLOCK_ZONE(zone);
 }
 
+static isc_result_t
+zone_journal_rollforward(dns_zone_t *zone, dns_db_t *db, bool *needdump,
+                        bool *fixjournal) {
+       dns_journal_t *journal = NULL;
+       unsigned int options;
+       isc_result_t result;
+
+       if (zone->type == dns_zone_master &&
+           (inline_secure(zone) ||
+            (zone->update_acl != NULL || zone->ssutable != NULL)))
+       {
+               options = DNS_JOURNALOPT_RESIGN;
+       } else {
+               options = 0;
+       }
+
+       result = dns_journal_open(zone->mctx, zone->journal, DNS_JOURNAL_READ,
+                                 &journal);
+       if (result == ISC_R_NOTFOUND) {
+               dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_DEBUG(3),
+                             "no journal file, but that's OK ");
+               return (ISC_R_SUCCESS);
+       } else if (result != ISC_R_SUCCESS) {
+               dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_ERROR,
+                             "journal open failed: %s",
+                             dns_result_totext(result));
+               return (result);
+       }
+
+       if (dns_journal_empty(journal)) {
+               dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_DEBUG(1),
+                             "journal empty");
+               dns_journal_destroy(&journal);
+               return (ISC_R_SUCCESS);
+       }
+
+       result = dns_journal_rollforward(journal, db, options);
+       switch (result) {
+       case ISC_R_SUCCESS:
+               *needdump = true;
+               /* FALLTHROUGH */
+       case DNS_R_UPTODATE:
+               if (dns_journal_recovered(journal)) {
+                       *fixjournal = true;
+                       dns_zone_logc(
+                               zone, DNS_LOGCATEGORY_ZONELOAD,
+                               ISC_LOG_DEBUG(1),
+                               "journal rollforward completed successfully "
+                               "using old journal format: %s",
+                               dns_result_totext(result));
+               } else {
+                       dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD,
+                                     ISC_LOG_DEBUG(1),
+                                     "journal rollforward completed "
+                                     "successfully: %s",
+                                     dns_result_totext(result));
+               }
+
+               dns_journal_destroy(&journal);
+               return (ISC_R_SUCCESS);
+       case ISC_R_NOTFOUND:
+       case ISC_R_RANGE:
+               dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_ERROR,
+                             "journal rollforward failed: journal out of sync "
+                             "with zone");
+               dns_journal_destroy(&journal);
+               return (result);
+       default:
+               dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_ERROR,
+                             "journal rollforward failed: %s",
+                             dns_result_totext(result));
+               dns_journal_destroy(&journal);
+               return (result);
+       }
+}
+
 static void
 zone_journal_compact(dns_zone_t *zone, dns_db_t *db, uint32_t serial) {
        isc_result_t result;