From e9d6289964139b370fbc4bbfb73b3462dc766752 Mon Sep 17 00:00:00 2001 From: Abin Shahab Date: Sat, 18 Oct 2014 22:01:33 +0000 Subject: [PATCH 1/4] Added /proc/diskstats Read disk io information from /proc/diskstats. This will allow the user who provides partition container hints to get partition-specific io (blkio provides io for the container, but at the disk device level). --- container/raw/handler.go | 30 ++++++++++++++++++-- fs/fs.go | 55 +++++++++++++++++++++++++++++++++++- fs/fs_test.go | 46 ++++++++++++++++++++++++++++++ fs/test_resources/diskstats | 52 ++++++++++++++++++++++++++++++++++ fs/types.go | 19 +++++++++++-- info/container.go | 56 ++++++++++++++++++++++++++++++++++++- 6 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 fs/fs_test.go create mode 100644 fs/test_resources/diskstats diff --git a/container/raw/handler.go b/container/raw/handler.go index 93869303..3b274ebb 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -188,7 +188,20 @@ func (self *rawContainerHandler) getFsStats(stats *info.ContainerStats) error { return err } for _, fs := range filesystems { - stats.Filesystem = append(stats.Filesystem, info.FsStats{fs.Device, fs.Capacity, fs.Capacity - fs.Free}) + stats.Filesystem = append(stats.Filesystem, + info.FsStats{fs.Device, fs.Capacity, fs.Capacity - fs.Free, + fs.DiskStats.ReadsCompleted, + fs.DiskStats.ReadsMerged, + fs.DiskStats.SectorsRead, + fs.DiskStats.ReadTime, + fs.DiskStats.WritesCompleted, + fs.DiskStats.WritesMerged, + fs.DiskStats.SectorsWritten, + fs.DiskStats.WriteTime, + fs.DiskStats.IOInProgress, + fs.DiskStats.IOTime, + fs.DiskStats.WeightedIOTime, + }) } } else if len(self.externalMounts) > 0 { var mountSet map[string]struct{} @@ -201,7 +214,20 @@ func (self *rawContainerHandler) getFsStats(stats *info.ContainerStats) error { return err } for _, fs := range filesystems { - stats.Filesystem = append(stats.Filesystem, info.FsStats{fs.Device, fs.Capacity, fs.Capacity - fs.Free}) + stats.Filesystem = append(stats.Filesystem, + info.FsStats{fs.Device, fs.Capacity, fs.Capacity - fs.Free, + fs.DiskStats.ReadsCompleted, + fs.DiskStats.ReadsMerged, + fs.DiskStats.SectorsRead, + fs.DiskStats.ReadTime, + fs.DiskStats.WritesCompleted, + fs.DiskStats.WritesMerged, + fs.DiskStats.SectorsWritten, + fs.DiskStats.WriteTime, + fs.DiskStats.IOInProgress, + fs.DiskStats.IOTime, + fs.DiskStats.WeightedIOTime, + }) } } return nil diff --git a/fs/fs.go b/fs/fs.go index db492ff6..a37ae482 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -10,8 +10,11 @@ package fs import "C" import ( + "bufio" "fmt" "os/exec" + "os" + "regexp" "strconv" "strings" "syscall" @@ -53,6 +56,10 @@ func NewFsInfo() (FsInfo, error) { func (self *RealFsInfo) GetFsInfoForPath(mountSet map[string]struct{}) ([]Fs, error) { filesystems := make([]Fs, 0) deviceSet := make(map[string]struct{}) + diskStatsMap, err := getDiskStatsMap("/proc/diskstats") + if err != nil { + return nil, err + } for device, partition := range self.partitions { _, hasMount := mountSet[partition.mountpoint] _, hasDevice := deviceSet[device] @@ -67,7 +74,7 @@ func (self *RealFsInfo) GetFsInfoForPath(mountSet map[string]struct{}) ([]Fs, er Major: uint(partition.major), Minor: uint(partition.minor), } - fs := Fs{deviceInfo, total, free} + fs := Fs{deviceInfo, total, free, diskStatsMap[device]} filesystems = append(filesystems, fs) } } @@ -75,6 +82,52 @@ func (self *RealFsInfo) GetFsInfoForPath(mountSet map[string]struct{}) ([]Fs, er return filesystems, nil } +func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { + file, err := os.Open(diskStatsFile) + if err != nil { + return nil, err + } + + defer file.Close() + + scanner := bufio.NewScanner(file) + diskStatsMap := make(map[string]DiskStats) + partitionRegex, _ := regexp.Compile("sd[a-z]\\d") + for scanner.Scan() { + line :=scanner.Text() + words := strings.Fields(line) + if !partitionRegex.MatchString(words[2]) { + continue + } + // 8 50 sdd2 40 0 280 223 7 0 22 108 0 330 330 + deviceName := "/dev/" + words[2] + wordLength := len(words) + var stats = make([]uint64,wordLength) + var error error + for i := 3; i < wordLength; i++ { + stats[i], error = strconv.ParseUint(words[i], 10, 64) + if error != nil { + return nil, error + } + } + diskStats := DiskStats { + ReadsCompleted: stats[3], + ReadsMerged: stats[4], + SectorsRead: stats[5], + ReadTime: stats[6], + WritesCompleted:stats[7], + WritesMerged: stats[8], + SectorsWritten: stats[9], + WriteTime: stats[10], + IOInProgress: stats[11], + IOTime: stats[12], + WeightedIOTime: stats[13], + } + diskStatsMap[deviceName] = diskStats + } + return diskStatsMap, nil +} + func (self *RealFsInfo) GetGlobalFsInfo() ([]Fs, error) { return self.GetFsInfoForPath(nil) } diff --git a/fs/fs_test.go b/fs/fs_test.go new file mode 100644 index 00000000..715c15ed --- /dev/null +++ b/fs/fs_test.go @@ -0,0 +1,46 @@ +package fs +import ( + "testing" +) + +func TestGetDiskStatsMap(t *testing.T) { + diskStatsMap, err := getDiskStatsMap("test_resources/diskstats") + if err != nil { + t.Errorf("Error calling getDiskStatMap %s", err) + } + if len(diskStatsMap) != 20 { + t.Errorf("diskStatsMap %s not valid", diskStatsMap) + } + keySet := map[string]string{ + "/dev/sdb1": "/dev/sdb1", + "/dev/sdb2": "/dev/sdb2", + "/dev/sda1": "/dev/sda1", + "/dev/sda2": "/dev/sda2", + "/dev/sdc1": "/dev/sdc1", + "/dev/sdc2": "/dev/sdc2", + "/dev/sdc3": "/dev/sdc3", + "/dev/sdc4": "/dev/sdc4", + "/dev/sdd1": "/dev/sdd1", + "/dev/sdd2": "/dev/sdd2", + "/dev/sdd3": "/dev/sdd3", + "/dev/sdd4": "/dev/sdd4", + "/dev/sde1": "/dev/sde1", + "/dev/sde2": "/dev/sde2", + "/dev/sdf1": "/dev/sdf1", + "/dev/sdf2": "/dev/sdf2", + "/dev/sdg1": "/dev/sdg1", + "/dev/sdg2": "/dev/sdg2", + "/dev/sdh1": "/dev/sdh1", + "/dev/sdh2": "/dev/sdh2", + } + + for device := range diskStatsMap { + if _, ok := keySet[device]; !ok { + t.Errorf("Cannot find device %s", device) + } + delete(keySet, device) + } + if len(keySet) != 0 { + t.Errorf("diskStatsMap %s contains illegal keys %s", diskStatsMap, keySet) + } +} diff --git a/fs/test_resources/diskstats b/fs/test_resources/diskstats new file mode 100644 index 00000000..3981bdf5 --- /dev/null +++ b/fs/test_resources/diskstats @@ -0,0 +1,52 @@ + 1 0 ram0 0 0 0 0 0 0 0 0 0 0 0 + 1 1 ram1 0 0 0 0 0 0 0 0 0 0 0 + 1 2 ram2 0 0 0 0 0 0 0 0 0 0 0 + 1 3 ram3 0 0 0 0 0 0 0 0 0 0 0 + 1 4 ram4 0 0 0 0 0 0 0 0 0 0 0 + 1 5 ram5 0 0 0 0 0 0 0 0 0 0 0 + 1 6 ram6 0 0 0 0 0 0 0 0 0 0 0 + 1 7 ram7 0 0 0 0 0 0 0 0 0 0 0 + 1 8 ram8 0 0 0 0 0 0 0 0 0 0 0 + 1 9 ram9 0 0 0 0 0 0 0 0 0 0 0 + 1 10 ram10 0 0 0 0 0 0 0 0 0 0 0 + 1 11 ram11 0 0 0 0 0 0 0 0 0 0 0 + 1 12 ram12 0 0 0 0 0 0 0 0 0 0 0 + 1 13 ram13 0 0 0 0 0 0 0 0 0 0 0 + 1 14 ram14 0 0 0 0 0 0 0 0 0 0 0 + 1 15 ram15 0 0 0 0 0 0 0 0 0 0 0 + 7 0 loop0 0 0 0 0 0 0 0 0 0 0 0 + 7 1 loop1 0 0 0 0 0 0 0 0 0 0 0 + 7 2 loop2 0 0 0 0 0 0 0 0 0 0 0 + 7 3 loop3 0 0 0 0 0 0 0 0 0 0 0 + 7 4 loop4 0 0 0 0 0 0 0 0 0 0 0 + 7 5 loop5 0 0 0 0 0 0 0 0 0 0 0 + 7 6 loop6 0 0 0 0 0 0 0 0 0 0 0 + 7 7 loop7 0 0 0 0 0 0 0 0 0 0 0 + 8 16 sdb 931 1157 7601 960 2 0 16 0 0 919 960 + 8 17 sdb1 477 1147 3895 271 1 0 8 0 0 271 271 + 8 18 sdb2 395 0 3154 326 1 0 8 0 0 326 326 + 8 0 sda 931 1157 7601 1065 2 0 16 0 0 873 1065 + 8 1 sda1 477 1147 3895 419 1 0 8 0 0 419 419 + 8 2 sda2 395 0 3154 328 1 0 8 0 0 328 328 + 8 32 sdc 12390 470 457965 36363 72184 244851 9824537 5359169 0 607738 5437210 + 8 33 sdc1 10907 221 446193 34366 72173 244851 9824499 5359063 0 606972 5435214 + 8 34 sdc2 650 249 5120 901 7 0 22 93 0 956 994 + 8 35 sdc3 264 0 2106 380 1 0 8 0 0 380 380 + 8 36 sdc4 392 0 3130 476 1 0 8 0 0 475 475 + 8 48 sdd 3371 134 58909 18327 73997 243043 9824537 4532714 0 594248 4602162 + 8 49 sdd1 2498 134 51977 17192 73986 243043 9824499 4532600 0 593618 4600885 + 8 50 sdd2 40 0 280 223 7 0 22 108 0 330 330 + 8 51 sdd3 264 0 2106 328 1 0 8 0 0 328 328 + 8 52 sdd4 392 0 3130 373 1 0 8 1 0 374 374 + 8 64 sde 931 1157 7601 768 2 0 16 0 0 632 768 + 8 65 sde1 477 1147 3895 252 1 0 8 0 0 252 252 + 8 66 sde2 395 0 3154 281 1 0 8 0 0 281 281 + 8 80 sdf 931 1157 7601 936 2 0 16 0 0 717 936 + 8 81 sdf1 477 1147 3895 382 1 0 8 0 0 382 382 + 8 82 sdf2 395 0 3154 321 1 0 8 0 0 321 321 + 8 96 sdg 931 1157 7601 858 2 0 16 0 0 804 858 + 8 97 sdg1 477 1147 3895 244 1 0 8 0 0 244 244 + 8 98 sdg2 395 0 3154 299 1 0 8 0 0 299 299 + 8 112 sdh 931 1157 7601 895 2 0 16 0 0 841 895 + 8 113 sdh1 477 1147 3895 264 1 0 8 0 0 264 264 + 8 114 sdh2 395 0 3154 311 1 0 8 0 0 311 311 \ No newline at end of file diff --git a/fs/types.go b/fs/types.go index 3d54ecb6..1ff9e4d5 100644 --- a/fs/types.go +++ b/fs/types.go @@ -8,8 +8,23 @@ type DeviceInfo struct { type Fs struct { DeviceInfo - Capacity uint64 - Free uint64 + Capacity uint64 + Free uint64 + DiskStats DiskStats +} + +type DiskStats struct { + ReadsCompleted uint64 + ReadsMerged uint64 + SectorsRead uint64 + ReadTime uint64 + WritesCompleted uint64 + WritesMerged uint64 + SectorsWritten uint64 + WriteTime uint64 + IOInProgress uint64 + IOTime uint64 + WeightedIOTime uint64 } type FsInfo interface { diff --git a/info/container.go b/info/container.go index 5eaacb59..43322bb0 100644 --- a/info/container.go +++ b/info/container.go @@ -218,7 +218,7 @@ type NetworkStats struct { // Cumulative count of packets received. RxPackets uint64 `json:"rx_packets"` // Cumulative count of receive errors encountered. - RxErrors uint64 `json:"rx_errors"` + RxErrors uint64 `json:"rx_errors"` // Cumulative count of packets dropped while receiving. RxDropped uint64 `json:"rx_dropped"` // Cumulative count of bytes transmitted. @@ -240,6 +240,60 @@ type FsStats struct { // Number of bytes that is consumed by the container on this filesystem. Usage uint64 `json:"usage"` + + // # of reads completed + // This is the total number of reads completed successfully. + ReadsCompleted uint64 `json:"reads_completed"` + + // # of reads merged + // Reads and writes which are adjacent to each other may be merged for + // efficiency. Thus two 4K reads may become one 8K read before it is + // ultimately handed to the disk, and so it will be counted (and queued) + // as only one I/O. This field lets you know how often this was done. + ReadsMerged uint64 `json:"reads_merged"` + + // # of sectors read + // This is the total number of sectors read successfully. + SectorsRead uint64 `json:"sectors_read"` + + // # of milliseconds spent reading + // This is the total number of milliseconds spent by all reads (as + // measured from __make_request() to end_that_request_last()). + ReadTime uint64 `json:"read_time"` + + // # of writes completed + // This is the total number of writes completed successfully. + WritesCompleted uint64 `json:"writes_completed"` + + // # of writes merged + // See the description of reads merged. + WritesMerged uint64 `json:"writes_merged"` + + // # of sectors written + // This is the total number of sectors written successfully. + SectorsWritten uint64 `json:"sectors_written"` + + // # of milliseconds spent writing + // This is the total number of milliseconds spent by all writes (as + // measured from __make_request() to end_that_request_last()). + WriteTime uint64 `json:"write_time"` + + // # of I/Os currently in progress + // The only field that should go to zero. Incremented as requests are + // given to appropriate struct request_queue and decremented as they finish. + IOInProgress uint64 `json:"io_in_progress"` + + // # of milliseconds spent doing I/Os + // This field increases so long as field 9 is nonzero. + IOTime uint64 `json:"io_time"` + + // weighted # of milliseconds spent doing I/Os + // This field is incremented at each I/O start, I/O completion, I/O + // merge, or read of these stats by the number of I/Os in progress + // (field 9) times the number of milliseconds spent doing I/O since the + // last update of this field. This can provide an easy measure of both + // I/O completion time and the backlog that may be accumulating. + WeightedIOTime uint64 `json:"weighted_io_time"` } type ContainerStats struct { From e8ea485a0db8e550f05d6ed08515974ace41adf2 Mon Sep 17 00:00:00 2001 From: Abin Shahab Date: Sun, 19 Oct 2014 23:39:04 +0000 Subject: [PATCH 2/4] Gofmt all files --- container/raw/handler.go | 4 ++-- fs/fs.go | 32 +++++++++++++++---------------- fs/fs_test.go | 41 ++++++++++++++++++++-------------------- fs/types.go | 28 +++++++++++++-------------- info/container.go | 24 +++++++++++------------ 5 files changed, 65 insertions(+), 64 deletions(-) diff --git a/container/raw/handler.go b/container/raw/handler.go index 3b274ebb..955e804b 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -238,9 +238,9 @@ func (self *rawContainerHandler) GetStats() (*info.ContainerStats, error) { if self.networkInterface != nil { state = dockerlibcontainer.State{ NetworkState: network.NetworkState{ - VethHost: self.networkInterface.VethHost, + VethHost: self.networkInterface.VethHost, VethChild: self.networkInterface.VethChild, - NsPath: "unknown", + NsPath: "unknown", }, } } diff --git a/fs/fs.go b/fs/fs.go index a37ae482..3de66d21 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -12,8 +12,8 @@ import "C" import ( "bufio" "fmt" - "os/exec" "os" + "os/exec" "regexp" "strconv" "strings" @@ -63,7 +63,7 @@ func (self *RealFsInfo) GetFsInfoForPath(mountSet map[string]struct{}) ([]Fs, er for device, partition := range self.partitions { _, hasMount := mountSet[partition.mountpoint] _, hasDevice := deviceSet[device] - if mountSet == nil || hasMount && !hasDevice { + if mountSet == nil || hasMount && !hasDevice { total, free, err := getVfsStats(partition.mountpoint) if err != nil { glog.Errorf("Statvfs failed. Error: %v", err) @@ -94,7 +94,7 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { diskStatsMap := make(map[string]DiskStats) partitionRegex, _ := regexp.Compile("sd[a-z]\\d") for scanner.Scan() { - line :=scanner.Text() + line := scanner.Text() words := strings.Fields(line) if !partitionRegex.MatchString(words[2]) { continue @@ -102,7 +102,7 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { // 8 50 sdd2 40 0 280 223 7 0 22 108 0 330 330 deviceName := "/dev/" + words[2] wordLength := len(words) - var stats = make([]uint64,wordLength) + var stats = make([]uint64, wordLength) var error error for i := 3; i < wordLength; i++ { stats[i], error = strconv.ParseUint(words[i], 10, 64) @@ -110,18 +110,18 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { return nil, error } } - diskStats := DiskStats { - ReadsCompleted: stats[3], - ReadsMerged: stats[4], - SectorsRead: stats[5], - ReadTime: stats[6], - WritesCompleted:stats[7], - WritesMerged: stats[8], - SectorsWritten: stats[9], - WriteTime: stats[10], - IOInProgress: stats[11], - IOTime: stats[12], - WeightedIOTime: stats[13], + diskStats := DiskStats{ + ReadsCompleted: stats[3], + ReadsMerged: stats[4], + SectorsRead: stats[5], + ReadTime: stats[6], + WritesCompleted: stats[7], + WritesMerged: stats[8], + SectorsWritten: stats[9], + WriteTime: stats[10], + IOInProgress: stats[11], + IOTime: stats[12], + WeightedIOTime: stats[13], } diskStatsMap[deviceName] = diskStats } diff --git a/fs/fs_test.go b/fs/fs_test.go index 715c15ed..27664b2c 100644 --- a/fs/fs_test.go +++ b/fs/fs_test.go @@ -1,4 +1,5 @@ package fs + import ( "testing" ) @@ -12,26 +13,26 @@ func TestGetDiskStatsMap(t *testing.T) { t.Errorf("diskStatsMap %s not valid", diskStatsMap) } keySet := map[string]string{ - "/dev/sdb1": "/dev/sdb1", - "/dev/sdb2": "/dev/sdb2", - "/dev/sda1": "/dev/sda1", - "/dev/sda2": "/dev/sda2", - "/dev/sdc1": "/dev/sdc1", - "/dev/sdc2": "/dev/sdc2", - "/dev/sdc3": "/dev/sdc3", - "/dev/sdc4": "/dev/sdc4", - "/dev/sdd1": "/dev/sdd1", - "/dev/sdd2": "/dev/sdd2", - "/dev/sdd3": "/dev/sdd3", - "/dev/sdd4": "/dev/sdd4", - "/dev/sde1": "/dev/sde1", - "/dev/sde2": "/dev/sde2", - "/dev/sdf1": "/dev/sdf1", - "/dev/sdf2": "/dev/sdf2", - "/dev/sdg1": "/dev/sdg1", - "/dev/sdg2": "/dev/sdg2", - "/dev/sdh1": "/dev/sdh1", - "/dev/sdh2": "/dev/sdh2", + "/dev/sdb1": "/dev/sdb1", + "/dev/sdb2": "/dev/sdb2", + "/dev/sda1": "/dev/sda1", + "/dev/sda2": "/dev/sda2", + "/dev/sdc1": "/dev/sdc1", + "/dev/sdc2": "/dev/sdc2", + "/dev/sdc3": "/dev/sdc3", + "/dev/sdc4": "/dev/sdc4", + "/dev/sdd1": "/dev/sdd1", + "/dev/sdd2": "/dev/sdd2", + "/dev/sdd3": "/dev/sdd3", + "/dev/sdd4": "/dev/sdd4", + "/dev/sde1": "/dev/sde1", + "/dev/sde2": "/dev/sde2", + "/dev/sdf1": "/dev/sdf1", + "/dev/sdf2": "/dev/sdf2", + "/dev/sdg1": "/dev/sdg1", + "/dev/sdg2": "/dev/sdg2", + "/dev/sdh1": "/dev/sdh1", + "/dev/sdh2": "/dev/sdh2", } for device := range diskStatsMap { diff --git a/fs/types.go b/fs/types.go index 1ff9e4d5..3b454407 100644 --- a/fs/types.go +++ b/fs/types.go @@ -8,23 +8,23 @@ type DeviceInfo struct { type Fs struct { DeviceInfo - Capacity uint64 - Free uint64 - DiskStats DiskStats + Capacity uint64 + Free uint64 + DiskStats DiskStats } type DiskStats struct { - ReadsCompleted uint64 - ReadsMerged uint64 - SectorsRead uint64 - ReadTime uint64 - WritesCompleted uint64 - WritesMerged uint64 - SectorsWritten uint64 - WriteTime uint64 - IOInProgress uint64 - IOTime uint64 - WeightedIOTime uint64 + ReadsCompleted uint64 + ReadsMerged uint64 + SectorsRead uint64 + ReadTime uint64 + WritesCompleted uint64 + WritesMerged uint64 + SectorsWritten uint64 + WriteTime uint64 + IOInProgress uint64 + IOTime uint64 + WeightedIOTime uint64 } type FsInfo interface { diff --git a/info/container.go b/info/container.go index 43322bb0..bbf3d2a7 100644 --- a/info/container.go +++ b/info/container.go @@ -218,7 +218,7 @@ type NetworkStats struct { // Cumulative count of packets received. RxPackets uint64 `json:"rx_packets"` // Cumulative count of receive errors encountered. - RxErrors uint64 `json:"rx_errors"` + RxErrors uint64 `json:"rx_errors"` // Cumulative count of packets dropped while receiving. RxDropped uint64 `json:"rx_dropped"` // Cumulative count of bytes transmitted. @@ -243,49 +243,49 @@ type FsStats struct { // # of reads completed // This is the total number of reads completed successfully. - ReadsCompleted uint64 `json:"reads_completed"` + ReadsCompleted uint64 `json:"reads_completed"` // # of reads merged // Reads and writes which are adjacent to each other may be merged for // efficiency. Thus two 4K reads may become one 8K read before it is // ultimately handed to the disk, and so it will be counted (and queued) // as only one I/O. This field lets you know how often this was done. - ReadsMerged uint64 `json:"reads_merged"` + ReadsMerged uint64 `json:"reads_merged"` // # of sectors read // This is the total number of sectors read successfully. - SectorsRead uint64 `json:"sectors_read"` + SectorsRead uint64 `json:"sectors_read"` // # of milliseconds spent reading // This is the total number of milliseconds spent by all reads (as // measured from __make_request() to end_that_request_last()). - ReadTime uint64 `json:"read_time"` + ReadTime uint64 `json:"read_time"` // # of writes completed // This is the total number of writes completed successfully. - WritesCompleted uint64 `json:"writes_completed"` + WritesCompleted uint64 `json:"writes_completed"` // # of writes merged // See the description of reads merged. - WritesMerged uint64 `json:"writes_merged"` + WritesMerged uint64 `json:"writes_merged"` // # of sectors written // This is the total number of sectors written successfully. - SectorsWritten uint64 `json:"sectors_written"` + SectorsWritten uint64 `json:"sectors_written"` // # of milliseconds spent writing // This is the total number of milliseconds spent by all writes (as // measured from __make_request() to end_that_request_last()). - WriteTime uint64 `json:"write_time"` + WriteTime uint64 `json:"write_time"` // # of I/Os currently in progress // The only field that should go to zero. Incremented as requests are // given to appropriate struct request_queue and decremented as they finish. - IOInProgress uint64 `json:"io_in_progress"` + IOInProgress uint64 `json:"io_in_progress"` // # of milliseconds spent doing I/Os // This field increases so long as field 9 is nonzero. - IOTime uint64 `json:"io_time"` + IOTime uint64 `json:"io_time"` // weighted # of milliseconds spent doing I/Os // This field is incremented at each I/O start, I/O completion, I/O @@ -293,7 +293,7 @@ type FsStats struct { // (field 9) times the number of milliseconds spent doing I/O since the // last update of this field. This can provide an easy measure of both // I/O completion time and the backlog that may be accumulating. - WeightedIOTime uint64 `json:"weighted_io_time"` + WeightedIOTime uint64 `json:"weighted_io_time"` } type ContainerStats struct { From 48129c03d1dd6b3ac8dbfdc48cc6763bc23d0065 Mon Sep 17 00:00:00 2001 From: Abin Shahab Date: Mon, 20 Oct 2014 02:08:54 +0000 Subject: [PATCH 3/4] code review fixes --- container/raw/container_hints_test.go | 5 +-- container/raw/handler.go | 12 +++---- fs/fs.go | 46 ++++++++++++++++----------- fs/fs_test.go | 7 ++++ fs/types.go | 6 ++-- info/container.go | 28 ++++++++-------- 6 files changed, 59 insertions(+), 45 deletions(-) diff --git a/container/raw/container_hints_test.go b/container/raw/container_hints_test.go index 4d29b3b4..50ceb340 100644 --- a/container/raw/container_hints_test.go +++ b/container/raw/container_hints_test.go @@ -36,11 +36,8 @@ func TestGetContainerHintsFromFile(t *testing.T) { } func TestFileNotExist(t *testing.T) { - cHints, err := getContainerHintsFromFile("/file_does_not_exist.json") + _, err := getContainerHintsFromFile("/file_does_not_exist.json") if err != nil { t.Fatalf("getContainerHintsFromFile must not error for blank file: %s", err) } - for _, container := range cHints.AllHosts { - t.Logf("Container: %s", container) - } } diff --git a/container/raw/handler.go b/container/raw/handler.go index 955e804b..a06f1470 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -198,9 +198,9 @@ func (self *rawContainerHandler) getFsStats(stats *info.ContainerStats) error { fs.DiskStats.WritesMerged, fs.DiskStats.SectorsWritten, fs.DiskStats.WriteTime, - fs.DiskStats.IOInProgress, - fs.DiskStats.IOTime, - fs.DiskStats.WeightedIOTime, + fs.DiskStats.IoInProgress, + fs.DiskStats.IoTime, + fs.DiskStats.WeightedIoTime, }) } } else if len(self.externalMounts) > 0 { @@ -224,9 +224,9 @@ func (self *rawContainerHandler) getFsStats(stats *info.ContainerStats) error { fs.DiskStats.WritesMerged, fs.DiskStats.SectorsWritten, fs.DiskStats.WriteTime, - fs.DiskStats.IOInProgress, - fs.DiskStats.IOTime, - fs.DiskStats.WeightedIOTime, + fs.DiskStats.IoInProgress, + fs.DiskStats.IoTime, + fs.DiskStats.WeightedIoTime, }) } } diff --git a/fs/fs.go b/fs/fs.go index 3de66d21..d3a439c2 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -12,6 +12,7 @@ import "C" import ( "bufio" "fmt" + "path" "os" "os/exec" "regexp" @@ -24,6 +25,7 @@ import ( "github.com/golang/glog" ) +var partitionRegex = regexp.MustCompile("sd[a-z]+\\d") type partition struct { mountpoint string major uint @@ -83,16 +85,20 @@ func (self *RealFsInfo) GetFsInfoForPath(mountSet map[string]struct{}) ([]Fs, er } func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { + diskStatsMap := make(map[string]DiskStats) file, err := os.Open(diskStatsFile) if err != nil { + if os.IsNotExist(err) { + glog.Infof("not collecting filesystem statistics because file %q was not available", diskStatsFile) + return diskStatsMap, nil + } return nil, err } defer file.Close() - scanner := bufio.NewScanner(file) - diskStatsMap := make(map[string]DiskStats) - partitionRegex, _ := regexp.Compile("sd[a-z]\\d") + + for scanner.Scan() { line := scanner.Text() words := strings.Fields(line) @@ -100,28 +106,32 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { continue } // 8 50 sdd2 40 0 280 223 7 0 22 108 0 330 330 - deviceName := "/dev/" + words[2] + deviceName := path.Join("/dev", words[2]) wordLength := len(words) - var stats = make([]uint64, wordLength) + offset := 3 + var stats = make([]uint64, wordLength - offset) + if len(stats) < 11 { + return nil, fmt.Errorf("could not parse all 11 columns of /proc/diskstats") + } var error error - for i := 3; i < wordLength; i++ { - stats[i], error = strconv.ParseUint(words[i], 10, 64) + for i := offset; i < wordLength; i++ { + stats[i - offset], error = strconv.ParseUint(words[i], 10, 64) if error != nil { return nil, error } } diskStats := DiskStats{ - ReadsCompleted: stats[3], - ReadsMerged: stats[4], - SectorsRead: stats[5], - ReadTime: stats[6], - WritesCompleted: stats[7], - WritesMerged: stats[8], - SectorsWritten: stats[9], - WriteTime: stats[10], - IOInProgress: stats[11], - IOTime: stats[12], - WeightedIOTime: stats[13], + ReadsCompleted: stats[0], + ReadsMerged: stats[1], + SectorsRead: stats[2], + ReadTime: stats[3], + WritesCompleted: stats[4], + WritesMerged: stats[5], + SectorsWritten: stats[6], + WriteTime: stats[7], + IoInProgress: stats[8], + IoTime: stats[9], + WeightedIoTime: stats[10], } diskStatsMap[deviceName] = diskStats } diff --git a/fs/fs_test.go b/fs/fs_test.go index 27664b2c..3000c8af 100644 --- a/fs/fs_test.go +++ b/fs/fs_test.go @@ -45,3 +45,10 @@ func TestGetDiskStatsMap(t *testing.T) { t.Errorf("diskStatsMap %s contains illegal keys %s", diskStatsMap, keySet) } } + +func TestFileNotExist(t *testing.T) { + _, err := getDiskStatsMap("/file_does_not_exist") + if err != nil { + t.Fatalf("getDiskStatsMap must not error for absent file: %s", err) + } +} diff --git a/fs/types.go b/fs/types.go index 3b454407..5e250d7b 100644 --- a/fs/types.go +++ b/fs/types.go @@ -22,9 +22,9 @@ type DiskStats struct { WritesMerged uint64 SectorsWritten uint64 WriteTime uint64 - IOInProgress uint64 - IOTime uint64 - WeightedIOTime uint64 + IoInProgress uint64 + IoTime uint64 + WeightedIoTime uint64 } type FsInfo interface { diff --git a/info/container.go b/info/container.go index bbf3d2a7..a48a6fdd 100644 --- a/info/container.go +++ b/info/container.go @@ -241,59 +241,59 @@ type FsStats struct { // Number of bytes that is consumed by the container on this filesystem. Usage uint64 `json:"usage"` - // # of reads completed + // Number of reads completed // This is the total number of reads completed successfully. ReadsCompleted uint64 `json:"reads_completed"` - // # of reads merged + // Number of reads merged // Reads and writes which are adjacent to each other may be merged for // efficiency. Thus two 4K reads may become one 8K read before it is // ultimately handed to the disk, and so it will be counted (and queued) // as only one I/O. This field lets you know how often this was done. ReadsMerged uint64 `json:"reads_merged"` - // # of sectors read + // Number of sectors read // This is the total number of sectors read successfully. SectorsRead uint64 `json:"sectors_read"` - // # of milliseconds spent reading + // Number of milliseconds spent reading // This is the total number of milliseconds spent by all reads (as // measured from __make_request() to end_that_request_last()). ReadTime uint64 `json:"read_time"` - // # of writes completed + // Number of writes completed // This is the total number of writes completed successfully. WritesCompleted uint64 `json:"writes_completed"` - // # of writes merged + // Number of writes merged // See the description of reads merged. WritesMerged uint64 `json:"writes_merged"` - // # of sectors written + // Number of sectors written // This is the total number of sectors written successfully. SectorsWritten uint64 `json:"sectors_written"` - // # of milliseconds spent writing + // Number of milliseconds spent writing // This is the total number of milliseconds spent by all writes (as // measured from __make_request() to end_that_request_last()). WriteTime uint64 `json:"write_time"` - // # of I/Os currently in progress + // Number of I/Os currently in progress // The only field that should go to zero. Incremented as requests are // given to appropriate struct request_queue and decremented as they finish. - IOInProgress uint64 `json:"io_in_progress"` + IoInProgress uint64 `json:"io_in_progress"` - // # of milliseconds spent doing I/Os + // Number of milliseconds spent doing I/Os // This field increases so long as field 9 is nonzero. - IOTime uint64 `json:"io_time"` + IoTime uint64 `json:"io_time"` - // weighted # of milliseconds spent doing I/Os + // weighted number of milliseconds spent doing I/Os // This field is incremented at each I/O start, I/O completion, I/O // merge, or read of these stats by the number of I/Os in progress // (field 9) times the number of milliseconds spent doing I/O since the // last update of this field. This can provide an easy measure of both // I/O completion time and the backlog that may be accumulating. - WeightedIOTime uint64 `json:"weighted_io_time"` + WeightedIoTime uint64 `json:"weighted_io_time"` } type ContainerStats struct { From 7133ab0f7d60196ca848ddf2c1c9f06a5d4062c6 Mon Sep 17 00:00:00 2001 From: Abin Shahab Date: Tue, 21 Oct 2014 05:23:23 +0000 Subject: [PATCH 4/4] gofmt all files --- container/raw/container_hints_test.go | 2 +- fs/fs.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/container/raw/container_hints_test.go b/container/raw/container_hints_test.go index 50ceb340..6fa03475 100644 --- a/container/raw/container_hints_test.go +++ b/container/raw/container_hints_test.go @@ -27,7 +27,7 @@ func TestGetContainerHintsFromFile(t *testing.T) { if len(cHints.AllHosts[0].Mounts) == 0 { t.Errorf("Cannot find any mounts") } - + for i, mountDir := range cHints.AllHosts[0].Mounts { if correctMountDirs[i] != mountDir.HostDir { t.Errorf("Cannot find mount %s in %s", mountDir.HostDir, cHints) diff --git a/fs/fs.go b/fs/fs.go index d3a439c2..f1330035 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -12,9 +12,9 @@ import "C" import ( "bufio" "fmt" - "path" "os" "os/exec" + "path" "regexp" "strconv" "strings" @@ -26,6 +26,7 @@ import ( ) var partitionRegex = regexp.MustCompile("sd[a-z]+\\d") + type partition struct { mountpoint string major uint @@ -98,7 +99,6 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { defer file.Close() scanner := bufio.NewScanner(file) - for scanner.Scan() { line := scanner.Text() words := strings.Fields(line) @@ -109,13 +109,13 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) { deviceName := path.Join("/dev", words[2]) wordLength := len(words) offset := 3 - var stats = make([]uint64, wordLength - offset) + var stats = make([]uint64, wordLength-offset) if len(stats) < 11 { return nil, fmt.Errorf("could not parse all 11 columns of /proc/diskstats") } var error error for i := offset; i < wordLength; i++ { - stats[i - offset], error = strconv.ParseUint(words[i], 10, 64) + stats[i-offset], error = strconv.ParseUint(words[i], 10, 64) if error != nil { return nil, error }