Closed Bug 734044 Opened 12 years ago Closed 12 years ago

Intermittent toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Test timed out.

Categories

(Toolkit :: Places, defect)

Product:
Toolkit
Component:
Places
Platform:
All
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla13

People

(Reporter: mak, Assigned: mak)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

patch v1.1
3.73 KB, patch
dietrich
: review+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=9906040&tree=Mozilla-Inbound

13591 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Add livemark should succeed
13592 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Add livemark should succeed
13593 INFO TEST-INFO | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Reloading livemarks with forceUpdate: false
13594 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Get livemark should succeed
13595 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Get livemark should succeed
13596 INFO TEST-INFO | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Reloading livemarks with forceUpdate: true
13597 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Get livemark should succeed
13598 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Get livemark should succeed
WARNING: 1 sort operation has occurred for the SQL statement '0xb58303d8'.  See https://developer.mozilla.org/En/Storage/Warnings details.: file ../../../storage/src/mozStoragePrivateHelpers.cpp, line 144
13599 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul | Test timed out.
Disabled on linux for now.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0416556da72d

Btw my suspect is that it just takes more than the 30s timeout, it re-enters later and causes other tests to fail.
Whiteboard: [orange] → [orange][leave open][disabled on linux]
Locally I found the test hangs on loading the last livemark for many seconds, waiting for the cache.  Using LOAD_BYPASS_CACHE instead of ALWAYS_VALIDATE for the channel, makes the test blazing fast, that makes me think would also help perf in real-life.
I'm not sure if there's any really valid reason to not use LOAD_BYPASS_CACHE here, the only bad thing would be if an add-on calls multiple forced reloads for the same livemark in a short time, that is quite unlikely.
Attached patch patch v1.0 (obsolete) — — Splinter Review 
I'm still collecting data from the Tryserver, btw would like feedback on using the BYPASS
Attachment #604110 - Flags: feedback?(mano)
Attachment #604110 - Flags: feedback?(dietrich)
I don't understand why loading from cache is so slow, and slower than force-reload (unless this is just because our cache is a pile of fail).
I had issues following all the async calls through the cache, there is a lot of indirection and enqueue something that enqueues something else that enqueues again. plus disk IO seems to put a limit (and I have a quite fast disk). This is extremely bad in debug mode, much better in opt. The difference bypassing the cache is outstanding though, like instant vs 5 seconds.
btw, this was almost permaorange in inbound due to timeouts (that means the test was taking more than 30 seconds), now waiting on try with the change, first result is green.
https://tbpl.mozilla.org/?tree=Try&rev=b3a4a1f72309
ugh, I'm crazy, I posted to try without re-enabling the test :(
Comment on attachment 604110 [details] [diff] [review]
patch v1.0

Review of attachment 604110 [details] [diff] [review]:
-----------------------------------------------------------------

f+ then - no reason to not make this change at this time. i'd like to be able to know more about *why* cache is so slow in this case, but that might just be chalked up to our existing known "cache is slow" bugs.
Attachment #604110 - Flags: feedback?(dietrich) → feedback+
Attached patch patch v1.1 — — Splinter Review 
Actually also re-enable the test.
These are proper results:
https://tbpl.mozilla.org/?tree=Try&rev=66f9bc0a359e
Attachment #604110 - Attachment is obsolete: true
Attachment #604235 - Flags: review?(dietrich)
Attachment #604110 - Flags: feedback?(mano)
Attachment #604235 - Flags: review?(dietrich) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dbf57eb4ed5
Whiteboard: [orange][leave open][disabled on linux] → [orange]
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/0dbf57eb4ed5
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: