User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release) Build ID: 20140513011231 Steps to reproduce: trunk + mozillabuild1.9 + vs2013u2 need to delete the built files in python/psutil everytime RuntimeError: couldn't find any physical disk File "c:\develop\mozilla\central\python/mozbuild/mozbuild/mach_commands.py", line 287, in build monitor.init(warnings_path) File "c:\develop\mozilla\central\python/mozbuild\mozbuild\controller\building.py", line 161, in init self.resources = SystemResourceMonitor(poll_interval=1.0) File "c:\develop\mozilla\central\testing/mozbase/mozsystemmonitor\mozsystemmonitor\resourcemonitor.py", line 179, in __init__ io = psutil.disk_io_counters() File "c:\develop\mozilla\central\python/psutil\psutil\__init__.py", line 1229, in disk_io_counters raise RuntimeError("couldn't find any physical disk")

I hit this on one of our Windows build machines as well while testing bug 978211. I manually ran 'diskperf -y' in a shell, and it seemed to make the problem go away. Maybe we want to catch this error and run that command automatically? I found the diskperf thing from here: https://code.google.com/p/psutil/issues/detail?id=351

I'd like to get this fixed, since we hit this when running mach in automation. One workaround is to get 'diskperf -y' in the build machines, but since it is possible for developers to hit it locally as well, we should fix it in tree. The attached patch makes the behavior go away, but is obviously wrong. Some questions: 1) I would think I'd need to wrap disk_io_counters() in _collect() as well, but _collect() doesn't get called. Any idea why it isn't? (And do we care on Windows?) 2) Should we automatically run 'diskperf -y' for the user if disk_io_counters() fails? Or just print a warning and not collect io stats? Note I can't reproduce this locally on Windows 7, since 'diskperf -n' seems to not actually disable the feature. However, on a build machine (Windows Server 2008), I can run 'diskperf -n' to see the error, or 'diskperf -y' to make it go away.

If we must run diskperf -y to make the exception go away, my vote is to wrap calls to disk_io_counters() and call diskperf -y (if available) to make it work. Alternatively, we could wrap the entire collector thread in a try..finally that saves and restores the diskperf setting. That being said, according to http://technet.microsoft.com/en-us/library/hh875645.aspx, the performance counters are only available after a restart. Is that not behavior in the real world? This sounds like something we may want to punt to a higher-level (bootstrap/MozillaBuild or machine provisioning).

Comment on attachment 8473662 [details] [diff] [review] disk_io.patch Review of attachment 8473662 [details] [diff] [review]: ----------------------------------------------------------------- I'm fine with a hack. But please test this first. I'd prefer the robust solution as documented in my earlier comment.

mshal: flipping on more, branches, can we land this fix even if it's not perfect?

gps isn't back yet, but I think this is what he's suggesting. I've tested it on a Windows builder - if diskperf is disabled, it will now automatically run and avoid the RuntimeError.

Comment on attachment 8496046 [details] [diff] [review] 0001-Bug-1013730-Automatically-run-diskperf-y-for-IO-stat.patch Review of attachment 8496046 [details] [diff] [review]: ----------------------------------------------------------------- I think it's safer to silently swallow the exception and not capture I/O than to enable disk perf as a possibly-unintended side-effect. Keep in mind this code runs on users' machines as part of regular Firefox builds. I'm not comfortable silently making changes to system settings like this.

(In reply to Gregory Szorc [:gps] from comment #7) > I think it's safer to silently swallow the exception and not capture I/O > than to enable disk perf as a possibly-unintended side-effect. > > Keep in mind this code runs on users' machines as part of regular Firefox > builds. I'm not comfortable silently making changes to system settings like > this. Oh, I thought you were suggesting to do it this way. Swallowing the exception sounds fine to me - I'll post a new patch.

Comment on attachment 8498292 [details] [diff] [review] 0001-Bug-1013730-Have-mach-ignore-broken-disk-io-stats.patch Review of attachment 8498292 [details] [diff] [review]: ----------------------------------------------------------------- This /might/ cause issues with downstream systems expecting there to be values here. But if we don't have test coverage, then it's a follow-up bug.

try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0cd7bb95ef4d

inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/ead2392a01f6

beta uplift for bug 1093897: https://hg.mozilla.org/releases/mozilla-beta/rev/daabf4d8995f