You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by sr...@apache.org on 2022/05/05 15:40:00 UTC
[trafficcontrol] branch master updated: Fix max_origin_connections calculation for topology-based DSes (#6807)
This is an automated email from the ASF dual-hosted git repository.
srijeet0406 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new d7deaec62d Fix max_origin_connections calculation for topology-based DSes (#6807)
d7deaec62d is described below
commit d7deaec62d068d26d2283ef14a384cce827e17b2
Author: Rawlin Peters <ra...@apache.org>
AuthorDate: Thu May 5 09:39:52 2022 -0600
Fix max_origin_connections calculation for topology-based DSes (#6807)
Instead of only dividing the maxOriginConnections by the number of
caches in the same cachegroup, divide it by the number of caches in the
same tier.
Closes: #6806
---
CHANGELOG.md | 1 +
lib/go-atscfg/headerrewritedotconfig.go | 82 ++++++++++-
lib/go-atscfg/headerrewritedotconfig_test.go | 194 +++++++++++++++++++++++++++
3 files changed, 270 insertions(+), 7 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 701fdd04ec..eff31f54f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Only `operations` and `admin` roles should have the `DELIVERY-SERVICE:UPDATE` permission.
- [#6369](https://github.com/apache/trafficcontrol/pull/6369) Fixed `/acme_accounts` endpoint to validate email and URL fields
- Fixed searching of the ds parameter merge_parent_groups slice.
+- [#6806](https://github.com/apache/trafficcontrol/issues/6806) t3c calculates max_origin_connections incorrectly for topology-based delivery services
- Fixed TO API `PUT /servers/:id/status` to only queue updates on the same CDN as the updated server
- t3c-generate fix for combining remapconfig and cachekeyconfig parameters for MakeRemapDotConfig call.
- [#6780](https://github.com/apache/trafficcontrol/issues/6780) Fixed t3c to use secondary parents when there are no primary parents available.
diff --git a/lib/go-atscfg/headerrewritedotconfig.go b/lib/go-atscfg/headerrewritedotconfig.go
index c7f41723ca..ce2423830f 100644
--- a/lib/go-atscfg/headerrewritedotconfig.go
+++ b/lib/go-atscfg/headerrewritedotconfig.go
@@ -154,7 +154,7 @@ func MakeHeaderRewriteDotConfig(
atsMajorVersion, verWarns := getATSMajorVersion(tcServerParams)
warnings = append(warnings, verWarns...)
- assignedTierPeers, assignWarns := getAssignedTierPeers(server, ds, servers, deliveryServiceServers, cacheGroupsArr, serverCapabilities, requiredCapabilities[*ds.ID])
+ assignedTierPeers, assignWarns := getAssignedTierPeers(server, ds, topology, servers, deliveryServiceServers, cacheGroupsArr, serverCapabilities, requiredCapabilities[*ds.ID])
warnings = append(warnings, assignWarns...)
dsOnlinePeerCount := 0
@@ -282,6 +282,7 @@ func serverIsMid(server *Server) bool {
func getAssignedTierPeers(
server *Server,
ds *DeliveryService,
+ topology tc.Topology,
servers []Server,
deliveryServiceServers []DeliveryServiceServer,
cacheGroups []tc.CacheGroupNullable,
@@ -289,7 +290,7 @@ func getAssignedTierPeers(
dsRequiredCapabilities map[ServerCapability]struct{},
) ([]Server, []string) {
if ds.Topology != nil {
- return getTopologyTierServers(dsRequiredCapabilities, tc.CacheGroupName(*server.Cachegroup), servers, serverCapabilities)
+ return getTopologyTierServers(dsRequiredCapabilities, tc.CacheGroupName(*server.Cachegroup), topology, cacheGroups, servers, serverCapabilities)
}
if serverIsMid(server) {
return getAssignedMids(server, ds, servers, deliveryServiceServers, cacheGroups)
@@ -422,14 +423,14 @@ func getAssignedMids(
return assignedMids, warnings
}
-// getTopologyDSServerCount returns the servers in cg which will be used to serve ds.
+// getTopologyTierServers returns the servers in the same tier as cg which will be used to serve ds.
// This should only be used for DSes with Topologies.
-// It returns all servers in CG with the Capabilities of ds in cg.
-// It will not be the number of servers for Delivery Services not using Topologies, which use DeliveryService-Server assignments instead.
+// It returns all servers in with the Capabilities of ds in the same tier as cg.
// Returns the servers, and any warnings.
-func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}, cg tc.CacheGroupName, servers []Server, serverCapabilities map[int]map[ServerCapability]struct{}) ([]Server, []string) {
+func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}, cg tc.CacheGroupName, topology tc.Topology, cacheGroups []tc.CacheGroupNullable, servers []Server, serverCapabilities map[int]map[ServerCapability]struct{}) ([]Server, []string) {
warnings := []string{}
topoServers := []Server{}
+ cacheGroupsInSameTier := getCachegroupsInSameTopologyTier(string(cg), cacheGroups, topology)
for _, sv := range servers {
if sv.Cachegroup == nil {
warnings = append(warnings, "Servers had server with nil cachegroup, skipping!")
@@ -439,7 +440,7 @@ func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}
continue
}
- if *sv.Cachegroup != string(cg) {
+ if !cacheGroupsInSameTier[*sv.Cachegroup] {
continue
}
if !hasRequiredCapabilities(serverCapabilities[*sv.ID], dsRequiredCapabilities) {
@@ -450,6 +451,73 @@ func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}
return topoServers, warnings
}
+func getCachegroupsInSameTopologyTier(cg string, cacheGroups []tc.CacheGroupNullable, topology tc.Topology) map[string]bool {
+ cacheGroupMap := make(map[string]tc.CacheGroupNullable)
+ originCacheGroups := make(map[string]bool)
+ for _, cg := range cacheGroups {
+ if cg.Name == nil || cg.Type == nil {
+ continue
+ }
+ cacheGroupMap[*cg.Name] = cg
+ if *cg.Type == tc.CacheGroupOriginTypeName {
+ originCacheGroups[*cg.Name] = true
+ }
+ }
+ originNodes := make(map[int]bool)
+ nodeIndex := -1
+ for i, node := range topology.Nodes {
+ if node.Cachegroup == cg {
+ nodeIndex = i
+ }
+ if originCacheGroups[node.Cachegroup] {
+ originNodes[i] = true
+ }
+ }
+ nodesWithSameOriginDistances := getNodesWithSameOriginDistances(nodeIndex, topology, originNodes)
+ cacheGroupsInSameTopologyTier := make(map[string]bool)
+ for _, nodeI := range nodesWithSameOriginDistances {
+ cacheGroupsInSameTopologyTier[topology.Nodes[nodeI].Cachegroup] = true
+ }
+ return cacheGroupsInSameTopologyTier
+}
+
+func getNodesWithSameOriginDistances(nodeIndex int, topology tc.Topology, originNodes map[int]bool) []int {
+ originDistances := make(map[int]int)
+ nodeDistance := -1
+ for i := range topology.Nodes {
+ d := getOriginDistance(topology, i, originNodes, originDistances)
+ if nodeIndex == i {
+ nodeDistance = d
+ }
+ }
+ sameDistances := make([]int, 0)
+ for i, d := range originDistances {
+ if d == nodeDistance {
+ sameDistances = append(sameDistances, i)
+ }
+ }
+ return sameDistances
+}
+
+func getOriginDistance(topology tc.Topology, nodeIndex int, originNodes map[int]bool, originDistances map[int]int) int {
+ if originDistance, ok := originDistances[nodeIndex]; ok {
+ return originDistance
+ }
+ parents := topology.Nodes[nodeIndex].Parents
+ if len(parents) == 0 {
+ originDistances[nodeIndex] = 1
+ return originDistances[nodeIndex]
+ }
+ for _, p := range parents {
+ if originNodes[p] {
+ originDistances[nodeIndex] = 1
+ return originDistances[nodeIndex]
+ }
+ }
+ originDistances[nodeIndex] = 1 + getOriginDistance(topology, parents[0], originNodes, originDistances)
+ return originDistances[nodeIndex]
+}
+
var returnRe = regexp.MustCompile(`\s*__RETURN__\s*`)
// makeATCHeaderRewriteDirectives returns the Header Rewrite text for all per-Delivery-Service Traffic Control directives, such as MaxOriginConnections and ServiceCategory.
diff --git a/lib/go-atscfg/headerrewritedotconfig_test.go b/lib/go-atscfg/headerrewritedotconfig_test.go
index 946e982328..4c98d7300b 100644
--- a/lib/go-atscfg/headerrewritedotconfig_test.go
+++ b/lib/go-atscfg/headerrewritedotconfig_test.go
@@ -20,6 +20,7 @@ package atscfg
*/
import (
+ "reflect"
"strings"
"testing"
@@ -170,6 +171,199 @@ func TestMakeHeaderRewriteDotConfigNoMaxOriginConnections(t *testing.T) {
}
}
+func TestGetCachegroupsInSameTopologyTier(t *testing.T) {
+ allCachegroups := []tc.CacheGroupNullable{
+ {
+ Name: util.StrPtr("edge1"),
+ Type: util.StrPtr(tc.CacheGroupEdgeTypeName),
+ },
+ {
+ Name: util.StrPtr("edge2"),
+ Type: util.StrPtr(tc.CacheGroupEdgeTypeName),
+ },
+ {
+ Name: util.StrPtr("deep1"),
+ Type: util.StrPtr(tc.CacheGroupEdgeTypeName),
+ },
+ {
+ Name: util.StrPtr("mid1"),
+ Type: util.StrPtr(tc.CacheGroupMidTypeName),
+ },
+ {
+ Name: util.StrPtr("mid2"),
+ Type: util.StrPtr(tc.CacheGroupMidTypeName),
+ },
+ {
+ Name: util.StrPtr("org1"),
+ Type: util.StrPtr(tc.CacheGroupOriginTypeName),
+ },
+ {
+ Name: util.StrPtr("org2"),
+ Type: util.StrPtr(tc.CacheGroupOriginTypeName),
+ },
+ }
+ type testCase struct {
+ cachegroup string
+ cachegroups []tc.CacheGroupNullable
+ topology tc.Topology
+ expected map[string]bool
+ }
+ testCases := []testCase{
+ {
+ cachegroup: "edge1",
+ cachegroups: allCachegroups,
+ topology: tc.Topology{
+ Nodes: []tc.TopologyNode{
+ {
+ // 0
+ Cachegroup: "edge1",
+ Parents: []int{3},
+ },
+ {
+ // 1
+ Cachegroup: "deep1",
+ Parents: []int{0},
+ },
+ {
+ // 2
+ Cachegroup: "edge2",
+ Parents: []int{4},
+ },
+ {
+ // 3
+ Cachegroup: "mid1",
+ Parents: []int{},
+ },
+ {
+ // 4
+ Cachegroup: "mid2",
+ Parents: []int{},
+ },
+ },
+ },
+ expected: map[string]bool{"edge1": true, "edge2": true},
+ },
+ {
+ cachegroup: "deep1",
+ cachegroups: allCachegroups,
+ topology: tc.Topology{
+ Nodes: []tc.TopologyNode{
+ {
+ // 0
+ Cachegroup: "edge1",
+ Parents: []int{3},
+ },
+ {
+ // 1
+ Cachegroup: "deep1",
+ Parents: []int{0},
+ },
+ {
+ // 2
+ Cachegroup: "edge2",
+ Parents: []int{4},
+ },
+ {
+ // 3
+ Cachegroup: "mid1",
+ Parents: []int{},
+ },
+ {
+ // 4
+ Cachegroup: "mid2",
+ Parents: []int{},
+ },
+ },
+ },
+ expected: map[string]bool{"deep1": true},
+ },
+ {
+ cachegroup: "mid1",
+ cachegroups: allCachegroups,
+ topology: tc.Topology{
+ Nodes: []tc.TopologyNode{
+ {
+ // 0
+ Cachegroup: "edge1",
+ Parents: []int{3},
+ },
+ {
+ // 1
+ Cachegroup: "deep1",
+ Parents: []int{0},
+ },
+ {
+ // 2
+ Cachegroup: "edge2",
+ Parents: []int{4},
+ },
+ {
+ // 3
+ Cachegroup: "mid1",
+ Parents: []int{},
+ },
+ {
+ // 4
+ Cachegroup: "mid2",
+ Parents: []int{},
+ },
+ },
+ },
+ expected: map[string]bool{"mid1": true, "mid2": true},
+ },
+ {
+ cachegroup: "edge2",
+ cachegroups: allCachegroups,
+ topology: tc.Topology{
+ Nodes: []tc.TopologyNode{
+ {
+ // 0
+ Cachegroup: "edge1",
+ Parents: []int{3},
+ },
+ {
+ // 1
+ Cachegroup: "deep1",
+ Parents: []int{0},
+ },
+ {
+ // 2
+ Cachegroup: "edge2",
+ Parents: []int{4},
+ },
+ {
+ // 3
+ Cachegroup: "mid1",
+ Parents: []int{5},
+ },
+ {
+ // 4
+ Cachegroup: "mid2",
+ Parents: []int{6},
+ },
+ {
+ // 5
+ Cachegroup: "org1",
+ Parents: []int{},
+ },
+ {
+ // 5
+ Cachegroup: "org2",
+ Parents: []int{},
+ },
+ },
+ },
+ expected: map[string]bool{"edge1": true, "edge2": true},
+ },
+ }
+ for _, tc := range testCases {
+ actual := getCachegroupsInSameTopologyTier(tc.cachegroup, tc.cachegroups, tc.topology)
+ if !reflect.DeepEqual(tc.expected, actual) {
+ t.Errorf("getting cachegroups in same topology tier -- expected: %v, actual: %v", tc.expected, actual)
+ }
+ }
+}
+
func TestMakeHeaderRewriteMidDotConfig(t *testing.T) {
cdnName := "mycdn"
hdr := "myHeaderComment"