rawlinp commented on a change in pull request #4409: Optimize TR DNSSEC zone
re-signing
URL: https://github.com/apache/trafficcontrol/pull/4409#discussion_r382223394
##########
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:
So, I tried it to split this out, but I wanted to include all of L513-532
since I wanted all of the modification of `newDomainsToZoneKeys` to be done in
the same place. Then I found that the early return on L522 makes splitting this
out trickier because the new method would have to have a `Zone` passed in to be
set as one method result but also return a `boolean` for whether or not an
existing zone was matched and reused.
That said, I think the way the code is currently would be easier to
comprehend than what I just described. What do you think?
----------------------------------------------------------------
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
