zrhoffman commented on code in PR #7732:
URL: https://github.com/apache/trafficcontrol/pull/7732#discussion_r1297816772
##########
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/NameServer.java:
##########
@@ -192,7 +193,7 @@ private static void addAuthority(final Zone zone, final
Message response, final
response.getHeader().setFlag(Flags.AA);
}
- private static void addSOA(final Zone zone, final Message response,
final int section, final int flags) {
+ private static void addSOA(final Zone zone, final Message response,
final int section, final int flags, final long negativeCachingTTL) {
Review Comment:
Looks like the argument given for the `negativeCachingTTL` parameter is
always `getNegativeCachingTTL()`. Why not make `negativeCachingTTL` static and
use it directly instead without the function parameter?
##########
traffic_router/core/src/main/webapp/WEB-INF/applicationContext.xml:
##########
@@ -267,6 +267,7 @@
<bean id="NameServer"
class="org.apache.traffic_control.traffic_router.core.dns.NameServer">
<property name="trafficRouterManager"
ref="trafficRouterManager" />
+ <property name="negativeCachingTTL" value="900" />
Review Comment:
This is on the right track. Since `applicationContext.xml` is not
configurable (not [one of the
files](https://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_router.html#id9)
we list for configuration), this should be a property in one of the
configuration files, maybe `dns.properties`, then referenced in
`applicationContext.xml`.
##########
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/NameServer.java:
##########
@@ -328,11 +329,19 @@ private static RRset setNegativeTTL(final RRset original,
final int flags) {
if (record instanceof SOARecord) {
final SOARecord soa = (SOARecord) record;
+ // Set the "minimum" attribute to be the
maximum of the current minimum value and 900 (seconds)
+ // This is done to increase the negative
caching TTL, so as to maximize the time interval between
+ // successive NXDOMAIN or NXRRSET responses.
+ final long minimum = Math.max(soa.getMinimum(),
negativeCachingTTL);
+ record = new SOARecord(soa.getName(),
DClass.IN, soa.getTTL(), soa.getHost(), soa.getAdmin(),
+ soa.getSerial(),
soa.getRefresh(), soa.getRetry(), soa.getExpire(),
+ minimum);
+
// the value of the minimum field is less than
the actual TTL; adjust
if (soa.getMinimum() != 0 || soa.getTTL() >
soa.getMinimum()) {
record = new SOARecord(soa.getName(),
DClass.IN, soa.getMinimum(), soa.getHost(), soa.getAdmin(),
soa.getSerial(),
soa.getRefresh(), soa.getRetry(), soa.getExpire(),
- soa.getMinimum());
+ minimum);
} // else use the unmodified record
Review Comment:
Instead of potentially setting `record` twice here, use a condition for
`ttl` field and set `record` only once?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
