rawlinp commented on a change in pull request #1908: Changes for Backup Edge 
Cache Group
URL: 
https://github.com/apache/incubator-trafficcontrol/pull/1908#discussion_r172619956
 
 

 ##########
 File path: 
traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
 ##########
 @@ -675,17 +678,40 @@ public CacheLocation getCoverageZoneCacheLocation(final 
String ip, final String
                        return cacheLocation;
                }
 
-               // We had a hit in the CZF but the name does not match a known 
cache location.
-               // Check whether the CZF entry has a geolocation and use it if 
so.
-               return 
getClosestCacheLocation(cacheRegister.filterAvailableLocations(deliveryServiceId),
 networkNode.getGeolocation(), 
cacheRegister.getDeliveryService(deliveryServiceId));
+               if (networkNode.getBackupCacheGroups() != null) {
+                       for (final String cacheGroup : 
networkNode.getBackupCacheGroups()) {
+                               final CacheLocation bkCacheLocation = 
getCacheRegister().getCacheLocationById(cacheGroup);
+                               if (bkCacheLocation != null && 
!getSupportingCaches(bkCacheLocation.getCaches(), deliveryService).isEmpty()) {
+                                       LOGGER.debug("Got backup CZ cache group 
" + bkCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId);
+                                       if (track != null) {
+                                               
track.setFromBackupCzGroup(true);
+                                       }
+                                       return bkCacheLocation;
+                               }
+                       }
+               } 
+
+               if (networkNode.isUseClosest()) {
+                       // We had a hit in the CZF but the name does not match 
a known cache location.
+                       // Check whether the CZF entry has a geolocation and 
use it if so.
+                       final CacheLocation closestCacheLocation = 
getClosestCacheLocation(cacheRegister.filterAvailableLocations(deliveryServiceId),
 networkNode.getGeolocation(), 
cacheRegister.getDeliveryService(deliveryServiceId));
+                       if (closestCacheLocation != null) {
+                               LOGGER.debug("Got closest CZ cache group " + 
closestCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId);
+                               if (track != null) {
+                                       track.setFromBackupCzGroup(true);
+                               }
+                       }
+                       return closestCacheLocation;
+               }
+               return null;
 
 Review comment:
   I agree with that, but the scenario I'm concerned about is when 
`fallbackToNextClosest == false`. In that case I don't think the request should 
fallback to Geo when the client was found in the CZF but the matched cachegroup 
has no caches available. In the best-case geo-lookup that client would still be 
traversing the same network path to the next available cachegroup (which is 
what the backup list is supposed to prevent).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to