Mark Wu has uploaded a new change for review.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Don't monitor the usage of '/var/run/vdsm' in diskStats
Engine raises a warning that free space in /var/run/vdsm is less than 1GB. But actually, '/var/run' is mounted as a tmpfs. By default, the maximum size is half of the total physical memory. The warning is misleading because user could find it still has a lot of free space on the disk filesystem. And 1GB shouldn't be a low threshold for memory. So we needn't monitor the usage of '/var/run/vdsm' in 'diskStats' since it could be covered by memory usage monitoring.
Bug-uri: https://bugzilla.redhat.com/show_bug.cgi?id=906788 Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Signed-off-by: Mark Wu wudxw@linux.vnet.ibm.com --- M vdsm/utils.py 1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/75/11675/1
diff --git a/vdsm/utils.py b/vdsm/utils.py index 35338a0..57c3616 100644 --- a/vdsm/utils.py +++ b/vdsm/utils.py @@ -231,8 +231,7 @@
Contains the sate of the host in the time of initialization. """ - MONITORED_PATHS = ['/tmp', '/var/log', '/var/log/core', - constants.P_VDSM_RUN] + MONITORED_PATHS = ['/tmp', '/var/log', '/var/log/core']
def _getDiskStats(self): d = {}
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1057/ (2/3)
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1092/ (3/3)
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/203/ (1/3)
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 1: Verified
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 1: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1057/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1092/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/203/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1798/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1747/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1747/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1798/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File lib/vdsm/utils.py Line 230: A sample of host-related statistics. Line 231: Line 232: Contains the sate of the host in the time of initialization. Line 233: """ Line 234: MONITORED_PATHS = ['/tmp', '/var/log', '/var/log/core'] On Fedora 18, /tmp is also tmpfs by default, if /var/run/vdsm is removed, /tmp should be removed as well.
Do we need to change Engine code after changing MONITORED_PATHS? Line 235: Line 236: def _getDiskStats(self): Line 237: d = {} Line 238: for p in self.MONITORED_PATHS:
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File lib/vdsm/utils.py Line 230: A sample of host-related statistics. Line 231: Line 232: Contains the sate of the host in the time of initialization. Line 233: """ Line 234: MONITORED_PATHS = ['/tmp', '/var/log', '/var/log/core'] According to the following engine code, it looks engine go through the items in 'diskStats' reported by vdsm. That means vdsm decides the monitored paths and engine just retrieve information from it. So we don't need change the engine code
http://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=blob;f=backend/manager/m...
http://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=blob;f=backend/manager/m...
For /tmp, you're right. But it's still a normal local disk filesystem on RHEL6. So I need figure out a way to cover different platforms. Line 235: Line 236: def _getDiskStats(self): Line 237: d = {} Line 238: for p in self.MONITORED_PATHS:
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File lib/vdsm/utils.py Line 230: A sample of host-related statistics. Line 231: Line 232: Contains the sate of the host in the time of initialization. Line 233: """ Line 234: MONITORED_PATHS = ['/tmp', '/var/log', '/var/log/core'] I think we can keep /tmp, /var/tmp and other directories, then filter out the directory which is on tmpfs. Line 235: Line 236: def _getDiskStats(self): Line 237: d = {} Line 238: for p in self.MONITORED_PATHS:
-- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Deepak C Shetty has posted comments on this change.
Change subject: Don't monitor the usage of '/var/run/vdsm' in diskStats ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File lib/vdsm/utils.py Line 230: A sample of host-related statistics. Line 231: Line 232: Contains the sate of the host in the time of initialization. Line 233: """ Line 234: MONITORED_PATHS = ['/tmp', '/var/log', '/var/log/core'] [dpkshetty@deepakcs-lx v7k]$ stat --file-system --format=%T /run tmpfs [dpkshetty@deepakcs-lx v7k]$ stat --file-system --format=%T /var/run tmpfs
The above can help to find the fstype. But python's os.stat doesn't provide an equivalent of the above. Line 235: Line 236: def _getDiskStats(self): Line 237: d = {} Line 238: for p in self.MONITORED_PATHS:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2532/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3339/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3422/ : SUCCESS
Mark Wu has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3: Verified
Zhou Zheng Sheng has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3: (1 inline comment)
.................................................... File vdsm/sampling.py Line 160: for p in self.MONITORED_PATHS: Line 161: rc, out, _ = utils.execCmd(['stat', '--file-system', '--format=%T', Line 162: p]) Line 163: if rc == 0 and out[0] == 'tmpfs': Line 164: continue I think this piece of code can be put in __init__():
self.MONITORED_PATHS = filter( lambda p: utils.execCmd( ['stat', '--file-system', '--format=%T', p]) != (0, ['tmpfs'], []), self.MONITORED_PATHS)
or
self.MONITORED_PATHS = [ p for p in MONITORED_PATHS if utils.execCmd( ['stat', '--file-system', '--format=%T', p]) != (0, ['tmpfs'], [])]
Then _getDiskStats() does not have to stat the file system every time. Line 165: Line 166: free = 0 Line 167: try: Line 168: stat = os.statvfs(p)
Mark Wu has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3: (1 inline comment)
.................................................... File vdsm/sampling.py Line 160: for p in self.MONITORED_PATHS: Line 161: rc, out, _ = utils.execCmd(['stat', '--file-system', '--format=%T', Line 162: p]) Line 163: if rc == 0 and out[0] == 'tmpfs': Line 164: continue Good point! But in current Host sampling code, it has the same effect of putting it into __init__ and _getDiskStats, because it create a new instance of HostSample in every sampling. You approach will show its advantadge after we refactor the host sampling code. Line 165: Line 166: free = 0 Line 167: try: Line 168: stat = os.statvfs(p)
Zhou Zheng Sheng has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3: Looks good to me, but someone else must approve
(1 inline comment)
Current solution is good.
.................................................... File vdsm/sampling.py Line 160: for p in self.MONITORED_PATHS: Line 161: rc, out, _ = utils.execCmd(['stat', '--file-system', '--format=%T', Line 162: p]) Line 163: if rc == 0 and out[0] == 'tmpfs': Line 164: continue So can we put this piece near the definition of MONITORED_PATHS ?
MONITORED_PATHS = [ p for p in ['/tmp', '/var/log', '/var/log/core', P_VDSM_RUN] if utils.execCmd( ['stat', '--file-system', '--format=%T', p]) != (0, ['tmpfs'], [])]
or
MONITORED_PATHS = filter( lambda p: utils.execCmd( ['stat', '--file-system', '--format=%T', p]) != (0, ['tmpfs'], []), ['/tmp', '/var/log', '/var/log/core', P_VDSM_RUN]) Line 165: Line 166: free = 0 Line 167: try: Line 168: stat = os.statvfs(p)
Dan Kenigsberg has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/sampling.py Line 160: for p in self.MONITORED_PATHS: Line 161: rc, out, _ = utils.execCmd(['stat', '--file-system', '--format=%T', Line 162: p]) Line 163: if rc == 0 and out[0] == 'tmpfs': Line 164: continue If we have to filter out the tmpfs-mounted paths, how about doing this with storage.mount.mount? I'd prefer to have a path->vfstype mapping function there.
But in a more general note - if we do not need to monitor these paths on Fedora 18-based nodes, we most probably do not need to monitor them AT all. I do think that we SHOULD monitor them, but lower the threshold on Engine. Line 165: Line 166: free = 0 Line 167: try: Line 168: stat = os.statvfs(p)
Itamar Heim has posted comments on this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Patch Set 3:
ping?
Mark Wu has abandoned this change.
Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats ......................................................................
Abandoned
It should be done on engine side
vdsm-patches@lists.fedorahosted.org