[NEW PATCH] Related BZ#720385 - Report disk latency (read , write & flush ) for each storage device. (via gerrit-bot)
8:46 a.m.
New patch submitted by Igor Lvovsky (ilvovsky(a)redhat.com)
You can review this change at: http://gerrit.usersys.redhat.com/935
commit 21cfe4c8c08191908142fcfa98346f811fd90739
Author: Igor Lvovsky <ilvovsky(a)redhat.com>
Date: Wed Sep 14 16:37:36 2011 +0300
Related BZ#720385 - Report disk latency (read , write & flush ) for each storage
device.
This patch temporary return zeros instead of real latency values.
The real values will be returned when libvirt will support python binding for
virDomainQemuMonitor command.
Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
diff --git a/vdsm/config.py b/vdsm/config.py
index 217b58c..7ad29d9 100644
--- a/vdsm/config.py
+++ b/vdsm/config.py
@@ -80,6 +80,8 @@ config.set('vars', 'vm_sample_cpu_interval',
'15')
config.set('vars', 'vm_sample_cpu_window', '2')
config.set('vars', 'vm_sample_disk_interval', '60')
config.set('vars', 'vm_sample_disk_window', '2')
+config.set('vars', 'vm_sample_disk_latency_interval', '60')
+config.set('vars', 'vm_sample_disk_latency_window', '2')
config.set('vars', 'vm_sample_net_interval', '5')
config.set('vars', 'vm_sample_net_window', '2')
diff --git a/vdsm/libvirtvm.py b/vdsm/libvirtvm.py
index 221589b..4b1fb8b 100644
--- a/vdsm/libvirtvm.py
+++ b/vdsm/libvirtvm.py
@@ -56,12 +56,15 @@ class VmStatsThread(utils.AdvancedStatsThread):
self.sampleDisk = utils.AdvancedStatsFunction(self._sampleDisk,
config.getint('vars',
'vm_sample_disk_interval'),
config.getint('vars',
'vm_sample_disk_window'))
+ self.sampleDiskLatency = utils.AdvancedStatsFunction(self._sampleDiskLatency,
+ config.getint('vars',
'vm_sample_disk_latency_interval'),
+ config.getint('vars',
'vm_sample_disk_latency_window'))
self.sampleNet = utils.AdvancedStatsFunction(self._sampleNet,
config.getint('vars',
'vm_sample_net_interval'),
config.getint('vars',
'vm_sample_net_window'))
- self.addStatsFunction(self.highWrite, self.updateVolumes,
- self.sampleCpu, self.sampleDisk, self.sampleNet)
+ self.addStatsFunction(self.highWrite, self.updateVolumes, self.sampleCpu,
+ self.sampleDisk, self.sampleDiskLatency, self.sampleNet)
def _highWrite(self):
if self._vm._incomingMigrationPending():
@@ -107,6 +110,17 @@ class VmStatsThread(utils.AdvancedStatsThread):
diskSamples[vmDrive.name] = self._vm._dom.blockStats(vmDrive.name)
return diskSamples
+ def _sampleDiskLatency(self):
+ if not self._vm._volumesPrepared:
+ # Avoid queries from storage during recovery process
+ return
+
+ diskLatency = {}
+ for vmDrive in self._vm._drives:
+ # Will be done by virDomainQemuMonitor
+ diskLatency[vmDrive.name] = ()
+ return diskLatency
+
def _sampleNet(self):
netSamples = {}
for vmIface in self._vm.interfaces.keys():
@@ -175,6 +189,22 @@ class VmStatsThread(utils.AdvancedStatsThread):
stats[dName] = dStats
+ def _getDiskLatency(self, stats):
+ sInfo, eInfo, sampleInterval = self.sampleDiskLatency.getStats()
+
+ for vmDrive in self._vm._drives:
+ dName = vmDrive.name
+
+ dLatency = {'readLatency': '0.00',
+ 'writeLatency': '0.00',}
+ try:
+ dLatency['readLatency'] = '0.00'
+ dLatency['writeLatency'] = '0.00'
+ except (KeyError, TypeError):
+ self._log.debug("Disk latency not available")
+
+ stats[dName].update(dLatency)
+
def get(self):
stats = {}
@@ -187,6 +217,7 @@ class VmStatsThread(utils.AdvancedStatsThread):
self._getCpuStats(stats)
self._getNetworkStats(stats)
self._getDiskStats(stats)
+ self._getDiskLatency(stats)
return stats
10:57 a.m.
New subject: Change in vdsm[master]: BZ#720385 - Report disk latency (read , write & flush ) for ...
Dan Kenigsberg has submitted this change and it was merged.
Change subject: BZ#720385 - Report disk latency (read , write & flush ) for each
storage device.
......................................................................
BZ#720385 - Report disk latency (read , write & flush ) for each storage device.
Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
---
M vdsm.spec.in
M vdsm/config.py
M vdsm/libvirtvm.py
3 files changed, 100 insertions(+), 7 deletions(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Rami Vaknin <rvaknin(a)redhat.com>
10:57 a.m.
New subject: Change in vdsm[master]: BZ#720385 - Report disk latency (read , write & flush ) for ...
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#720385 - Report disk latency (read , write & flush ) for each
storage device.
......................................................................
Patch Set 5: Verified; Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Rami Vaknin <rvaknin(a)redhat.com>
9:35 a.m.
New subject: Change in vdsm[master]: Related BZ#720385 - Report disk latency (read , write & flus...
Federico Simoncelli has posted comments on this change.
Change subject: Related BZ#720385 - Report disk latency (read , write & flush ) for
each storage device.
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/libvirtvm.py
Line 204: dLatency['flushLatency'] = str(eInfo[dName][2])
I might be wrong but I believe they are converted to string later on before sending the
response.
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
9:41 a.m.
New subject: Change in vdsm[master]: Related BZ#720385 - Report disk latency (read , write & flus...
Dan Kenigsberg has posted comments on this change.
Change subject: Related BZ#720385 - Report disk latency (read , write & flush ) for
each storage device.
......................................................................
Patch Set 1: Verified; I would prefer that you didn't submit this
(2 inline comments)
let's report all three values. I've verified that the new values are reported per
Vm disk.
....................................................
File vdsm/libvirtvm.py
Line 121: diskLatency[vmDrive.name] = ()
I'd put 3 zeros here, so the rest of the code can be finalized.
Line 199: 'writeLatency': '0.00',}
we've been asked to report flushLatency, too.
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
12:15 p.m.
New subject: Change in vdsm[master]: BZ#720385 - Report disk latency (read , write & flush ) for ...
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#720385 - Report disk latency (read , write & flush ) for each
storage device.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 48:
Line 49: self.highWrite = utils.AdvancedStatsFunction(self._highWrite,
Line 50: config.getint('vars',
'vm_watermark_interval'))
Line 51: self.updateVolumes = utils.AdvancedStatsFunction(self._updateVolumes,
Line 52: config.getint('irs',
'vol_size_sample_interval'))
but I liked this whitespace!
Line 53: self.sampleCpu = utils.AdvancedStatsFunction(self._sampleCpu,
Line 54: config.getint('vars',
'vm_sample_cpu_interval'),
Line 55: config.getint('vars',
'vm_sample_cpu_window'))
Line 56: self.sampleDisk = utils.AdvancedStatsFunction(self._sampleDisk,
Line 129: return stats
Line 130:
Line 131: diskLatency = {}
Line 132: res = libvirt_qemu.qemuMonitorCommand(self._vm._dom, 'info
blockstats',
Line 133:
libvirt_qemu.VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP)
H stands for Human, which Vdsm is not... Why not use the programatical api, which should
be more robust? Instead of parsing `info blockstats` yourself, you'd ask json to do
it.
Line 134: stats = _blockstatsParses(res)
Line 135: for vmDrive in self._vm._drives:
Line 136: try:
Line 137: diskLatency[vmDrive.name] = stats[vmDrive.alias]
Line 135: for vmDrive in self._vm._drives:
Line 136: try:
Line 137: diskLatency[vmDrive.name] = stats[vmDrive.alias]
Line 138: except KeyError:
Line 139: diskLatency[vmDrive.name] = [0, 0, 0, 0, 0, 0, 0, 0]
a dictionary would make a much clearer internal interface.
Line 140: self._log.warn("Disk %s latency not available",
vmDrive.name)
Line 141:
Line 142: return diskLatency
Line 143:
Line 217: wr_total_time_ns, rd_total_time_ns, flush_total_time_ns = rawData
Line 218: try:
Line 219: readLatency = rd_total_time_ns / rd_op
Line 220: writeLatency = wr_total_time_ns / wr_op
Line 221: flushLatency = flush_total_time_ns / flush_op
this gives us average since VM creation, which is not very interesting, as local jumps
would be hidden. We should compare start sample to end sample.
(eInfo[read_ns] - sInfo[read_ns] ) / (eInfo[rd_op] - sInfo[rd_op] )
Line 222: except ZeroDivisionError:
Line 223: self._log.debug("Disk latency not available")
Line 224: readLatency, writeLatency, flushLatency = (0, 0, 0)
Line 225:
Line 981: raise Exception('destroy() called before Vm started')
Line 982: self._initInterfaces()
Line 983: self._getUnderlyingDriveInfo()
Line 984: self._getUnderlyingDisplayPort()
Line 985: # VmStatsThread may use block devices info from libvirt.
hey, this race is old!
Line 986: # So, run it after you have this info
Line 987: self._initVmStats()
Line 988: self.guestAgent = guestIF.GuestAgent(self._guestSocektFile, self.log,
Line 989: connect=utils.tobool(self.conf.get('vmchannel',
'true')))
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Rami Vaknin <rvaknin(a)redhat.com>
5:17 p.m.
New subject: Change in vdsm[master]: BZ#720385 - Report disk latency (read , write & flush ) for ...
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#720385 - Report disk latency (read , write & flush ) for each
storage device.
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 134: for item in devList['return']:
Line 135: fullDevName = item['device']
Line 136: alias = fullDevName[len('drive-'):].strip()
Line 137: devStats = item['stats']
Line 138: stats[alias] = dict(rd_op=devStats['rd_operations'],
{} is 3 times quicker than dict()
Line 139: wr_op=devStats['wr_operations'],
Line 140:
flush_op=devStats['flush_operations'],
Line 141:
rd_total_time_ns=devStats['rd_total_time_ns'],
Line 142:
wr_total_time_ns=devStats['wr_total_time_ns'],
Line 148: cmd = json.dumps({ "execute" : "query-blockstats"
})
Line 149: res = libvirt_qemu.qemuMonitorCommand(self._vm._dom, cmd,
Line 150:
libvirt_qemu.VIR_DOMAIN_QEMU_MONITOR_COMMAND_DEFAULT)
Line 151: out = json.loads(res)
Line 152:
evil whitespace
Line 153: stats = _blockstatsParses(out)
Line 154: for vmDrive in self._vm._drives:
Line 155: try:
Line 156: diskLatency[vmDrive.name] = stats[vmDrive.alias]
Line 232: stats[dName] = dStats
Line 233:
Line 234: def _getDiskLatency(self, stats):
Line 235: sInfo, eInfo, sampleInterval = self.sampleDiskLatency.getStats()
Line 236:
evil
Line 237: def _avgLatencyCalc(sData, eData):
Line 238: try:
Line 239: readLatency = (eData['rd_total_time_ns'] -
sData['rd_total_time_ns']) / \
Line 240: (eData['rd_op'] - sData['rd_op'])
Line 241: writeLatency = (eData['wr_total_time_ns'] -
sData['wr_total_time_ns']) / \
Line 242: (eData['wr_op'] -
sData['wr_op'])
Line 243: flushLatency = (eData['flush_total_time_ns'] -
sData['flush_total_time_ns']) / \
Line 244: (eData['flush_op'] -
sData['flush_op'])
Line 245: except ZeroDivisionError:
this should be per-value, since a guest may have 0 flushes in the last 10 seconds, but
several reads.
also please consider avoiding ZeroDivisionError,
readLatency = 0 if (eData['rd_op'] - sData['rd_op']) else
(eData['rd_total_time_ns'] - sData['rd_total_time_ns']) /
(eData['rd_op'] - sData['rd_op'])
it seems faster and cleaner to me.
Line 246: self._log.debug("Disk latency not available")
Line 247: readLatency, writeLatency, flushLatency = (0, 0, 0)
Line 248:
Line 249: return str(readLatency), str(writeLatency), str(flushLatency)
--
To view, visit http://gerrit.usersys.redhat.com/935
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0d3d4869bcf35ed4d2ff72a5e1ef7aa21fe3455
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Rami Vaknin <rvaknin(a)redhat.com>
4584
days inactive
4607
days old
vdsm-patches@lists.fedorahosted.org
6 comments
3 participants
participants (3)
-
Dan Kenigsberg -
Federico Simoncelli -
Igor Lvovsky