zrhoffman commented on a change in pull request #4409: Optimize TR DNSSEC zone 
re-signing
URL: https://github.com/apache/trafficcontrol/pull/4409#discussion_r382244989
 
 

 ##########
 File path: 
traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManager.java
 ##########
 @@ -460,18 +499,37 @@ private static void generateZones(final TrafficRouter 
tr, final LoadingCache<Zon
                        LOGGER.fatal("Unable to create zone: " + 
ex.getMessage(), ex);
                }
 
-               primeZoneCache(domain, name, list, tr, zc, dzc, 
generationTasks, primingTasks, ds);
+               primeZoneCache(domain, name, list, tr, zc, dzc, 
generationTasks, primingTasks, ds, newDomainsToZoneKeys);
 
                return records;
        }
 
-       @SuppressWarnings("PMD.CyclomaticComplexity")
+       @SuppressWarnings({"PMD.CyclomaticComplexity", 
"PMD.ExcessiveParameterList"})
        private static void primeZoneCache(final String domain, final Name 
name, final List<Record> list, final TrafficRouter tr,
                        final LoadingCache<ZoneKey, Zone> zc, final 
LoadingCache<ZoneKey, Zone> dzc, final List<Runnable> generationTasks,
-                       final BlockingQueue<Runnable> primingTasks, final 
DeliveryService ds) {
+                       final BlockingQueue<Runnable> primingTasks, final 
DeliveryService ds, final ConcurrentMap<String, ZoneKey> newDomainsToZoneKeys) {
                generationTasks.add(() -> {
                        try {
-                               final Zone zone = 
zc.get(signatureManager.generateZoneKey(name, list)); // cause the zone to be 
loaded into the new cache
+                               final ZoneKey newZoneKey = 
signatureManager.generateZoneKey(name, list);
+                               if (tr.isDnssecZoneDiffingEnabled() && 
domainsToZoneKeys.containsKey(domain)) {
 
 Review comment:
   The new method would not need a `Zone` passed in if you are okay also 
calling declaring setting `Zone zone = zc.get(newZoneKey)` outside that method 
[for line 
541](https://github.com/apache/trafficcontrol/blob/afc6baab28/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManager.java#L541).
   
   The early return on L522 doesn't need to make splitting this out trickier if 
you are okay having multiple returns. Something like:
   
   ```java
   private static boolean diffZones(...) throws ExecutionException {
       ...
       if (!tr.isDnssecZoneDiffingEnabled() || 
!domainsToZoneKeys.containsKey(domain)) {
           return false;
       }
       ...
       if (!zonesAreEqual(newZoneKey.getRecords(), oldZoneKey.getRecords())) {
           ...
           return false;
       }
       ...
       if (oldZone != null) {
           ...
       }
       ...
       return true;
   }
   ```
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to