You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by dneuman64 <gi...@git.apache.org> on 2017/02/02 22:31:11 UTC
[GitHub] incubator-trafficcontrol pull request #250: CrStates now correctly updates t...
GitHub user dneuman64 opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/250
CrStates now correctly updates the state of a Delivery Service
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dneuman64/incubator-trafficcontrol goTM-crstates
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/250.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #250
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #250: CrStates now correctly updates t...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/250#discussion_r99372539
--- Diff: traffic_monitor_golang/traffic_monitor/health/cache.go ---
@@ -201,7 +201,7 @@ func CalcAvailability(results []cache.Result, pollerName string, statResultHisto
localStates.SetCache(result.ID, peer.IsAvailable{IsAvailable: isAvailable})
}
- calculateDeliveryServiceState(toData.DeliveryServiceServers, localStates)
+ // calculateDeliveryServiceState(toData.DeliveryServiceServers, localStates)
--- End diff --
It looks like the primary job of this func was to compute DisabledLocations. Is `disabledLocations` in `CrStates` still correctly populated?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #250: CrStates now correctly updates t...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/250
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #250: CrStates now correctly updates the stat...
Posted by dneuman64 <gi...@git.apache.org>.
Github user dneuman64 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/250
@rob05c you are right, didn't mean to keep that commented out. I removed the states functionality and kept the disabled locations functionality.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #250: CrStates now correctly updates t...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/250#discussion_r99379507
--- Diff: traffic_monitor_golang/traffic_monitor/deliveryservice/stat.go ---
@@ -241,8 +241,12 @@ func addDSPerSecStats(dsName enum.DeliveryServiceName, stat dsdata.Stat, lastSta
if dsErr != nil {
stat.CommonStats.IsAvailable.Value = false
stat.CommonStats.IsHealthy.Value = false
- stat.CommonStats.ErrorStr.Value = err.Error()
+ stat.CommonStats.ErrorStr.Value = dsErr.Error()
+
}
+ dsState, _ := states.GetDeliveryService(dsName)
--- End diff --
Nevermind, if it doesn't exist, it's default-constructed, which is fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #250: CrStates now correctly updates t...
Posted by elsloo <gi...@git.apache.org>.
Github user elsloo commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/250#discussion_r99355200
--- Diff: traffic_monitor_golang/traffic_monitor/health/cache.go ---
@@ -251,22 +251,22 @@ func eventDesc(status enum.CacheStatus, message string) string {
}
// calculateDeliveryServiceState calculates the state of delivery services from the new cache state data `cacheState` and the CRConfig data `deliveryServiceServers` and puts the calculated state in the outparam `deliveryServiceStates`
-func calculateDeliveryServiceState(deliveryServiceServers map[enum.DeliveryServiceName][]enum.CacheName, states peer.CRStatesThreadsafe) {
- deliveryServices := states.GetDeliveryServices()
- for deliveryServiceName, deliveryServiceState := range deliveryServices {
- if _, ok := deliveryServiceServers[deliveryServiceName]; !ok {
- // log.Errorf("CRConfig does not have delivery service %s, but traffic monitor poller does; skipping\n", deliveryServiceName)
- continue
- }
- deliveryServiceState.IsAvailable = false
- deliveryServiceState.DisabledLocations = []enum.CacheName{} // it's important this isn't nil, so it serialises to the JSON `[]` instead of `null`
- for _, server := range deliveryServiceServers[deliveryServiceName] {
- if available, _ := states.GetCache(server); available.IsAvailable {
- deliveryServiceState.IsAvailable = true
- } else {
- deliveryServiceState.DisabledLocations = append(deliveryServiceState.DisabledLocations, server)
- }
- }
- states.SetDeliveryService(deliveryServiceName, deliveryServiceState)
- }
-}
+// func calculateDeliveryServiceState(deliveryServiceServers map[enum.DeliveryServiceName][]enum.CacheName, states peer.CRStatesThreadsafe) {
--- End diff --
Can you delete this function since it's commented out?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #250: CrStates now correctly updates t...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/250#discussion_r99379255
--- Diff: traffic_monitor_golang/traffic_monitor/deliveryservice/stat.go ---
@@ -241,8 +241,12 @@ func addDSPerSecStats(dsName enum.DeliveryServiceName, stat dsdata.Stat, lastSta
if dsErr != nil {
stat.CommonStats.IsAvailable.Value = false
stat.CommonStats.IsHealthy.Value = false
- stat.CommonStats.ErrorStr.Value = err.Error()
+ stat.CommonStats.ErrorStr.Value = dsErr.Error()
+
}
+ dsState, _ := states.GetDeliveryService(dsName)
--- End diff --
This should handle the DS not existing (at least log)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---