You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/11/23 23:09:34 UTC
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5265: Exclude Mid-Tier cache servers from CDN Snapshots
zrhoffman commented on a change in pull request #5265:
URL: https://github.com/apache/trafficcontrol/pull/5265#discussion_r528983580
##########
File path: traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go
##########
@@ -32,16 +32,29 @@ import (
"github.com/lib/pq"
)
-const CDNSOAMinimum = 30 * time.Second
-const CDNSOAExpire = 604800 * time.Second
-const CDNSOARetry = 7200 * time.Second
-const CDNSOARefresh = 28800 * time.Second
-const CDNSOAAdmin = "traffic_ops"
+// Start-Of-Authority record fields; see RFC1035 section 3.3.13.
+// (https://tools.ietf.org/html/rfc1035#section-3.3.13).
+const (
+ CDNSOAAdmin = "traffic_ops"
+ CDNSOAExpire = 604800 * time.Second
Review comment:
`CDNSOAExpire` can be written in terms of `time.Hour`.
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -34,10 +34,13 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
)
-const RouterTypeName = "CCR"
-const MonitorTypeName = "RASCAL"
-const EdgeTypePrefix = "EDGE"
-const MidTypePrefix = "MID"
+// Names prefixes for each type of server to appear in CDN Snapshots.
+const (
+ RouterTypeName = "CCR"
+ MonitorTypeName = "RASCAL"
+ EdgeTypePrefix = "EDGE"
+ MidTypePrefix = "MID"
+)
Review comment:
All of these constants exist in `tc`, they can be removed from from this package.
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -304,20 +321,21 @@ func getServerDSes(cdn string, tx *sql.Tx, domain string) (map[tc.CacheName]map[
return nil, errors.New("Error getting server deliveryservices: " + err.Error())
}
- q := `
-select ds.xml_id as ds, dt.name as ds_type, ds.routing_name, r.pattern as pattern,
-ds.topology IS NOT NULL as has_topology
-from regex as r
-inner join type as rt on r.type = rt.id
-inner join deliveryservice_regex as dsr on dsr.regex = r.id
-inner join deliveryservice as ds on ds.id = dsr.deliveryservice
-inner join type as dt on dt.id = ds.type
-where ds.cdn_id = (select id from cdn where name = $1)
-and ds.active = true` +
- fmt.Sprintf(" and dt.name != '%s' ", tc.DSTypeAnyMap) + `
-and rt.name = 'HOST_REGEXP'
-order by dsr.set_number asc
-`
+ q := `SELECT ds.xml_id AS ds,
+ dt.name AS ds_type,
+ ds.routing_name,
+ r.pattern AS pattern,
+ ds.topology IS NOT NULL AS has_topology
+ FROM regex AS r
+ INNER JOIN type AS rt ON r.type = rt.id
+ INNER JOIN deliveryservice_regex AS dsr ON dsr.regex = r.id
+ INNER JOIN deliveryservice AS ds ON ds.id = dsr.deliveryservice
+ INNER JOIN type AS dt ON dt.id = ds.type
+ WHERE ds.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND ds.active = true
+ AND dt.name != '` + string(tc.DSTypeAnyMap) + `'
+ AND rt.name = 'HOST_REGEXP'
Review comment:
`HOST_REGEXP` can be `tc.DSMatchTypeHostRegex`
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -389,16 +407,20 @@ type ServerParams struct {
func getServerParams(cdn string, tx *sql.Tx) (map[string]ServerParams, error) {
params := map[string]ServerParams{}
- q := `
-select s.host_name, p.name, p.value
-from server as s
-left join profile_parameter as pp on pp.profile = s.profile
-left join parameter as p on p.id = pp.parameter
-inner join status as st ON st.id = s.status
-where s.cdn_id = (select id from cdn where name = $1)
-and ((p.config_file = 'CRConfig.json' and (p.name = 'weight' or p.name = 'weightMultiplier')) or (p.name = 'api.port') or (p.name = 'secure.api.port'))
-and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
-`
+ q := `SELECT s.host_name,
+ p.name,
+ p.value
+ FROM server AS s
+ LEFT JOIN profile_parameter AS pp ON pp.profile = s.profile
+ LEFT JOIN parameter AS p ON p.id = pp.parameter
+ INNER JOIN status AS st ON st.id = s.status
+ WHERE s.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND (
+ (p.config_file = 'CRConfig.json' AND (p.name = 'weight' OR p.name = 'weightMultiplier')) OR
+ (p.name = 'api.port') OR
+ (p.name = 'secure.api.port')
+ )
+ AND (st.name = 'REPORTED' OR st.name = 'ONLINE' OR st.name = 'ADMIN_DOWN')`
Review comment:
`REPORTED` can be `tc.CacheStatusReported`
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -260,20 +272,19 @@ func getAllServers(cdn string, tx *sql.Tx) (map[string]ServerUnion, error) {
}
func getServerDSNames(cdn string, tx *sql.Tx) (map[tc.CacheName][]tc.DeliveryServiceName, error) {
- q := `
-select s.host_name, ds.xml_id
-from deliveryservice_server as dss
-inner join server as s on dss.server = s.id
-inner join deliveryservice as ds on ds.id = dss.deliveryservice
-inner join type as dt on dt.id = ds.type
-inner join profile as p on p.id = s.profile
-inner join status as st ON st.id = s.status
-where ds.cdn_id = (select id from cdn where name = $1)
-and ds.active = true` +
- fmt.Sprintf(" and dt.name != '%s' ", tc.DSTypeAnyMap) + `
-and p.routing_disabled = false
-and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
-`
+ q := `SELECT s.host_name,
+ ds.xml_id
+ FROM deliveryservice_server AS dss
+ INNER JOIN server AS s ON dss.server = s.id
+ INNER JOIN deliveryservice AS ds ON ds.id = dss.deliveryservice
+ INNER JOIN type AS dt ON dt.id = ds.type
+ INNER JOIN profile AS p ON p.id = s.profile
+ INNER JOIN status AS st ON st.id = s.status
+ WHERE ds.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND ds.active = true
+ AND dt.name != '` + string(tc.DSTypeAnyMap) + `'
+ AND p.routing_disabled = false
+ AND (st.name = 'REPORTED' OR st.name = 'ONLINE' OR st.name = 'ADMIN_DOWN')`
Review comment:
`ADMIN_DOWN` can be `tc.CacheStatusAdminDown`
##########
File path: traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go
##########
@@ -32,16 +32,29 @@ import (
"github.com/lib/pq"
)
-const CDNSOAMinimum = 30 * time.Second
-const CDNSOAExpire = 604800 * time.Second
-const CDNSOARetry = 7200 * time.Second
-const CDNSOARefresh = 28800 * time.Second
-const CDNSOAAdmin = "traffic_ops"
+// Start-Of-Authority record fields; see RFC1035 section 3.3.13.
+// (https://tools.ietf.org/html/rfc1035#section-3.3.13).
+const (
+ CDNSOAAdmin = "traffic_ops"
+ CDNSOAExpire = 604800 * time.Second
+ CDNSOAMinimum = 30 * time.Second
+ CDNSOARefresh = 28800 * time.Second
+ CDNSOARetry = 7200 * time.Second
Review comment:
`CDNSOARefresh` and `CDNSOARetry` can be written in terms of `time.Hour`.
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -260,20 +272,19 @@ func getAllServers(cdn string, tx *sql.Tx) (map[string]ServerUnion, error) {
}
func getServerDSNames(cdn string, tx *sql.Tx) (map[tc.CacheName][]tc.DeliveryServiceName, error) {
- q := `
-select s.host_name, ds.xml_id
-from deliveryservice_server as dss
-inner join server as s on dss.server = s.id
-inner join deliveryservice as ds on ds.id = dss.deliveryservice
-inner join type as dt on dt.id = ds.type
-inner join profile as p on p.id = s.profile
-inner join status as st ON st.id = s.status
-where ds.cdn_id = (select id from cdn where name = $1)
-and ds.active = true` +
- fmt.Sprintf(" and dt.name != '%s' ", tc.DSTypeAnyMap) + `
-and p.routing_disabled = false
-and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
-`
+ q := `SELECT s.host_name,
+ ds.xml_id
+ FROM deliveryservice_server AS dss
+ INNER JOIN server AS s ON dss.server = s.id
+ INNER JOIN deliveryservice AS ds ON ds.id = dss.deliveryservice
+ INNER JOIN type AS dt ON dt.id = ds.type
+ INNER JOIN profile AS p ON p.id = s.profile
+ INNER JOIN status AS st ON st.id = s.status
+ WHERE ds.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND ds.active = true
+ AND dt.name != '` + string(tc.DSTypeAnyMap) + `'
+ AND p.routing_disabled = false
+ AND (st.name = 'REPORTED' OR st.name = 'ONLINE' OR st.name = 'ADMIN_DOWN')`
Review comment:
`REPORTED` can be `tc.CacheStatusReported`
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -389,16 +407,20 @@ type ServerParams struct {
func getServerParams(cdn string, tx *sql.Tx) (map[string]ServerParams, error) {
params := map[string]ServerParams{}
- q := `
-select s.host_name, p.name, p.value
-from server as s
-left join profile_parameter as pp on pp.profile = s.profile
-left join parameter as p on p.id = pp.parameter
-inner join status as st ON st.id = s.status
-where s.cdn_id = (select id from cdn where name = $1)
-and ((p.config_file = 'CRConfig.json' and (p.name = 'weight' or p.name = 'weightMultiplier')) or (p.name = 'api.port') or (p.name = 'secure.api.port'))
-and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
-`
+ q := `SELECT s.host_name,
+ p.name,
+ p.value
+ FROM server AS s
+ LEFT JOIN profile_parameter AS pp ON pp.profile = s.profile
+ LEFT JOIN parameter AS p ON p.id = pp.parameter
+ INNER JOIN status AS st ON st.id = s.status
+ WHERE s.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND (
+ (p.config_file = 'CRConfig.json' AND (p.name = 'weight' OR p.name = 'weightMultiplier')) OR
+ (p.name = 'api.port') OR
+ (p.name = 'secure.api.port')
+ )
+ AND (st.name = 'REPORTED' OR st.name = 'ONLINE' OR st.name = 'ADMIN_DOWN')`
Review comment:
`ONLINE` can be `tc.CacheStatusOnline`
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -260,20 +272,19 @@ func getAllServers(cdn string, tx *sql.Tx) (map[string]ServerUnion, error) {
}
func getServerDSNames(cdn string, tx *sql.Tx) (map[tc.CacheName][]tc.DeliveryServiceName, error) {
- q := `
-select s.host_name, ds.xml_id
-from deliveryservice_server as dss
-inner join server as s on dss.server = s.id
-inner join deliveryservice as ds on ds.id = dss.deliveryservice
-inner join type as dt on dt.id = ds.type
-inner join profile as p on p.id = s.profile
-inner join status as st ON st.id = s.status
-where ds.cdn_id = (select id from cdn where name = $1)
-and ds.active = true` +
- fmt.Sprintf(" and dt.name != '%s' ", tc.DSTypeAnyMap) + `
-and p.routing_disabled = false
-and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
-`
+ q := `SELECT s.host_name,
+ ds.xml_id
+ FROM deliveryservice_server AS dss
+ INNER JOIN server AS s ON dss.server = s.id
+ INNER JOIN deliveryservice AS ds ON ds.id = dss.deliveryservice
+ INNER JOIN type AS dt ON dt.id = ds.type
+ INNER JOIN profile AS p ON p.id = s.profile
+ INNER JOIN status AS st ON st.id = s.status
+ WHERE ds.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND ds.active = true
+ AND dt.name != '` + string(tc.DSTypeAnyMap) + `'
+ AND p.routing_disabled = false
+ AND (st.name = 'REPORTED' OR st.name = 'ONLINE' OR st.name = 'ADMIN_DOWN')`
Review comment:
`ONLINE` can be `tc.CacheStatusOnline`
##########
File path: traffic_ops/traffic_ops_golang/crconfig/servers.go
##########
@@ -389,16 +407,20 @@ type ServerParams struct {
func getServerParams(cdn string, tx *sql.Tx) (map[string]ServerParams, error) {
params := map[string]ServerParams{}
- q := `
-select s.host_name, p.name, p.value
-from server as s
-left join profile_parameter as pp on pp.profile = s.profile
-left join parameter as p on p.id = pp.parameter
-inner join status as st ON st.id = s.status
-where s.cdn_id = (select id from cdn where name = $1)
-and ((p.config_file = 'CRConfig.json' and (p.name = 'weight' or p.name = 'weightMultiplier')) or (p.name = 'api.port') or (p.name = 'secure.api.port'))
-and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN')
-`
+ q := `SELECT s.host_name,
+ p.name,
+ p.value
+ FROM server AS s
+ LEFT JOIN profile_parameter AS pp ON pp.profile = s.profile
+ LEFT JOIN parameter AS p ON p.id = pp.parameter
+ INNER JOIN status AS st ON st.id = s.status
+ WHERE s.cdn_id = (SELECT id FROM cdn WHERE name = $1)
+ AND (
+ (p.config_file = 'CRConfig.json' AND (p.name = 'weight' OR p.name = 'weightMultiplier')) OR
+ (p.name = 'api.port') OR
+ (p.name = 'secure.api.port')
+ )
+ AND (st.name = 'REPORTED' OR st.name = 'ONLINE' OR st.name = 'ADMIN_DOWN')`
Review comment:
`ADMIN_DOWN` can be `tc.CacheStatusAdminDown`
##########
File path: traffic_monitor/todata/todata.go
##########
@@ -156,6 +157,14 @@ func (d TODataThreadsafe) Update(to towrap.TrafficOpsSessionThreadsafe, cdn stri
return fmt.Errorf("Error getting last CRConfig: %v", err)
}
+ monitoring, err := to.TrafficMonitorConfigMap(cdn)
+ if err != nil {
+ return fmt.Errorf("error getting monitoring configuration: %v", err)
Review comment:
`fmt.Errorf()` using `%v` is about 4.3 times slower than `errors.New()`. For TO it's not as important, but for TM it is.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org