You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by rs...@apache.org on 2023/06/29 22:46:00 UTC

[trafficcontrol] branch revert-TM-changes-for-bandwidth-bug created (now 3ae5fda3f0)

This is an automated email from the ASF dual-hosted git repository.

rshah pushed a change to branch revert-TM-changes-for-bandwidth-bug
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


      at 3ae5fda3f0 Revert "Use SOH timestamp to calculate bandwidth in TM (#7539)"

This branch includes the following new commits:

     new 8deee08b36 Revert "TM - Plugin Systems Stats Timestamp for SOH (#7551)"
     new 3ae5fda3f0 Revert "Use SOH timestamp to calculate bandwidth in TM (#7539)"

The 2 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.



[trafficcontrol] 01/02: Revert "TM - Plugin Systems Stats Timestamp for SOH (#7551)"

Posted by rs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rshah pushed a commit to branch revert-TM-changes-for-bandwidth-bug
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git

commit 8deee08b3681d4889587f4a9c6c44c9a1782f36f
Author: Rima Shah <ri...@comcast.com>
AuthorDate: Thu Jun 29 16:44:18 2023 -0600

    Revert "TM - Plugin Systems Stats Timestamp for SOH (#7551)"
    
    This reverts commit a1bd413864feef5a7b87228cc80f087cf1680812.
---
 traffic_monitor/cache/cache.go             | 2 +-
 traffic_monitor/cache/cache_test.go        | 8 ++++----
 traffic_monitor/cache/stats_over_http.json | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/traffic_monitor/cache/cache.go b/traffic_monitor/cache/cache.go
index 9bedd3494f..4d443c4088 100644
--- a/traffic_monitor/cache/cache.go
+++ b/traffic_monitor/cache/cache.go
@@ -307,7 +307,7 @@ func (handler Handler) Handle(id string, rdr io.Reader, format string, reqTime t
 		handler.resultChan <- result
 		return
 	}
-	if val, ok := miscStats["plugin.system_stats.timestamp_ms"]; ok {
+	if val, ok := miscStats["current_time_epoch_ms"]; ok {
 		valString := fmt.Sprintf("%s", val)
 		valInt, valErr := strconv.ParseInt(valString, 10, 64)
 		if valErr != nil {
diff --git a/traffic_monitor/cache/cache_test.go b/traffic_monitor/cache/cache_test.go
index 93ed80b687..077592ffca 100644
--- a/traffic_monitor/cache/cache_test.go
+++ b/traffic_monitor/cache/cache_test.go
@@ -124,12 +124,12 @@ func TestParseAndDecode(t *testing.T) {
 		t.Errorf("empty miscStats structure")
 	}
 
-	if val, ok := miscStats["plugin.system_stats.timestamp_ms"]; ok {
+	if val, ok := miscStats["current_time_epoch_ms"]; ok {
 		valString := fmt.Sprintf("%s", val)
-		if valString != "1684784877939" {
-			t.Errorf("unable to read `plugin.system_stats.timestamp_ms`")
+		if valString != "1684784878894" {
+			t.Errorf("unable to read `current_time_epoch_ms`")
 		}
 	} else {
-		t.Errorf("plugin.system_stats.timestamp_ms field was not found in the json file")
+		t.Errorf("current_time_epoch_ms field was not found in the json file")
 	}
 }
diff --git a/traffic_monitor/cache/stats_over_http.json b/traffic_monitor/cache/stats_over_http.json
index 979a2bac72..a7d8ed4b20 100644
--- a/traffic_monitor/cache/stats_over_http.json
+++ b/traffic_monitor/cache/stats_over_http.json
@@ -1,6 +1,5 @@
 {
 	"global": {
-		"plugin.system_stats.timestamp_ms": "1684784877939",
 		"proxy.process.http.completed_requests": 26220072200,
 		"proxy.process.http.total_incoming_connections": 770802777,
 		"proxy.process.http.total_client_connections": 770802777,
@@ -534,6 +533,7 @@
 		"plugin.system_stats.net.docker0.rx_length_errors": "0",
 		"proxy.process.cache.volume_0.span.offline": "0",
 		"proxy.process.cache.volume_0.span.online": "0",
+		"current_time_epoch_ms": "1684784878894",
 		"server": "10.0.0"
 	}
 }


[trafficcontrol] 02/02: Revert "Use SOH timestamp to calculate bandwidth in TM (#7539)"

Posted by rs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rshah pushed a commit to branch revert-TM-changes-for-bandwidth-bug
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git

commit 3ae5fda3f0131ae61ae2bf2202550940a3075846
Author: Rima Shah <ri...@comcast.com>
AuthorDate: Thu Jun 29 16:45:50 2023 -0600

    Revert "Use SOH timestamp to calculate bandwidth in TM (#7539)"
    
    This reverts commit f3c124f6
---
 CHANGELOG.md                               |  1 -
 traffic_monitor/cache/cache.go             | 12 ---------
 traffic_monitor/cache/cache_test.go        | 42 ++----------------------------
 traffic_monitor/cache/stats_over_http.json |  1 -
 traffic_monitor/health/cache.go            | 11 ++------
 traffic_monitor/health/cache_test.go       |  8 ------
 6 files changed, 4 insertions(+), 71 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 877d2b5d0e..b82cec39ad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -66,7 +66,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7570](https://github.com/apache/trafficcontrol/pull/7570) *Traffic Ops* Fixes `deliveryservice_request_comments` v5 apis to respond with `RFC3339` date/time Format.
 - [#7312](https://github.com/apache/trafficcontrol/issues/7312) *Docs* Changing docs for CDN locks for DELETE response structure v4 and v5. 
 - [#7572](https://github.com/apache/trafficcontrol/pull/7572) *Traffic Ops* Fixes Delivery Service Requests V5 apis docs with RFC3339 date/time Format
-- [#7539](https://github.com/apache/trafficcontrol/pull/7539) *Traffic Monitor* Use stats_over_http timestamp to calculate bandwidth for TM's health. 
 - [#7542](https://github.com/apache/trafficcontrol/pull/7542) *Traffic Ops* Fixed `CDN Locks` documentation to reflect the correct RFC3339 timestamps.
 - [#6340](https://github.com/apache/trafficcontrol/issues/6340) *Traffic Ops* Fixed alert messages for POST and PUT invalidation job APIs.
 - [#7519] (https://github.com/apache/trafficcontrol/issues/7519) *Traffic Ops* Fixed TO API /servers/{id}/deliveryservices endpoint to responding with all DS's on cache that are directly assigned and inherited through topology.
diff --git a/traffic_monitor/cache/cache.go b/traffic_monitor/cache/cache.go
index 4d443c4088..339c4a5f5c 100644
--- a/traffic_monitor/cache/cache.go
+++ b/traffic_monitor/cache/cache.go
@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"io"
 	"regexp"
-	"strconv"
 	"time"
 
 	"github.com/apache/trafficcontrol/lib/go-log"
@@ -307,17 +306,6 @@ func (handler Handler) Handle(id string, rdr io.Reader, format string, reqTime t
 		handler.resultChan <- result
 		return
 	}
-	if val, ok := miscStats["current_time_epoch_ms"]; ok {
-		valString := fmt.Sprintf("%s", val)
-		valInt, valErr := strconv.ParseInt(valString, 10, 64)
-		if valErr != nil {
-			log.Errorf("parse error '%v'", valErr)
-			result.Error = valErr
-			handler.resultChan <- result
-			return
-		}
-		result.Time = time.UnixMilli(valInt)
-	}
 	if value, ok := miscStats[rfc.Via]; ok {
 		result.ID = fmt.Sprintf("%v", value)
 	}
diff --git a/traffic_monitor/cache/cache_test.go b/traffic_monitor/cache/cache_test.go
index 077592ffca..b20a392507 100644
--- a/traffic_monitor/cache/cache_test.go
+++ b/traffic_monitor/cache/cache_test.go
@@ -20,15 +20,11 @@ package cache
  */
 
 import (
-	"bytes"
-	"fmt"
+	"testing"
+
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
-	"github.com/apache/trafficcontrol/traffic_monitor/poller"
 	"github.com/apache/trafficcontrol/traffic_monitor/todata"
-	"io/ioutil"
-	"net/http"
-	"testing"
 )
 
 func TestHandlerPrecompute(t *testing.T) {
@@ -99,37 +95,3 @@ func TestComputeStatGbps(t *testing.T) {
 		}
 	}
 }
-
-func TestParseAndDecode(t *testing.T) {
-	file, err := ioutil.ReadFile("stats_over_http.json")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	pl := &poller.HTTPPollCtx{HTTPHeader: http.Header{}}
-	ctx := interface{}(pl)
-	ctx.(*poller.HTTPPollCtx).HTTPHeader.Set("Content-Type", "text/json")
-
-	decoder, err := GetDecoder("stats_over_http")
-	if err != nil {
-		t.Errorf("decoder error, expected: nil, got: %v", err)
-	}
-
-	_, miscStats, err := decoder.Parse("1", bytes.NewReader(file), ctx)
-	if err != nil {
-		t.Errorf("decoder parse error, expected: nil, got: %v", err)
-	}
-
-	if len(miscStats) < 1 {
-		t.Errorf("empty miscStats structure")
-	}
-
-	if val, ok := miscStats["current_time_epoch_ms"]; ok {
-		valString := fmt.Sprintf("%s", val)
-		if valString != "1684784878894" {
-			t.Errorf("unable to read `current_time_epoch_ms`")
-		}
-	} else {
-		t.Errorf("current_time_epoch_ms field was not found in the json file")
-	}
-}
diff --git a/traffic_monitor/cache/stats_over_http.json b/traffic_monitor/cache/stats_over_http.json
index a7d8ed4b20..70cce951b4 100644
--- a/traffic_monitor/cache/stats_over_http.json
+++ b/traffic_monitor/cache/stats_over_http.json
@@ -533,7 +533,6 @@
 		"plugin.system_stats.net.docker0.rx_length_errors": "0",
 		"proxy.process.cache.volume_0.span.offline": "0",
 		"proxy.process.cache.volume_0.span.online": "0",
-		"current_time_epoch_ms": "1684784878894",
 		"server": "10.0.0"
 	}
 }
diff --git a/traffic_monitor/health/cache.go b/traffic_monitor/health/cache.go
index 53f932285c..94489884db 100644
--- a/traffic_monitor/health/cache.go
+++ b/traffic_monitor/health/cache.go
@@ -60,7 +60,6 @@ func (t Threshold) String() string {
 
 // GetVitals Gets the vitals to decide health on in the right format
 func GetVitals(newResult *cache.Result, prevResult *cache.Result, mc *tc.TrafficMonitorConfigMap) {
-	var elapsedTimeInSecs float64
 	if newResult.Error != nil {
 		log.Errorf("cache_health.GetVitals() called with an errored Result!")
 		return
@@ -88,14 +87,6 @@ func GetVitals(newResult *cache.Result, prevResult *cache.Result, mc *tc.Traffic
 		return
 	}
 
-	if prevResult != nil {
-		elapsedTimeInSecs = float64(newResult.Time.UnixMilli()-prevResult.Time.UnixMilli()) / 1000
-		if elapsedTimeInSecs <= 0 {
-			*newResult = *prevResult
-			return
-		}
-	}
-
 	var monitoredInterfaces []tc.ServerInterfaceInfo
 	for _, srvrIfaceInfo := range mc.TrafficServer[newResult.ID].Interfaces {
 		if srvrIfaceInfo.Monitor {
@@ -124,6 +115,7 @@ func GetVitals(newResult *cache.Result, prevResult *cache.Result, mc *tc.Traffic
 		}
 
 		if prevResult != nil && prevResult.InterfaceVitals != nil && prevResult.InterfaceVitals[ifaceName].BytesOut != 0 {
+			elapsedTimeInSecs := float64(newResult.Time.UnixNano()-prevResult.Time.UnixNano()) / 1000000000
 			ifaceVitals.KbpsOut = int64(float64((ifaceVitals.BytesOut-prevResult.InterfaceVitals[ifaceName].BytesOut)*8/1000) / elapsedTimeInSecs)
 		}
 		newResult.InterfaceVitals[ifaceName] = ifaceVitals
@@ -136,6 +128,7 @@ func GetVitals(newResult *cache.Result, prevResult *cache.Result, mc *tc.Traffic
 	}
 
 	if prevResult != nil && prevResult.Vitals.BytesOut != 0 {
+		elapsedTimeInSecs := float64(newResult.Time.UnixNano()-prevResult.Time.UnixNano()) / 1000000000
 		newResult.Vitals.KbpsOut = int64(float64((newResult.Vitals.BytesOut-prevResult.Vitals.BytesOut)*8/1000) / elapsedTimeInSecs)
 	}
 
diff --git a/traffic_monitor/health/cache_test.go b/traffic_monitor/health/cache_test.go
index 3b3c151eb5..847d82931a 100644
--- a/traffic_monitor/health/cache_test.go
+++ b/traffic_monitor/health/cache_test.go
@@ -310,14 +310,6 @@ func TestDualHomingMonitoredInterfacesGetVitals(t *testing.T) {
 	if firstResult.Vitals != expectedFirstVitals {
 		t.Errorf("Vitals do not match expected output. expected: %v actual: %v:", expectedFirstVitals, firstResult.Vitals)
 	}
-
-	//Test if elapsedTimeInSecs == 0
-	secondResult.Time = firstResult.Time
-	GetVitals(&secondResult, &firstResult, &tmcm)
-	if firstResult.Statistics.Interfaces["bond0"] != secondResult.Statistics.Interfaces["bond0"] {
-		t.Errorf("Load avg statistics do not match. expected: %v, got: %v", firstResult.Statistics.Interfaces["bond0"], secondResult.Statistics.Interfaces["bond0"])
-	}
-
 }
 
 func TestCalcAvailabilityThresholds(t *testing.T) {