Merge pull request #201 from vishh/stats_rounding

Saturate negative memory stat values at '0'.

cgroups/fs/cpuacct.go

@@ -40,7 +40,7 @@ func (s *CpuacctGroup) GetStats(path string, stats *cgroups.Stats) error {
 		return err
 	}
 
-	totalUsage, err := getCgroupParamInt(path, "cpuacct.usage")
+	totalUsage, err := getCgroupParamUint(path, "cpuacct.usage")
 	if err != nil {
 		return err
 	}

cgroups/fs/memory.go

@@ -2,6 +2,7 @@ package fs
 
 import (
 	"bufio"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -66,25 +67,25 @@ func (s *MemoryGroup) GetStats(path string, stats *cgroups.Stats) error {
 	for sc.Scan() {
 		t, v, err := getCgroupParamKeyValue(sc.Text())
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to parse memory.stat (%q) - %v", sc.Text(), err)
 		}
 		stats.MemoryStats.Stats[t] = v
 	}
 
 	// Set memory usage and max historical usage.
-	value, err := getCgroupParamInt(path, "memory.usage_in_bytes")
+	value, err := getCgroupParamUint(path, "memory.usage_in_bytes")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to parse memory.usage_in_bytes - %v", err)
 	}
 	stats.MemoryStats.Usage = value
-	value, err = getCgroupParamInt(path, "memory.max_usage_in_bytes")
+	value, err = getCgroupParamUint(path, "memory.max_usage_in_bytes")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to parse memory.max_usage_in_bytes - %v", err)
 	}
 	stats.MemoryStats.MaxUsage = value
-	value, err = getCgroupParamInt(path, "memory.failcnt")
+	value, err = getCgroupParamUint(path, "memory.failcnt")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to parse memory.failcnt - %v", err)
 	}
 	stats.MemoryStats.Failcnt = value
 

cgroups/fs/utils.go

@@ -14,27 +14,49 @@ var (
 	ErrNotValidFormat = errors.New("line is not a valid key value format")
 )
 
+// Saturates negative values at zero and returns a uint64.
+// Due to kernel bugs, some of the memory cgroup stats can be negative.
+func parseUint(s string, base, bitSize int) (uint64, error) {
+	value, err := strconv.ParseUint(s, base, bitSize)
+	if err != nil {
+		intValue, intErr := strconv.ParseInt(s, base, bitSize)
+		// 1. Handle negative values greater than MinInt64 (and)
+		// 2. Handle negative values lesser than MinInt64
+		if intErr == nil && intValue < 0 {
+			return 0, nil
+		} else if intErr != nil && intErr.(*strconv.NumError).Err == strconv.ErrRange && intValue < 0 {
+			return 0, nil
+		}
+
+		return value, err
+	}
+
+	return value, nil
+}
+
 // Parses a cgroup param and returns as name, value
 //  i.e. "io_service_bytes 1234" will return as io_service_bytes, 1234
 func getCgroupParamKeyValue(t string) (string, uint64, error) {
 	parts := strings.Fields(t)
 	switch len(parts) {
 	case 2:
-		value, err := strconv.ParseUint(parts[1], 10, 64)
+		value, err := parseUint(parts[1], 10, 64)
 		if err != nil {
-			return "", 0, fmt.Errorf("Unable to convert param value to uint64: %s", err)
+			return "", 0, fmt.Errorf("Unable to convert param value (%q) to uint64: %v", parts[1], err)
 		}
+
 		return parts[0], value, nil
 	default:
 		return "", 0, ErrNotValidFormat
 	}
 }
 
-// Gets a single int64 value from the specified cgroup file.
-func getCgroupParamInt(cgroupPath, cgroupFile string) (uint64, error) {
+// Gets a single uint64 value from the specified cgroup file.
+func getCgroupParamUint(cgroupPath, cgroupFile string) (uint64, error) {
 	contents, err := ioutil.ReadFile(filepath.Join(cgroupPath, cgroupFile))
 	if err != nil {
 		return 0, err
 	}
-	return strconv.ParseUint(strings.TrimSpace(string(contents)), 10, 64)
+
+	return parseUint(strings.TrimSpace(string(contents)), 10, 64)
 }

cgroups/fs/utils_test.go

@@ -2,8 +2,10 @@ package fs
 
 import (
 	"io/ioutil"
+	"math"
 	"os"
 	"path/filepath"
+	"strconv"
 	"testing"
 )
 
@@ -27,7 +29,7 @@ func TestGetCgroupParamsInt(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	value, err := getCgroupParamInt(tempDir, cgroupFile)
+	value, err := getCgroupParamUint(tempDir, cgroupFile)
 	if err != nil {
 		t.Fatal(err)
 	} else if value != floatValue {
@@ -39,19 +41,44 @@ func TestGetCgroupParamsInt(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	value, err = getCgroupParamInt(tempDir, cgroupFile)
+	value, err = getCgroupParamUint(tempDir, cgroupFile)
 	if err != nil {
 		t.Fatal(err)
 	} else if value != floatValue {
 		t.Fatalf("Expected %d to equal %f", value, floatValue)
 	}
 
+	// Success with negative values
+	err = ioutil.WriteFile(tempFile, []byte("-12345"), 0755)
+	if err != nil {
+		t.Fatal(err)
+	}
+	value, err = getCgroupParamUint(tempDir, cgroupFile)
+	if err != nil {
+		t.Fatal(err)
+	} else if value != 0 {
+		t.Fatalf("Expected %d to equal %f", value, 0)
+	}
+
+	// Success with negative values lesser than min int64
+	s := strconv.FormatFloat(math.MinInt64, 'f', -1, 64)
+	err = ioutil.WriteFile(tempFile, []byte(s), 0755)
+	if err != nil {
+		t.Fatal(err)
+	}
+	value, err = getCgroupParamUint(tempDir, cgroupFile)
+	if err != nil {
+		t.Fatal(err)
+	} else if value != 0 {
+		t.Fatalf("Expected %d to equal %f", value, 0)
+	}
+
 	// Not a float.
 	err = ioutil.WriteFile(tempFile, []byte("not-a-float"), 0755)
 	if err != nil {
 		t.Fatal(err)
 	}
-	_, err = getCgroupParamInt(tempDir, cgroupFile)
+	_, err = getCgroupParamUint(tempDir, cgroupFile)
 	if err == nil {
 		t.Fatal("Expecting error, got none")
 	}
@@ -61,7 +88,7 @@ func TestGetCgroupParamsInt(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	_, err = getCgroupParamInt(tempDir, cgroupFile)
+	_, err = getCgroupParamUint(tempDir, cgroupFile)
 	if err == nil {
 		t.Fatal("Expecting error, got none")
 	}