elsloo closed pull request #1843: Fix steering race condition in TR
URL: https://github.com/apache/incubator-trafficcontrol/pull/1843
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/docs/source/admin/traffic_router.rst
b/docs/source/admin/traffic_router.rst
index 474dc96be7..c1f33c8a40 100644
--- a/docs/source/admin/traffic_router.rst
+++ b/docs/source/admin/traffic_router.rst
@@ -415,6 +415,7 @@ Special regular expressions called Filters can also be
configured for target del
A client can bypass the steering functionality by providing a header called
X-TC-Steering-Option with the xml_id of the target delivery service to route
to. When Traffic Router receives this header it will route to the requested
target delivery service regardless of weight configuration.
Some other points of interest:
+
- Steering is currently only available for HTTP delivery services that are a
part of the same CDN.
- A new role called STEERING has been added to the traffic ops database. Only
users with Admin or Steering privileges can modify steering assignments for a
Delivery Service.
- A new API has been created in Traffic Ops under /internal. This API is used
by a Steering user to add filters and modify assignments. (Filters can only be
added via the API).
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/config/ConfigHandler.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/config/ConfigHandler.java
index c13b4e6e61..d458651d56 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/config/ConfigHandler.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/config/ConfigHandler.java
@@ -207,7 +207,6 @@ public boolean processConfig(final String jsonStr) throws
JsonUtilsException, IO
federationsWatcher.configure(config);
steeringWatcher.configure(config);
- steeringWatcher.setCacheRegister(cacheRegister);
trafficRouterManager.setCacheRegister(cacheRegister);
trafficRouterManager.getTrafficRouter().setRequestHeaders(parseRequestHeaders(config.get("requestHeaders")));
trafficRouterManager.getTrafficRouter().configurationChanged();
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringWatcher.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringWatcher.java
index 66174ce8e1..74b0c4b4d5 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringWatcher.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringWatcher.java
@@ -15,17 +15,12 @@
package com.comcast.cdn.traffic_control.traffic_router.core.ds;
-import com.comcast.cdn.traffic_control.traffic_router.core.cache.CacheRegister;
import
com.comcast.cdn.traffic_control.traffic_router.core.util.AbstractResourceWatcher;
import org.apache.log4j.Logger;
-import java.util.ArrayList;
-import java.util.List;
-
public class SteeringWatcher extends AbstractResourceWatcher {
private static final Logger LOGGER =
Logger.getLogger(SteeringWatcher.class);
private SteeringRegistry steeringRegistry;
- private CacheRegister cacheRegister;
public static final String DEFAULT_STEERING_DATA_URL =
"https://${toHostname}/internal/api/1.2/steering.json";;
@@ -33,26 +28,13 @@ public SteeringWatcher() {
setDatabaseUrl(DEFAULT_STEERING_DATA_URL);
}
- public void setCacheRegister(final CacheRegister cacheRegister) {
- this.cacheRegister = cacheRegister;
- }
-
@Override
public boolean useData(final String data) {
try {
+ // NOTE: it is likely that the steering data will
contain xml_ids for delivery services
+ // that haven't been added to the CRConfig yet. This is
okay because the SteeringRegistry
+ // will only be queried for Delivery Service xml_ids
that exist in CRConfig
steeringRegistry.update(data);
- if (cacheRegister != null) {
- final List<String> invalidOnes = new
ArrayList<String>();
-
- for (final Steering steering :
steeringRegistry.getAll()) {
- if
(cacheRegister.getDeliveryService(steering.getDeliveryService()) == null) {
- LOGGER.warn("Steering data from
" + dataBaseURL + " contains delivery service id reference '" +
steering.getDeliveryService() + "' that's not in cr-config");
-
invalidOnes.add(steering.getDeliveryService());
- }
- }
-
- steeringRegistry.removeAll(invalidOnes);
- }
return true;
} catch (Exception e) {
diff --git
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
index b63e075fb6..224e1edca3 100644
---
a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
+++
b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
@@ -27,6 +27,7 @@
import java.util.Map;
import java.util.Random;
import java.util.Set;
+import java.util.stream.Collectors;
import
com.comcast.cdn.traffic_control.traffic_router.configuration.ConfigurationListener;
import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringTarget;
@@ -772,7 +773,7 @@ private boolean isMultiRouteRequest(final HTTPRequest
request) {
for (final SteeringTarget steeringTarget : steeringTargets) {
final DeliveryService target =
cacheRegister.getDeliveryService(steeringTarget.getDeliveryService());
- if (target != null) {
+ if (target != null) { // target might not be in
CRConfig yet
deliveryServices.add(target);
}
}
@@ -797,10 +798,17 @@ public DeliveryService
consistentHashDeliveryService(final DeliveryService deliv
final String bypassDeliveryServiceId =
steering.getBypassDestination(requestPath);
if (bypassDeliveryServiceId != null &&
!bypassDeliveryServiceId.isEmpty()) {
- return
cacheRegister.getDeliveryService(bypassDeliveryServiceId);
+ final DeliveryService bypass =
cacheRegister.getDeliveryService(bypassDeliveryServiceId);
+ if (bypass != null) { // bypass DS target might not be
in CRConfig yet. Until then, try existing targets
+ return bypass;
+ }
}
- final SteeringTarget steeringTarget =
consistentHasher.selectHashable(steering.getTargets(),
deliveryService.getDispersion(), requestPath);
+ // only select from targets in CRConfig
+ final List<SteeringTarget> availableTargets =
steering.getTargets().stream()
+ .filter(target ->
cacheRegister.getDeliveryService(target.getDeliveryService()) != null)
+ .collect(Collectors.toList());
+ final SteeringTarget steeringTarget =
consistentHasher.selectHashable(availableTargets,
deliveryService.getDispersion(), requestPath);
return
cacheRegister.getDeliveryService(steeringTarget.getDeliveryService());
}
----------------------------------------------------------------
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
