Permission denied trying to remove chrome-trace-config.json atexit |
||||
Issue descriptionA few builds on the N5 and N5X perf bots have been showing failures like: Error in atexit._run_exitfuncs: [...] File "/b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py", line 299, in _RemoveTraceConfigFile [...] AdbShellCommandFailedError: (device: 01d30cfd7bc7d319) shell command run via adb failed on the device: command: rm -f /data/local/chrome-trace-config.json exit status: 1 output: - rm: /data/local/chrome-trace-config.json: Permission denied https://logs.chromium.org/v/?s=chrome%2Fbb%2Fchromium.perf%2FAndroid_Nexus5X_Perf%2F855%2F%2B%2Frecipes%2Fsteps%2Fsystem_health.memory_mobile_on_Android%2F0%2Fstdout Not sure why this is failing but, at least, we should make errors in "atexit" be non-fatal (as we're exiting the script now, and this should just be code running on a best-effort basis). John, Ned, what do you think?
,
Nov 28 2017
I am bit worry about making this non-fatal. Wouldn't this mess with the state of the next test run on the same bot?
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/cbba42a8e6eb4e5f35b02a1e1611e515e7f5013f commit cbba42a8e6eb4e5f35b02a1e1611e515e7f5013f Author: Juan Antonio Navarro Perez <perezju@google.com> Date: Wed Nov 29 09:38:57 2017 [py_utils] Log and skip through atexit_with_log exceptions Code running during atexit should be considered running on a best-effort basis, so it's ok to skip through any exceptions (while still logging them). Bug: chromium:789119 Change-Id: I1451ef7f1e5b2d4e79bef3710499d1b6ada19f08 Reviewed-on: https://chromium-review.googlesource.com/793049 Reviewed-by: Ned Nguyen <nednguyen@google.com> Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org> [modify] https://crrev.com/cbba42a8e6eb4e5f35b02a1e1611e515e7f5013f/common/py_utils/py_utils/atexit_with_log.py
,
Nov 29 2017
Re #2: Yeah, I can see how that may be a problem. Fatal or non-fatal as we're exiting the step; the next step will run regardless from a possibly bad state. The only difference is whether the step itself is colored red or not. To be honest, I dislike the idea of "atexit" to begin with. We should be doing more context-managers and try-finally blocks instead. Anyway, about the root cause of this issue, and looking at device_utils.RemovePath [1], maybe the rename option would be helpful here? I'll give that a go. [1]: https://github.com/catapult-project/catapult/blob/11d7efb857ae77eff1cea4640e3f3d9ac49cba0a/devil/devil/android/device_utils.py#L1661
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/b0b1ce2c6e9001363a35c0ec943dc873d0b24d4a commit b0b1ce2c6e9001363a35c0ec943dc873d0b24d4a Author: Juan Antonio Navarro Perez <perezju@google.com> Date: Wed Nov 29 10:58:14 2017 [Telemetry] Speculative fix for removing trace config file Rather than device.RunShellCommand(['rm', ...], ...), use the more specific device.RemovePath(...) method. We also set rename=True to hopefully avoid filesystem errors while trying to remove the file. Bug: chromium:789119 Change-Id: I52f4f45a04c472a497dac04d9e72c3b4e1283b7d Reviewed-on: https://chromium-review.googlesource.com/795952 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org> [modify] https://crrev.com/b0b1ce2c6e9001363a35c0ec943dc873d0b24d4a/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
,
Nov 30 2017
This appears to be fixed.
,
Jan 16
(6 days ago)
,
Jan 16
(6 days ago)
|
||||
►
Sign in to add a comment |
||||
Comment 1 by jbudorick@chromium.org
, Nov 28 2017