You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by el...@apache.org on 2018/06/14 17:31:27 UTC
[trafficcontrol] branch master updated (55d6eff -> 0a37234)
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git.
from 55d6eff Remove deliveryservice.org_server_fqdn column/compute it from Origin table
new a625fda Implement geo-sorting of CLIENT_STEERING targets
new ea97ee6 Implement Traffic Router unit tests for geo-steering
new 291f356 Augment Steering API to allow CRUD of geo-targets
new a792934 Populate coordinates in the steering API results
new 0a37234 Validate that STEERING_GEO targets are only used in CLIENT_STEERING DSes
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
traffic_ops/app/db/seeds.sql | 2 +
.../app/lib/API/DeliveryService/Steering.pm | 53 +++++++-
.../app/lib/API/DeliveryService/SteeringTarget.pm | 24 ++--
traffic_ops/app/lib/UI/Steering.pm | 8 +-
.../traffic_router/core/cache/Cache.java | 13 +-
.../traffic_router/core/config/ConfigHandler.java | 2 +-
.../core/ds/SteeringGeolocationComparator.java | 75 ++++++++++
.../traffic_router/core/ds/SteeringRegistry.java | 11 +-
.../traffic_router/core/ds/SteeringResult.java | 54 ++++++++
.../traffic_router/core/ds/SteeringTarget.java | 57 ++++++++
.../traffic_router/core/router/TrafficRouter.java | 121 ++++++++++++-----
.../core/ds/SteeringGeolocationComparatorTest.java | 151 +++++++++++++++++++++
.../core/router/GeoSortSteeringResultsTest.java | 134 ++++++++++++++++++
13 files changed, 651 insertions(+), 54 deletions(-)
create mode 100644 traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java
create mode 100644 traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringResult.java
create mode 100644 traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparatorTest.java
create mode 100644 traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/GeoSortSteeringResultsTest.java
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.
[trafficcontrol] 03/05: Augment Steering API to allow CRUD of
geo-targets
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 291f3567dd800b552dd70df60225b592bd29d70e
Author: Rawlin Peters <ra...@comcast.com>
AuthorDate: Tue Apr 3 16:37:32 2018 -0600
Augment Steering API to allow CRUD of geo-targets
Add two new steering_target types:
- STEERING_GEO_ORDER
- STEERING_GEO_WEIGHT
---
traffic_ops/app/db/seeds.sql | 2 ++
.../app/lib/API/DeliveryService/Steering.pm | 35 +++++++++++++++-------
traffic_ops/app/lib/UI/Steering.pm | 9 ++++--
3 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/traffic_ops/app/db/seeds.sql b/traffic_ops/app/db/seeds.sql
index 6c11adb..5928970 100644
--- a/traffic_ops/app/db/seeds.sql
+++ b/traffic_ops/app/db/seeds.sql
@@ -400,6 +400,8 @@ insert into type (name, description, use_in_table) values ('TXT_RECORD', 'Static
--steering_target types
insert into type (name, description, use_in_table) values ('STEERING_WEIGHT', 'Weighted steering target', 'steering_target') ON CONFLICT (name) DO NOTHING;
insert into type (name, description, use_in_table) values ('STEERING_ORDER', 'Ordered steering target', 'steering_target') ON CONFLICT (name) DO NOTHING;
+insert into type (name, description, use_in_table) values ('STEERING_GEO_ORDER', 'Geo-ordered steering target', 'steering_target') ON CONFLICT (name) DO NOTHING;
+insert into type (name, description, use_in_table) values ('STEERING_GEO_WEIGHT', 'Geo-weighted steering target', 'steering_target') ON CONFLICT (name) DO NOTHING;
-- users
insert into tm_user (username, role, full_name, token) values ('extension', (select id from role where name = 'operations'), 'Extension User, DO NOT DELETE', '91504CE6-8E4A-46B2-9F9F-FE7C15228498') ON CONFLICT DO NOTHING;
diff --git a/traffic_ops/app/lib/API/DeliveryService/Steering.pm b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
index 6ce009a..002a95a 100644
--- a/traffic_ops/app/lib/API/DeliveryService/Steering.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
@@ -43,12 +43,10 @@ sub find_steering {
my %steering;
- my $rs_data = $self->db->resultset('SteeringView')->search({}, {order_by => ['steering_xml_id', 'target_xml_id']});
+ my %criteria = length $steering_xml_id ? (steering_xml_id => $steering_xml_id) : ();
+ my $rs_data = $self->db->resultset('SteeringView')->search(\%criteria, {order_by => ['steering_xml_id', 'target_xml_id']});
while ( my $row = $rs_data->next ) {
- if ($steering_xml_id && $row->steering_xml_id ne $steering_xml_id) { # TODO: can this be optimized into the SQL query?
- next;
- }
if (!&is_admin($self)) {
my $name = $self->current_user()->{username};
@@ -92,9 +90,6 @@ sub find_steering {
my $targets = $steering_entry->{"targets"};
- # TODO: add new STEERING_GEO_WEIGHT and STEERING_GEO_ORDER types, handle them here
- # verify if STEERING_ORDER and STEERING_WEIGHT should omit latitude, longitude, and geoOrder
- # if GEO, get the target DS's lat/long, add it to result (add it to SteeringView?)
if ( $row->type eq "STEERING_ORDER" ) {
push(@{$targets},{
'deliveryService' => $row->target_xml_id,
@@ -102,10 +97,30 @@ sub find_steering {
'weight' => 0
});
}
- else {
+ elsif ( $row->type eq "STEERING_WEIGHT" ) {
+ push(@{$targets},{
+ 'deliveryService' => $row->target_xml_id,
+ 'order' => 0,
+ 'weight' => $row->value
+ });
+ }
+ elsif ( $row->type eq "STEERING_GEO_ORDER" ) {
+ push(@{$targets},{
+ 'deliveryService' => $row->target_xml_id,
+ 'order' => 0,
+ 'geoOrder' => $row->value,
+ 'latitude' => 0, # TODO: fill these in w/ the DeliveryService Origin lat/lon
+ 'longitude' => 0,
+ 'weight' => 0
+ });
+ }
+ elsif ( $row->type eq "STEERING_GEO_WEIGHT" ) {
push(@{$targets},{
'deliveryService' => $row->target_xml_id,
'order' => 0,
+ 'geoOrder' => 0,
+ 'latitude' => 0, # TODO: fill these in w/ the DeliveryService Origin lat/lon
+ 'longitude' => 0,
'weight' => $row->value
});
}
@@ -151,6 +166,8 @@ sub get_type {
return $type;
}
+# NOTE: STEERING_GEO* types are deliberately ignored in the following endpoint b/c it's soon to be deprecated (use
+# the non-internal PUT endpoint instead)
sub update() {
my $self = shift;
@@ -194,7 +211,6 @@ sub update() {
my $req_targets = $self->req->json->{'targets'};
foreach my $req_target (@{$req_targets}) {
- # TODO: update this validation to handle new GEO types
if (!$req_target->{'deliveryService'} && ( !$req_target->{'weight'} || !$req_target->{'order'} ) || ( $req_target->{'weight'} && $req_target->{'order'} ) ) {
return $self->render(json => {"message" => "please provide a valid json for targets"}, status => 400);
}
@@ -222,7 +238,6 @@ sub update() {
foreach my $req_target (@{$req_targets}) {
my $target_id = $valid_targets->{$req_target->{'deliveryService'}};
- # TODO: update this to handle GEO-type targets
if ($req_target->{'weight'}) {
my $steering_target_row = $self->db->resultset('SteeringTarget')->find({ deliveryservice => $steering_id, target => $target_id});
$steering_target_row->value($req_target->{weight});
diff --git a/traffic_ops/app/lib/UI/Steering.pm b/traffic_ops/app/lib/UI/Steering.pm
index 1e7f98b..a69b375 100644
--- a/traffic_ops/app/lib/UI/Steering.pm
+++ b/traffic_ops/app/lib/UI/Steering.pm
@@ -83,6 +83,7 @@ sub get_target_data {
my @weight_steering;
my @pos_order_steering;
my @neg_order_steering;
+ my @geo_steering;
my $target_rs = $self->db->resultset('SteeringTarget')->search( { deliveryservice => $ds_id } );
@@ -101,6 +102,9 @@ sub get_target_data {
elsif ( $row->type->name eq "STEERING_ORDER" && $row->value >= 0 ) {
push (@pos_order_steering, $t);
}
+ elsif ( $row->type->name =~ m/^STEERING_GEO/ ) {
+ push (@geo_steering, $t);
+ }
else { push (@weight_steering, $t); }
$i++;
}
@@ -108,9 +112,10 @@ sub get_target_data {
@weight_steering = sort { $b->{target_value} <=> $a->{target_value} } @weight_steering;
@neg_order_steering = sort { $a->{target_value} <=> $b->{target_value} } @neg_order_steering;
@pos_order_steering = sort { $a->{target_value} <=> $b->{target_value} } @pos_order_steering;
+ # TODO: group geo_steering targets by Origin lat/lon
- #push everything into an a single array - negative order values first, weights second, positive order last.
- push (@steering, @neg_order_steering, @weight_steering, @pos_order_steering);
+ #push everything into an a single array - negative order values 1st, geo 2nd, weights 3rd, positive order last.
+ push (@steering, @neg_order_steering, @geo_steering, @weight_steering, @pos_order_steering);
}
return @steering;
}
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.
[trafficcontrol] 02/05: Implement Traffic Router unit tests for
geo-steering
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit ea97ee6202d9d02ee51d87dbbbb5475c6f4b8a5e
Author: Rawlin Peters <ra...@comcast.com>
AuthorDate: Tue Mar 20 16:52:00 2018 -0600
Implement Traffic Router unit tests for geo-steering
---
.../core/ds/SteeringGeolocationComparator.java | 4 +-
.../traffic_router/core/ds/SteeringRegistry.java | 1 +
.../traffic_router/core/ds/SteeringTarget.java | 4 +
.../traffic_router/core/router/TrafficRouter.java | 8 +-
.../core/ds/SteeringGeolocationComparatorTest.java | 151 +++++++++++++++++++++
.../core/router/GeoSortSteeringResultsTest.java | 134 ++++++++++++++++++
6 files changed, 296 insertions(+), 6 deletions(-)
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java
index 5436dae..2dbe8d1 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java
@@ -19,7 +19,6 @@ import java.util.Comparator;
import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
-// TODO: add unit tests for this class
public class SteeringGeolocationComparator implements Comparator<SteeringResult> {
private final Geolocation clientLocation;
@@ -29,6 +28,7 @@ public class SteeringGeolocationComparator implements Comparator<SteeringResult>
}
@Override
+ @SuppressWarnings({"PMD.CyclomaticComplexity"})
public int compare(final SteeringResult result1, final SteeringResult result2) {
final Geolocation originGeo1 = result1.getSteeringTarget().getGeolocation();
final Geolocation originGeo2 = result2.getSteeringTarget().getGeolocation();
@@ -63,7 +63,7 @@ public class SteeringGeolocationComparator implements Comparator<SteeringResult>
// different cache and origin locations, prefer shortest total distance
if (totalDistance1 != totalDistance2) {
- // TODO: if the difference is smaller than a certain threshold, still prefer the closer edge even though distance is greater?
+ // TODO: if the difference is smaller than a certain threshold/ratio, still prefer the closer edge even though distance is greater?
return Double.compare(totalDistance1, totalDistance2);
}
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java
index 2bf197b..0b9437a 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java
@@ -33,6 +33,7 @@ public class SteeringRegistry {
private final Map<String, Steering> registry = new HashMap<String, Steering>();
private final ObjectMapper objectMapper = new ObjectMapper(new JsonFactory());
+ @SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.AvoidDuplicateLiterals"})
public void update(final String json) {
Map<String, List<Steering>> m;
try {
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
index d1a38df..c054ccd 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
@@ -102,6 +102,10 @@ public class SteeringTarget extends DefaultHashable {
return geolocation;
}
+ public void setGeolocation(final Geolocation geolocation) {
+ this.geolocation = geolocation;
+ }
+
@Override
@SuppressWarnings("PMD")
public boolean equals(Object o) {
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 5f49865..c67455c 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
@@ -617,6 +617,7 @@ public class TrafficRouter {
return routeResult;
}
+ @SuppressWarnings({"PMD.NPathComplexity"})
private List<SteeringResult> getSteeringResults(final HTTPRequest request, final Track track, final DeliveryService entryDeliveryService) {
if (isTlsMismatch(request, entryDeliveryService)) {
@@ -633,7 +634,7 @@ public class TrafficRouter {
return null;
}
- List<SteeringResult> toBeRemoved = new ArrayList<>();
+ final List<SteeringResult> toBeRemoved = new ArrayList<>();
for (final SteeringResult steeringResult : steeringResults) {
final DeliveryService ds = steeringResult.getDeliveryService();
if (isTlsMismatch(request, ds)) {
@@ -851,7 +852,7 @@ public class TrafficRouter {
return steeringRegistry.get(deliveryService.getId()).isClientSteering();
}
- private Geolocation getClientLocationByCoverageZoneOrGeo(final String clientIP, final DeliveryService deliveryService) {
+ protected Geolocation getClientLocationByCoverageZoneOrGeo(final String clientIP, final DeliveryService deliveryService) {
Geolocation clientLocation;
final NetworkNode networkNode = getNetworkNode(clientIP);
if (networkNode != null && networkNode.getGeolocation() != null) {
@@ -866,8 +867,7 @@ public class TrafficRouter {
return deliveryService.supportLocation(clientLocation);
}
- // TODO: add unit test for this method
- private void geoSortSteeringResults(final List<SteeringResult> steeringResults, final String clientIP, final DeliveryService deliveryService) {
+ protected void geoSortSteeringResults(final List<SteeringResult> steeringResults, final String clientIP, final DeliveryService deliveryService) {
if (clientIP == null || clientIP.isEmpty()
|| steeringResults.stream().allMatch(t -> t.getSteeringTarget().getGeolocation() == null)) {
return;
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparatorTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparatorTest.java
new file mode 100644
index 0000000..ed72081
--- /dev/null
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparatorTest.java
@@ -0,0 +1,151 @@
+/*
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.comcast.cdn.traffic_control.traffic_router.core.ds;
+
+import com.comcast.cdn.traffic_control.traffic_router.core.cache.Cache;
+import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class SteeringGeolocationComparatorTest {
+ /*
+ This test class assumes some knowledge of United States geography. For reference,
+ here are the rough distances looking at a map from left to right:
+
+ Seattle <--- 1300 mi ---> Denver <--- 2000 mi ---> Boston
+
+ */
+
+ private Geolocation seattleGeolocation;
+ private Geolocation denverGeolocation;
+ private Geolocation bostonGeolocation;
+
+ private Cache seattleCache;
+ private Cache denverCache;
+ private Cache bostonCache;
+
+ private SteeringTarget seattleTarget;
+ private SteeringTarget seattleTarget2;
+ private SteeringTarget denverTarget;
+ private SteeringTarget bostonTarget;
+
+ private SteeringResult seattleResult;
+ private SteeringResult seattleResult2;
+ private SteeringResult denverResult;
+ private SteeringResult bostonResult;
+
+ private SteeringGeolocationComparator seattleComparator;
+ private SteeringGeolocationComparator denverComparator;
+ private SteeringGeolocationComparator bostonComparator;
+
+ @Before
+ public void before() {
+ seattleGeolocation = new Geolocation(47.0, -122.0);
+ denverGeolocation = new Geolocation(39.0, -104.0);
+ bostonGeolocation = new Geolocation(42.0, -71.0);
+
+ seattleCache = new Cache("seattle-id", "seattle-hash-id", 1, seattleGeolocation);
+ denverCache = new Cache("denver-id", "denver-hash-id", 1, denverGeolocation);
+ bostonCache = new Cache("boston-id", "boston-hash-id", 1, bostonGeolocation);
+
+ seattleTarget = new SteeringTarget();
+ seattleTarget.setGeolocation(seattleGeolocation);
+ seattleResult = new SteeringResult(seattleTarget, null);
+ seattleResult.setCache(seattleCache);
+
+ seattleTarget2 = new SteeringTarget();
+ seattleTarget2.setGeolocation(seattleGeolocation);
+ seattleResult2 = new SteeringResult(seattleTarget2, null);
+ seattleResult2.setCache(seattleCache);
+
+ denverTarget = new SteeringTarget();
+ denverTarget.setGeolocation(denverGeolocation);
+ denverResult = new SteeringResult(denverTarget, null);
+ denverResult.setCache(denverCache);
+
+ bostonTarget = new SteeringTarget();
+ bostonTarget.setGeolocation(bostonGeolocation);
+ bostonResult = new SteeringResult(bostonTarget, null);
+ bostonResult.setCache(bostonCache);
+
+ seattleComparator = new SteeringGeolocationComparator(seattleGeolocation);
+ denverComparator = new SteeringGeolocationComparator(denverGeolocation);
+ bostonComparator = new SteeringGeolocationComparator(bostonGeolocation);
+
+ }
+
+ @Test
+ public void testLeftNullOriginGeo() {
+ seattleResult.getSteeringTarget().setGeolocation(null);
+ assertEquals(1, seattleComparator.compare(seattleResult, bostonResult));
+ }
+
+ @Test
+ public void testRightNullOriginGeo() {
+ denverResult.getSteeringTarget().setGeolocation(null);
+ assertEquals(-1, seattleComparator.compare(seattleResult, denverResult));
+ }
+
+ @Test
+ public void testBothNullOriginGeo() {
+ seattleResult.getSteeringTarget().setGeolocation(null);
+ denverResult.getSteeringTarget().setGeolocation(null);
+ assertEquals(0, seattleComparator.compare(seattleResult, denverResult));
+ }
+
+ @Test
+ public void testSameCacheAndOriginGeo() {
+ assertEquals(0, seattleComparator.compare(seattleResult, seattleResult));
+ }
+
+ @Test
+ public void testSameCacheAndOriginGeoWithGeoOrder() {
+ seattleTarget.setGeoOrder(1);
+ seattleTarget2.setGeoOrder(2);
+ assertEquals(-1, seattleComparator.compare(seattleResult, seattleResult2));
+ assertEquals(1, seattleComparator.compare(seattleResult2, seattleResult));
+ seattleTarget.setGeoOrder(2);
+ assertEquals(0, seattleComparator.compare(seattleResult, seattleResult2));
+ }
+
+ @Test
+ public void testDifferentCacheAndOriginGeo() {
+ assertEquals(-1, seattleComparator.compare(seattleResult, denverResult));
+ assertEquals(-1, seattleComparator.compare(denverResult, bostonResult));
+ assertEquals(1, seattleComparator.compare(bostonResult, seattleResult));
+ }
+
+ @Test
+ public void testCacheGeoDifferentFromOriginGeo() {
+ seattleResult.setCache(denverCache);
+ // seattle -> denver -> seattle || seattle -> denver
+ assertEquals(1, seattleComparator.compare(seattleResult, denverResult));
+ // denver -> seattle || denver
+ assertEquals(1, denverComparator.compare(seattleResult, denverResult));
+ // boston -> denver -> seattle || boston -> denver
+ assertEquals(1, bostonComparator.compare(seattleResult, denverResult));
+ // seattle -> denver -> seattle || seattle -> boston
+ assertEquals(-1, seattleComparator.compare(seattleResult, bostonResult));
+ seattleResult.setCache(bostonCache);
+ bostonResult.setCache(denverCache);
+ // seattle -> boston -> seattle || seattle -> denver -> boston
+ assertEquals(1, seattleComparator.compare(seattleResult, bostonResult));
+ }
+
+}
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/GeoSortSteeringResultsTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/GeoSortSteeringResultsTest.java
new file mode 100644
index 0000000..81d3ed0
--- /dev/null
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/GeoSortSteeringResultsTest.java
@@ -0,0 +1,134 @@
+/*
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.comcast.cdn.traffic_control.traffic_router.core.router;
+
+import com.comcast.cdn.traffic_control.traffic_router.core.cache.Cache;
+import com.comcast.cdn.traffic_control.traffic_router.core.ds.DeliveryService;
+import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringResult;
+import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringTarget;
+import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyListOf;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({Collections.class})
+public class GeoSortSteeringResultsTest {
+
+ private TrafficRouter trafficRouter;
+ private List<SteeringResult> steeringResults;
+ private Geolocation clientLocation;
+ private DeliveryService deliveryService;
+
+ @Before
+ public void before() {
+ trafficRouter = mock(TrafficRouter.class);
+ steeringResults = new ArrayList<>();
+ clientLocation = new Geolocation(47.0, -122.0);
+ deliveryService = mock(DeliveryService.class);
+ doCallRealMethod().when(trafficRouter).geoSortSteeringResults(anyListOf(SteeringResult.class), anyString(), any(DeliveryService.class));
+ when(trafficRouter.getClientLocationByCoverageZoneOrGeo(anyString(), any(DeliveryService.class))).thenReturn(clientLocation);
+ }
+
+ @Test
+ public void testNullClientIP() {
+ trafficRouter.geoSortSteeringResults(steeringResults, null, deliveryService);
+ verify(trafficRouter, never()).getClientLocationByCoverageZoneOrGeo(null, deliveryService);
+ }
+
+ @Test
+ public void testEmptyClientIP() {
+ trafficRouter.geoSortSteeringResults(steeringResults, "", deliveryService);
+ verify(trafficRouter, never()).getClientLocationByCoverageZoneOrGeo("", deliveryService);
+ }
+
+ @Test
+ public void testNoSteeringTargetsHaveGeolocations() {
+ steeringResults.add(new SteeringResult(new SteeringTarget(), deliveryService));
+ trafficRouter.geoSortSteeringResults(steeringResults, "::1", deliveryService);
+ verify(trafficRouter, never()).getClientLocationByCoverageZoneOrGeo("::1", deliveryService);
+ }
+
+ @Test
+ public void testClientGeolocationIsNull() {
+ SteeringTarget steeringTarget = new SteeringTarget();
+ steeringTarget.setGeolocation(clientLocation);
+ steeringResults.add(new SteeringResult(steeringTarget, deliveryService));
+ clientLocation = null;
+ PowerMockito.mockStatic(Collections.class);
+
+ trafficRouter.geoSortSteeringResults(steeringResults, "::1", deliveryService);
+
+ PowerMockito.verifyStatic(never());
+ }
+
+ @Test
+ public void testGeoSortingMixedWithNonGeoTargets() {
+ Cache cache = new Cache("fake-id", "fake-hash-id",1, clientLocation);
+ SteeringTarget target;
+
+ target = new SteeringTarget();
+ target.setOrder(-1);
+ SteeringResult resultNoGeoNegativeOrder = new SteeringResult(target, deliveryService);
+ resultNoGeoNegativeOrder.setCache(cache);
+ steeringResults.add(resultNoGeoNegativeOrder);
+
+ target = new SteeringTarget();
+ target.setOrder(1);
+ SteeringResult resultNoGeoPositiveOrder = new SteeringResult(target, deliveryService);
+ resultNoGeoPositiveOrder.setCache(cache);
+ steeringResults.add(resultNoGeoPositiveOrder);
+
+ target = new SteeringTarget();
+ target.setOrder(0);
+ SteeringResult resultNoGeoZeroOrder = new SteeringResult(target, deliveryService);
+ resultNoGeoZeroOrder.setCache(cache);
+ steeringResults.add(resultNoGeoZeroOrder);
+
+ target = new SteeringTarget();
+ target.setGeolocation(clientLocation);
+ target.setOrder(0);
+ SteeringResult resultGeo = new SteeringResult(target, deliveryService);
+ resultGeo.setCache(cache);
+ steeringResults.add(resultGeo);
+
+ trafficRouter.geoSortSteeringResults(steeringResults, "::1", deliveryService);
+
+ assertEquals(resultNoGeoNegativeOrder, steeringResults.get(0));
+ assertEquals(resultGeo, steeringResults.get(1));
+ assertEquals(resultNoGeoZeroOrder, steeringResults.get(2));
+ assertEquals(resultNoGeoPositiveOrder, steeringResults.get(3));
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.
[trafficcontrol] 04/05: Populate coordinates in the steering API
results
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit a79293473944f26793f584bf1c6e76d933c8e694
Author: Rawlin Peters <ra...@comcast.com>
AuthorDate: Wed May 23 15:48:31 2018 -0600
Populate coordinates in the steering API results
---
.../app/lib/API/DeliveryService/Steering.pm | 31 +++++++++++++++++++---
.../app/lib/API/DeliveryService/SteeringTarget.pm | 16 ++++++-----
traffic_ops/app/lib/UI/Steering.pm | 1 -
3 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/traffic_ops/app/lib/API/DeliveryService/Steering.pm b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
index 002a95a..3c79615 100644
--- a/traffic_ops/app/lib/API/DeliveryService/Steering.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
@@ -105,22 +105,24 @@ sub find_steering {
});
}
elsif ( $row->type eq "STEERING_GEO_ORDER" ) {
+ my $coords = get_primary_origin_coordinates($self, $row->target_id);
push(@{$targets},{
'deliveryService' => $row->target_xml_id,
'order' => 0,
'geoOrder' => $row->value,
- 'latitude' => 0, # TODO: fill these in w/ the DeliveryService Origin lat/lon
- 'longitude' => 0,
+ 'latitude' => $coords->{lat},
+ 'longitude' => $coords->{lon},
'weight' => 0
});
}
elsif ( $row->type eq "STEERING_GEO_WEIGHT" ) {
+ my $coords = get_primary_origin_coordinates($self, $row->target_id);
push(@{$targets},{
'deliveryService' => $row->target_xml_id,
'order' => 0,
'geoOrder' => 0,
- 'latitude' => 0, # TODO: fill these in w/ the DeliveryService Origin lat/lon
- 'longitude' => 0,
+ 'latitude' => $coords->{lat},
+ 'longitude' => $coords->{lon},
'weight' => $row->value
});
}
@@ -145,6 +147,27 @@ sub find_steering {
return $response;
}
+sub get_primary_origin_coordinates {
+ my $self = shift;
+ my $ds_id = shift;
+
+ my %coordinates = (lat => 0.0, lon => 0.0);
+
+ my $origin_rs = $self->db->resultset('Origin')->find(
+ { deliveryservice => $ds_id, is_primary => 1 },
+ { prefetch => 'coordinate' });
+
+ if ( !defined($origin_rs) || !defined($origin_rs->coordinate) ) {
+ return \%coordinates;
+ }
+
+ $coordinates{lat} = $origin_rs->coordinate->latitude;
+ $coordinates{lon} = $origin_rs->coordinate->longitude;
+
+ return \%coordinates;
+}
+
+
sub get_ds_id {
my $self = shift;
my $xml_id = shift;
diff --git a/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm b/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
index 88bdb15..8e0c5f3 100644
--- a/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
@@ -308,15 +308,17 @@ sub is_target_valid {
# Validate the input against the rules
my $result = validate( $params, $rules );
- # TODO: fix indentation below, validate GEO-types
- if ( $result->{success} ) {
- if ( ( $target_type eq "STEERING_WEIGHT" ) and ( $params->{value} < 0 ) ) {
- return ( 0, "Invalid value for target type STEERING_WEIGHT: can not be negative" );
- }
- return ( 1, $result->{data} );
+ if ($result->{success}) {
+ if (($target_type eq "STEERING_WEIGHT") and ($params->{value} < 0)) {
+ return(0, "Invalid value for target type STEERING_WEIGHT: cannot be negative");
+ }
+ elsif (($target_type eq "STEERING_GEO_WEIGHT") and ($params->{value} < 0)) {
+ return(0, "Invalid value for target type STEERING_GEO_WEIGHT: cannot be negative");
+ }
+ return(1, $result->{data});
}
else {
- return ( 0, $result->{error} );
+ return(0, $result->{error});
}
}
diff --git a/traffic_ops/app/lib/UI/Steering.pm b/traffic_ops/app/lib/UI/Steering.pm
index a69b375..4bfc87d 100644
--- a/traffic_ops/app/lib/UI/Steering.pm
+++ b/traffic_ops/app/lib/UI/Steering.pm
@@ -112,7 +112,6 @@ sub get_target_data {
@weight_steering = sort { $b->{target_value} <=> $a->{target_value} } @weight_steering;
@neg_order_steering = sort { $a->{target_value} <=> $b->{target_value} } @neg_order_steering;
@pos_order_steering = sort { $a->{target_value} <=> $b->{target_value} } @pos_order_steering;
- # TODO: group geo_steering targets by Origin lat/lon
#push everything into an a single array - negative order values 1st, geo 2nd, weights 3rd, positive order last.
push (@steering, @neg_order_steering, @geo_steering, @weight_steering, @pos_order_steering);
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.
[trafficcontrol] 05/05: Validate that STEERING_GEO targets are only
used in CLIENT_STEERING DSes
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 0a3723485abfc29070ebd7759ecd05024fbc3301
Author: Rawlin Peters <ra...@comcast.com>
AuthorDate: Thu May 24 16:19:33 2018 -0600
Validate that STEERING_GEO targets are only used in CLIENT_STEERING DSes
Also, ensure that the coordinates in the json output are floats not
strings.
---
traffic_ops/app/lib/API/DeliveryService/Steering.pm | 4 ++--
traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm | 9 +++++++--
.../traffic_control/traffic_router/core/ds/SteeringTarget.java | 1 -
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/traffic_ops/app/lib/API/DeliveryService/Steering.pm b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
index 3c79615..e16ae5d 100644
--- a/traffic_ops/app/lib/API/DeliveryService/Steering.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
@@ -161,8 +161,8 @@ sub get_primary_origin_coordinates {
return \%coordinates;
}
- $coordinates{lat} = $origin_rs->coordinate->latitude;
- $coordinates{lon} = $origin_rs->coordinate->longitude;
+ $coordinates{lat} = $origin_rs->coordinate->latitude + 0.0;
+ $coordinates{lon} = $origin_rs->coordinate->longitude + 0.0;
return \%coordinates;
}
diff --git a/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm b/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
index 8e0c5f3..dad82e8 100644
--- a/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
@@ -126,7 +126,7 @@ sub update {
}
$params->{targetId} = $target_ds_id; # to ensure that is_valid passes
- my ( $is_valid, $result ) = $self->is_target_valid($params);
+ my ( $is_valid, $result ) = $self->is_target_valid($params, $ds);
if ( !$is_valid ) {
return $self->alert($result);
@@ -196,7 +196,7 @@ sub create {
return $self->alert("Steering target delivery-service tenant is not available to the user.");
}
- my ( $is_valid, $result ) = $self->is_target_valid($params);
+ my ( $is_valid, $result ) = $self->is_target_valid($params, $ds);
if ( !$is_valid ) {
return $self->alert($result);
@@ -289,12 +289,17 @@ sub delete {
sub is_target_valid {
my $self = shift;
my $params = shift;
+ my $steering_ds = shift;
my ( $is_valid, $target_type ) = $self->is_valid_target_type( $params->{typeId} );
if ( !$is_valid ) {
return ( 0, "Invalid target type" );
}
+ if ( $steering_ds->type->name ne "CLIENT_STEERING" && ($target_type eq "STEERING_GEO_WEIGHT" || $target_type eq "STEERING_GEO_ORDER") ) {
+ return(0, "Invalid target type: STEERING_GEO_WEIGHT and STEERING_GEO_ORDER are only supported in CLIENT_STEERING delivery services");
+ }
+
my $rules = {
fields => [qw/value typeId/],
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
index c054ccd..db1c015 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
@@ -32,7 +32,6 @@ public class SteeringTarget extends DefaultHashable {
private int order = 0;
@JsonProperty
private int geoOrder = 0;
- // TODO: lat/long should probably be moved to the DeliveryService itself, but this is ok for a start
@JsonProperty
private double latitude = DEFAULT_LAT;
@JsonProperty
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.
[trafficcontrol] 01/05: Implement geo-sorting of CLIENT_STEERING
targets
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elsloo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit a625fda763897b5dbf4b56b44845790ca5b8ffe2
Author: Rawlin Peters <ra...@comcast.com>
AuthorDate: Tue Feb 20 18:01:22 2018 -0700
Implement geo-sorting of CLIENT_STEERING targets
For GEO targets in a CLIENT_STEERING DS, sort the resulting list from
most optimal to least optimal, where "optimal" means shortest distance
from client -> edge -> origin.
If there is a mix of STEERING_WEIGHT, STEERING_ORDER, and GEO targets,
the ordering would be:
1. negative STEERING_ORDER
2. GEO
3. STEERING_WEIGHT
4. positive STEERING_ORDER
---
.../app/lib/API/DeliveryService/Steering.pm | 7 +-
.../app/lib/API/DeliveryService/SteeringTarget.pm | 1 +
.../traffic_router/core/cache/Cache.java | 13 ++-
.../traffic_router/core/config/ConfigHandler.java | 2 +-
.../core/ds/SteeringGeolocationComparator.java | 75 +++++++++++++
.../traffic_router/core/ds/SteeringRegistry.java | 10 +-
.../traffic_router/core/ds/SteeringResult.java | 54 +++++++++
.../traffic_router/core/ds/SteeringTarget.java | 54 +++++++++
.../traffic_router/core/router/TrafficRouter.java | 121 +++++++++++++++------
9 files changed, 297 insertions(+), 40 deletions(-)
diff --git a/traffic_ops/app/lib/API/DeliveryService/Steering.pm b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
index e036b5d..6ce009a 100644
--- a/traffic_ops/app/lib/API/DeliveryService/Steering.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/Steering.pm
@@ -46,7 +46,7 @@ sub find_steering {
my $rs_data = $self->db->resultset('SteeringView')->search({}, {order_by => ['steering_xml_id', 'target_xml_id']});
while ( my $row = $rs_data->next ) {
- if ($steering_xml_id && $row->steering_xml_id ne $steering_xml_id) {
+ if ($steering_xml_id && $row->steering_xml_id ne $steering_xml_id) { # TODO: can this be optimized into the SQL query?
next;
}
@@ -92,6 +92,9 @@ sub find_steering {
my $targets = $steering_entry->{"targets"};
+ # TODO: add new STEERING_GEO_WEIGHT and STEERING_GEO_ORDER types, handle them here
+ # verify if STEERING_ORDER and STEERING_WEIGHT should omit latitude, longitude, and geoOrder
+ # if GEO, get the target DS's lat/long, add it to result (add it to SteeringView?)
if ( $row->type eq "STEERING_ORDER" ) {
push(@{$targets},{
'deliveryService' => $row->target_xml_id,
@@ -191,6 +194,7 @@ sub update() {
my $req_targets = $self->req->json->{'targets'};
foreach my $req_target (@{$req_targets}) {
+ # TODO: update this validation to handle new GEO types
if (!$req_target->{'deliveryService'} && ( !$req_target->{'weight'} || !$req_target->{'order'} ) || ( $req_target->{'weight'} && $req_target->{'order'} ) ) {
return $self->render(json => {"message" => "please provide a valid json for targets"}, status => 400);
}
@@ -218,6 +222,7 @@ sub update() {
foreach my $req_target (@{$req_targets}) {
my $target_id = $valid_targets->{$req_target->{'deliveryService'}};
+ # TODO: update this to handle GEO-type targets
if ($req_target->{'weight'}) {
my $steering_target_row = $self->db->resultset('SteeringTarget')->find({ deliveryservice => $steering_id, target => $target_id});
$steering_target_row->value($req_target->{weight});
diff --git a/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm b/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
index 7ea4dee..88bdb15 100644
--- a/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/SteeringTarget.pm
@@ -308,6 +308,7 @@ sub is_target_valid {
# Validate the input against the rules
my $result = validate( $params, $rules );
+ # TODO: fix indentation below, validate GEO-types
if ( $result->{success} ) {
if ( ( $target_type eq "STEERING_WEIGHT" ) and ( $params->{value} < 0 ) ) {
return ( 0, "Invalid value for target type STEERING_WEIGHT: can not be negative" );
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/cache/Cache.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/cache/Cache.java
index a9882ff..efd676a 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/cache/Cache.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/cache/Cache.java
@@ -27,6 +27,7 @@ import java.util.Map;
import com.comcast.cdn.traffic_control.traffic_router.core.hash.DefaultHashable;
import com.comcast.cdn.traffic_control.traffic_router.core.hash.Hashable;
import com.comcast.cdn.traffic_control.traffic_router.core.util.JsonUtils;
+import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
import com.fasterxml.jackson.databind.JsonNode;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
@@ -43,12 +44,18 @@ public class Cache implements Comparable<Cache>, Hashable<Cache> {
private List<InetRecord> ipAddresses;
private int port;
private final Map<String, DeliveryServiceReference> deliveryServices = new HashMap<String, DeliveryServiceReference>();
+ private final Geolocation geolocation;
private final Hashable hashable = new DefaultHashable();
private int httpsPort = 443;
- public Cache(final String id, final String hashId, final int hashCount) {
+ public Cache(final String id, final String hashId, final int hashCount, final Geolocation geolocation) {
this.id = id;
hashable.generateHashes(hashId, hashCount > 0 ? hashCount : REPLICAS);
+ this.geolocation = geolocation;
+ }
+
+ public Cache(final String id, final String hashId, final int hashCount) {
+ this(id, hashId, hashCount, null);
}
@Override
@@ -74,6 +81,10 @@ public class Cache implements Comparable<Cache>, Hashable<Cache> {
return deliveryServices.values();
}
+ public Geolocation getGeolocation() {
+ return geolocation;
+ }
+
public String getFqdn() {
return fqdn;
}
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 6ab8fa1..90fc738 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
@@ -340,7 +340,7 @@ public class ConfigHandler {
hashId = jo.get("hashId").textValue();
}
- final Cache cache = new Cache(node, hashId, JsonUtils.optInt(jo, "hashCount"));
+ final Cache cache = new Cache(node, hashId, JsonUtils.optInt(jo, "hashCount"), loc.getGeolocation());
cache.setFqdn(JsonUtils.getString(jo, "fqdn"));
cache.setPort(JsonUtils.getInt(jo, "port"));
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java
new file mode 100644
index 0000000..5436dae
--- /dev/null
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringGeolocationComparator.java
@@ -0,0 +1,75 @@
+/*
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.comcast.cdn.traffic_control.traffic_router.core.ds;
+
+import java.util.Comparator;
+
+import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
+
+// TODO: add unit tests for this class
+public class SteeringGeolocationComparator implements Comparator<SteeringResult> {
+
+ private final Geolocation clientLocation;
+
+ public SteeringGeolocationComparator(final Geolocation clientLocation) {
+ this.clientLocation = clientLocation;
+ }
+
+ @Override
+ public int compare(final SteeringResult result1, final SteeringResult result2) {
+ final Geolocation originGeo1 = result1.getSteeringTarget().getGeolocation();
+ final Geolocation originGeo2 = result2.getSteeringTarget().getGeolocation();
+
+ final Geolocation cacheGeo1 = result1.getCache().getGeolocation();
+ final Geolocation cacheGeo2 = result2.getCache().getGeolocation();
+
+ // null origin geolocations are considered greater than (i.e. farther away) than non-null origin geolocations
+ if (originGeo1 != null && originGeo2 == null) {
+ return -1;
+ }
+ if (originGeo1 == null && originGeo2 != null) {
+ return 1;
+ }
+ if (originGeo1 == null && originGeo2 == null) {
+ return 0;
+ }
+
+ // same cache and origin locations, prefer lower geoOrder
+ if (cacheGeo1.equals(cacheGeo2) && originGeo1.equals(originGeo2)) {
+ return Integer.compare(result1.getSteeringTarget().getGeoOrder(), result2.getSteeringTarget().getGeoOrder());
+ }
+
+ final double distanceFromClientToCache1 = clientLocation.getDistanceFrom(cacheGeo1);
+ final double distanceFromClientToCache2 = clientLocation.getDistanceFrom(cacheGeo2);
+
+ final double distanceFromCacheToOrigin1 = cacheGeo1.getDistanceFrom(originGeo1);
+ final double distanceFromCacheToOrigin2 = cacheGeo2.getDistanceFrom(originGeo2);
+
+ final double totalDistance1 = distanceFromClientToCache1 + distanceFromCacheToOrigin1;
+ final double totalDistance2 = distanceFromClientToCache2 + distanceFromCacheToOrigin2;
+
+ // different cache and origin locations, prefer shortest total distance
+ if (totalDistance1 != totalDistance2) {
+ // TODO: if the difference is smaller than a certain threshold, still prefer the closer edge even though distance is greater?
+ return Double.compare(totalDistance1, totalDistance2);
+ }
+
+ // total distance is equal, prefer the closest edge to the client
+ return Double.compare(distanceFromClientToCache1, distanceFromClientToCache2);
+
+ }
+
+}
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java
index 17340ac..2bf197b 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringRegistry.java
@@ -56,9 +56,15 @@ public class SteeringRegistry {
registry.putAll(newSteerings);
for (final Steering steering : steerings) {
for (final SteeringTarget target : steering.getTargets()) {
- if (target.getWeight() > 0) {
+ if (target.getGeolocation() != null && target.getGeoOrder() != 0) {
+ LOGGER.info("Steering " + steering.getDeliveryService() + " target " + target.getDeliveryService() + " now has geolocation [" + target.getLatitude() + ", " + target.getLongitude() + "] and geoOrder " + target.getGeoOrder());
+ } else if (target.getGeolocation() != null && target.getWeight() > 0) {
+ LOGGER.info("Steering " + steering.getDeliveryService() + " target " + target.getDeliveryService() + " now has geolocation [" + target.getLatitude() + ", " + target.getLongitude() + "] and weight " + target.getWeight());
+ } else if (target.getGeolocation() != null) {
+ LOGGER.info("Steering " + steering.getDeliveryService() + " target " + target.getDeliveryService() + " now has geolocation [" + target.getLatitude() + ", " + target.getLongitude() + "]");
+ } else if (target.getWeight() > 0) {
LOGGER.info("Steering " + steering.getDeliveryService() + " target " + target.getDeliveryService() + " now has weight " + target.getWeight());
- } else if (target.getOrder() > 0 || target.getOrder() < 0) { // this target has a specific order set
+ } else if (target.getOrder() != 0) { // this target has a specific order set
LOGGER.info("Steering " + steering.getDeliveryService() + " target " + target.getDeliveryService() + " now has order " + target.getOrder());
} else {
LOGGER.info("Steering " + steering.getDeliveryService() + " target " + target.getDeliveryService() + " now has weight " + target.getWeight() + " and order " + target.getOrder());
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringResult.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringResult.java
new file mode 100644
index 0000000..b86ccbd
--- /dev/null
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringResult.java
@@ -0,0 +1,54 @@
+/*
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.comcast.cdn.traffic_control.traffic_router.core.ds;
+
+import com.comcast.cdn.traffic_control.traffic_router.core.cache.Cache;
+
+public class SteeringResult {
+ private SteeringTarget steeringTarget;
+ private DeliveryService deliveryService;
+ private Cache cache;
+
+ public SteeringResult(final SteeringTarget steeringTarget, final DeliveryService deliveryService) {
+ this.steeringTarget = steeringTarget;
+ this.deliveryService = deliveryService;
+ }
+
+ public SteeringTarget getSteeringTarget() {
+ return steeringTarget;
+ }
+
+ public void setSteeringTarget(final SteeringTarget steeringTarget) {
+ this.steeringTarget = steeringTarget;
+ }
+
+ public DeliveryService getDeliveryService() {
+ return deliveryService;
+ }
+
+ public void setDeliveryService(final DeliveryService deliveryService) {
+ this.deliveryService = deliveryService;
+ }
+
+ public Cache getCache() {
+ return cache;
+ }
+
+ public void setCache(final Cache cache) {
+ this.cache = cache;
+ }
+
+}
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
index ba400ac..d1a38df 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/SteeringTarget.java
@@ -16,15 +16,29 @@
package com.comcast.cdn.traffic_control.traffic_router.core.ds;
import com.comcast.cdn.traffic_control.traffic_router.core.hash.DefaultHashable;
+import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
import com.fasterxml.jackson.annotation.JsonProperty;
public class SteeringTarget extends DefaultHashable {
+
+ private static final double DEFAULT_LAT = 0.0;
+ private static final double DEFAULT_LON = 0.0;
+
@JsonProperty
private String deliveryService;
@JsonProperty
private int weight;
@JsonProperty
private int order = 0;
+ @JsonProperty
+ private int geoOrder = 0;
+ // TODO: lat/long should probably be moved to the DeliveryService itself, but this is ok for a start
+ @JsonProperty
+ private double latitude = DEFAULT_LAT;
+ @JsonProperty
+ private double longitude = DEFAULT_LON;
+
+ private Geolocation geolocation;
public DefaultHashable generateHashes() {
return generateHashes(deliveryService, weight);
@@ -54,6 +68,40 @@ public class SteeringTarget extends DefaultHashable {
return order;
}
+ public void setGeoOrder(final int geoOrder) {
+ this.geoOrder = geoOrder;
+ }
+
+ public int getGeoOrder() {
+ return geoOrder;
+ }
+
+ public void setLatitude(final double latitude) {
+ this.latitude = latitude;
+ }
+
+ public double getLatitude() {
+ return latitude;
+ }
+
+ public void setLongitude(final double longitude) {
+ this.longitude = longitude;
+ }
+
+ public double getLongitude() {
+ return longitude;
+ }
+
+ public Geolocation getGeolocation() {
+ if (geolocation != null) {
+ return geolocation;
+ }
+ if (latitude != DEFAULT_LAT && longitude != DEFAULT_LON) {
+ geolocation = new Geolocation(latitude, longitude);
+ }
+ return geolocation;
+ }
+
@Override
@SuppressWarnings("PMD")
public boolean equals(Object o) {
@@ -64,6 +112,9 @@ public class SteeringTarget extends DefaultHashable {
if (weight != target.weight) return false;
if (order != target.order) return false;
+ if (geoOrder != target.geoOrder) return false;
+ if (latitude != target.latitude) return false;
+ if (longitude != target.longitude) return false;
return deliveryService != null ? deliveryService.equals(target.deliveryService) : target.deliveryService == null;
}
@@ -73,6 +124,9 @@ public class SteeringTarget extends DefaultHashable {
int result = deliveryService != null ? deliveryService.hashCode() : 0;
result = 31 * result + weight;
result = 31 * result + order;
+ result = 31 * result + geoOrder;
+ result = 31 * result + (int) latitude;
+ result = 31 * result + (int) longitude;
return result;
}
}
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 caaf4e8..5f49865 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
@@ -22,6 +22,7 @@ import java.net.URL;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -31,6 +32,7 @@ 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.SteeringResult;
import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringTarget;
import com.comcast.cdn.traffic_control.traffic_router.core.ds.Steering;
import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringRegistry;
@@ -51,6 +53,7 @@ import com.comcast.cdn.traffic_control.traffic_router.core.cache.InetRecord;
import com.comcast.cdn.traffic_control.traffic_router.core.dns.ZoneManager;
import com.comcast.cdn.traffic_control.traffic_router.core.dns.DNSAccessRecord;
import com.comcast.cdn.traffic_control.traffic_router.core.ds.DeliveryService;
+import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringGeolocationComparator;
import com.comcast.cdn.traffic_control.traffic_router.core.loc.FederationRegistry;
import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
import com.comcast.cdn.traffic_control.traffic_router.geolocation.GeolocationException;
@@ -510,36 +513,38 @@ public class TrafficRouter {
public HTTPRouteResult multiRoute(final HTTPRequest request, final Track track) throws MalformedURLException, GeolocationException {
final DeliveryService entryDeliveryService = cacheRegister.getDeliveryService(request, true);
- if (isTlsMismatch(request, entryDeliveryService)) {
- track.setResult(ResultType.ERROR);
- track.setResultDetails(ResultDetails.DS_TLS_MISMATCH);
+ final List<SteeringResult> steeringResults = getSteeringResults(request, track, entryDeliveryService);
+
+ if (steeringResults == null) {
return null;
}
final HTTPRouteResult routeResult = new HTTPRouteResult(true);
- final List<DeliveryService> deliveryServices = getDeliveryServices(request, track, entryDeliveryService);
+ routeResult.setDeliveryService(entryDeliveryService);
- if (deliveryServices == null) {
- return null;
- }
+ final List<SteeringResult> resultsToRemove = new ArrayList<>();
- routeResult.setDeliveryService(entryDeliveryService);
- for (final DeliveryService deliveryService : deliveryServices) {
- if (deliveryService.isRegionalGeoEnabled()) {
- LOGGER.error("Regional Geo Blocking is not supported with multi-route delivery services.. skipping " + entryDeliveryService.getId() + "/" + deliveryService.getId());
- continue;
- }
+ for (final SteeringResult steeringResult : steeringResults) {
+ final DeliveryService ds = steeringResult.getDeliveryService();
- if (deliveryService.isAvailable()) {
- final List<Cache> caches = selectCaches(request, deliveryService, track);
+ final List<Cache> caches = selectCaches(request, ds, track);
- if (caches != null && !caches.isEmpty()) {
- final Cache cache = consistentHasher.selectHashable(caches, deliveryService.getDispersion(), request.getPath());
- routeResult.addUrl(new URL(deliveryService.createURIString(request, cache)));
- }
+ if (caches != null && !caches.isEmpty()) {
+ final Cache cache = consistentHasher.selectHashable(caches, ds.getDispersion(), request.getPath());
+ steeringResult.setCache(cache);
+ } else {
+ resultsToRemove.add(steeringResult);
}
}
+ steeringResults.removeAll(resultsToRemove);
+
+ geoSortSteeringResults(steeringResults, request.getClientIP(), entryDeliveryService);
+
+ for (final SteeringResult steeringResult: steeringResults) {
+ routeResult.addUrl(new URL(steeringResult.getDeliveryService().createURIString(request, steeringResult.getCache())));
+ }
+
if (routeResult.getUrls().isEmpty()) {
routeResult.addUrl(entryDeliveryService.getFailureHttpResponse(request, track));
}
@@ -612,24 +617,41 @@ public class TrafficRouter {
return routeResult;
}
- private List<DeliveryService> getDeliveryServices(final HTTPRequest request, final Track track, final DeliveryService entryDeliveryService) {
- final List<DeliveryService> deliveryServices = consistentHashMultiDeliveryService(entryDeliveryService, request.getPath());
+ private List<SteeringResult> getSteeringResults(final HTTPRequest request, final Track track, final DeliveryService entryDeliveryService) {
+
+ if (isTlsMismatch(request, entryDeliveryService)) {
+ track.setResult(ResultType.ERROR);
+ track.setResultDetails(ResultDetails.DS_TLS_MISMATCH);
+ return null;
+ }
+
+ final List<SteeringResult> steeringResults = consistentHashMultiDeliveryService(entryDeliveryService, request.getPath());
- if (deliveryServices == null || deliveryServices.isEmpty()) {
+ if (steeringResults == null || steeringResults.isEmpty()) {
track.setResult(ResultType.DS_MISS);
track.setResultDetails(ResultDetails.DS_NOT_FOUND);
return null;
}
- for (final DeliveryService deliveryService : deliveryServices) {
- if (isTlsMismatch(request, deliveryService)) {
+ List<SteeringResult> toBeRemoved = new ArrayList<>();
+ for (final SteeringResult steeringResult : steeringResults) {
+ final DeliveryService ds = steeringResult.getDeliveryService();
+ if (isTlsMismatch(request, ds)) {
track.setResult(ResultType.ERROR);
track.setResultDetails(ResultDetails.DS_TLS_MISMATCH);
return null;
}
+ if (ds.isRegionalGeoEnabled()) {
+ LOGGER.error("Regional Geo Blocking is not supported with multi-route delivery services.. skipping " + entryDeliveryService.getId() + "/" + ds.getId());
+ toBeRemoved.add(steeringResult);
+ } else if (!ds.isAvailable()) {
+ toBeRemoved.add(steeringResult);
+ }
+
}
- return deliveryServices;
+ steeringResults.removeAll(toBeRemoved);
+ return steeringResults.isEmpty() ? null : steeringResults;
}
private DeliveryService getDeliveryService(final HTTPRequest request, final Track track) {
@@ -815,10 +837,6 @@ public class TrafficRouter {
return consistentHasher.selectHashable(caches, deliveryService.getDispersion(), requestPath);
}
- public DeliveryService consistentHashDeliveryService(final String deliveryServiceId, final String requestPath) {
- return consistentHashDeliveryService(cacheRegister.getDeliveryService(deliveryServiceId), requestPath, "");
- }
-
private boolean isSteeringDeliveryService(final DeliveryService deliveryService) {
return deliveryService != null && steeringRegistry.has(deliveryService.getId());
}
@@ -833,16 +851,45 @@ public class TrafficRouter {
return steeringRegistry.get(deliveryService.getId()).isClientSteering();
}
- public List<DeliveryService> consistentHashMultiDeliveryService(final DeliveryService deliveryService, final String requestPath) {
+ private Geolocation getClientLocationByCoverageZoneOrGeo(final String clientIP, final DeliveryService deliveryService) {
+ Geolocation clientLocation;
+ final NetworkNode networkNode = getNetworkNode(clientIP);
+ if (networkNode != null && networkNode.getGeolocation() != null) {
+ clientLocation = networkNode.getGeolocation();
+ } else {
+ try {
+ clientLocation = getLocation(clientIP, deliveryService);
+ } catch (GeolocationException e) {
+ clientLocation = null;
+ }
+ }
+ return deliveryService.supportLocation(clientLocation);
+ }
+
+ // TODO: add unit test for this method
+ private void geoSortSteeringResults(final List<SteeringResult> steeringResults, final String clientIP, final DeliveryService deliveryService) {
+ if (clientIP == null || clientIP.isEmpty()
+ || steeringResults.stream().allMatch(t -> t.getSteeringTarget().getGeolocation() == null)) {
+ return;
+ }
+
+ final Geolocation clientLocation = getClientLocationByCoverageZoneOrGeo(clientIP, deliveryService);
+ if (clientLocation != null) {
+ Collections.sort(steeringResults, new SteeringGeolocationComparator(clientLocation));
+ Collections.sort(steeringResults, Comparator.comparingInt(s -> s.getSteeringTarget().getOrder())); // re-sort by order to preserve the ordering done by ConsistentHasher
+ }
+ }
+
+ public List<SteeringResult> consistentHashMultiDeliveryService(final DeliveryService deliveryService, final String requestPath) {
if (deliveryService == null) {
return null;
}
- final List<DeliveryService> deliveryServices = new ArrayList<DeliveryService>();
+ final List<SteeringResult> steeringResults = new ArrayList<>();
if (!isSteeringDeliveryService(deliveryService)) {
- deliveryServices.add(deliveryService);
- return deliveryServices;
+ steeringResults.add(new SteeringResult(null, deliveryService));
+ return steeringResults;
}
final Steering steering = steeringRegistry.get(deliveryService.getId());
@@ -852,11 +899,15 @@ public class TrafficRouter {
final DeliveryService target = cacheRegister.getDeliveryService(steeringTarget.getDeliveryService());
if (target != null) { // target might not be in CRConfig yet
- deliveryServices.add(target);
+ steeringResults.add(new SteeringResult(steeringTarget, target));
}
}
- return deliveryServices;
+ return steeringResults;
+ }
+
+ public DeliveryService consistentHashDeliveryService(final String deliveryServiceId, final String requestPath) {
+ return consistentHashDeliveryService(cacheRegister.getDeliveryService(deliveryServiceId), requestPath, "");
}
public DeliveryService consistentHashDeliveryService(final DeliveryService deliveryService, final String requestPath, final String xtcSteeringOption) {
--
To stop receiving notification emails like this one, please contact
elsloo@apache.org.