[autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py [chromiumos/third_party/autotest : master]
0 views
Skip to first unread message
Chris Masone (Code Review)
May 16, 2012, 2:50:15 PM5/16/12
to Scott Zawalski, Chris Sosa
Hello Scott Zawalski, Chris Sosa,
I'd like you to do a code review. Please visit
https://gerrit.chromium.org/gerrit/22839
to review the following change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
[autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
site_host.ChromiumOSHost implements a method that knows how to
install a new cros image on a device and do some checks to
make sure that it's not obviously bad. This has a timeout
that it's willing to wait for update_engine to mark the new
kernel as good. That timeout is currently only 15s longer
than the time that update_engine waits before even trying to
check on the kernel. We run afoul of this at times.
Extending the timeout costs us no correctness and reduces
flakiness. There's not a good way to determine reliably
when we should start waiting for this timeout, so this
method is the lesser of known evils.
BUG=chromium-os:30953
TEST=run_suite.py, which will reimage devices as a matter of course
Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
---
M server/hosts/site_host.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.chromium.org:29418/chromiumos/third_party/autotest refs/changes/39/22839/1
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Scott Zawalski <sco...@chromium.org>
I'd like you to do a code review. Please visit
https://gerrit.chromium.org/gerrit/22839
to review the following change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
[autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
site_host.ChromiumOSHost implements a method that knows how to
install a new cros image on a device and do some checks to
make sure that it's not obviously bad. This has a timeout
that it's willing to wait for update_engine to mark the new
kernel as good. That timeout is currently only 15s longer
than the time that update_engine waits before even trying to
check on the kernel. We run afoul of this at times.
Extending the timeout costs us no correctness and reduces
flakiness. There's not a good way to determine reliably
when we should start waiting for this timeout, so this
method is the lesser of known evils.
BUG=chromium-os:30953
TEST=run_suite.py, which will reimage devices as a matter of course
Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
---
M server/hosts/site_host.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.chromium.org:29418/chromiumos/third_party/autotest refs/changes/39/22839/1
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Scott Zawalski <sco...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 2:50:31 PM5/16/12
to Chris Sosa, Scott Zawalski
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Verified
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Scott Zawalski (Code Review)
May 16, 2012, 2:54:25 PM5/16/12
to Chris Masone, Chris Sosa
Scott Zawalski has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Looks good to me, approved
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 3:28:02 PM5/16/12
to Chris Sosa, Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1:
Gilad, does this timeout look reasonable?
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gilad Arnold <gar...@chromium.org>
Gerrit-Reviewer: Scott Zawalski <sco...@chromium.org>
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Gilad, does this timeout look reasonable?
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gilad Arnold <gar...@chromium.org>
Gerrit-Reviewer: Scott Zawalski <sco...@chromium.org>
Gilad Arnold (Code Review)
May 16, 2012, 3:48:52 PM5/16/12
to Chris Masone, Chris Sosa, Scott Zawalski
Gilad Arnold has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1:
When do we start counting? From reboot? From ssh login after reboot? Some other point in time?
I'd say that 90 seconds should be enough: (a) it's 45 seconds beyond the AU delay, which should suffice for booting up and the actual time it takes to mark the partition stable (about 17 seconds, based on the successful AU logs); (b) it's 30 seconds beyond the current timeout, which is plenty; (c) we don't want to permit too long of a wait before the new partition is marked stable, as this will increase the chances for updates not being successful.
What do you say? Otherwise, lgtm.
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
When do we start counting? From reboot? From ssh login after reboot? Some other point in time?
I'd say that 90 seconds should be enough: (a) it's 45 seconds beyond the AU delay, which should suffice for booting up and the actual time it takes to mark the partition stable (about 17 seconds, based on the successful AU logs); (b) it's 30 seconds beyond the current timeout, which is plenty; (c) we don't want to permit too long of a wait before the new partition is marked stable, as this will increase the chances for updates not being successful.
What do you say? Otherwise, lgtm.
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 3:52:52 PM5/16/12
to Chris Sosa, Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Ready
I think the 30 extra seconds isn't a big deal :-)
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
I think the 30 extra seconds isn't a big deal :-)
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 5:01:11 PM5/16/12
to Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Ready
chromeos-ec unittest failure. Again, not me.
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
chromeos-ec unittest failure. Again, not me.
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 5:03:12 PM5/16/12
to Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Ready
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 5:23:02 PM5/16/12
to Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Ready
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 6:44:28 PM5/16/12
to Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Ready
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gilad Arnold (Code Review)
May 16, 2012, 6:45:41 PM5/16/12
to Chris Masone, Scott Zawalski
Gilad Arnold has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1:
I'm telling you, 120 is too much... ;-)
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
I'm telling you, 120 is too much... ;-)
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Chris Masone (Code Review)
May 16, 2012, 7:03:56 PM5/16/12
to Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1:
Why? Why do we want to fail all test suites if it starts taking an extra 60 seconds to mark the kernel as ready? If we require a tighter bound, then by all means write a test that validates that behavior. We shouldn't break our ability to run any test at all if the timing of this regresses by a minute.
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Why? Why do we want to fail all test suites if it starts taking an extra 60 seconds to mark the kernel as ready? If we require a tighter bound, then by all means write a test that validates that behavior. We shouldn't break our ability to run any test at all if the timing of this regresses by a minute.
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Chris Masone (Code Review)
May 17, 2012, 12:39:14 AM5/17/12
to Scott Zawalski, Gilad Arnold
Chris Masone has posted comments on this change.
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
Patch Set 1: Ready
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: [autotest] Extend _KERNEL_UPDATE_TIMEOUT in site_host.py
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/22839
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Iec2d85d3412ccebaf8821c7f31ffeb9b8bdbf50e
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Reply all
Reply to author
Forward
0 new messages