New issue
Advanced search Search tips

Issue 839886 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

"ForegroundedOrClosedTest.SingleTab" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, May 4 2018

Issue description

"ForegroundedOrClosedTest.SingleTab" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 6 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLQsSBUZsYWtlIiJGb3JlZ3JvdW5kZWRPckNsb3NlZFRlc3QuU2luZ2xlVGFiDA.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 
Cc: fdoray@chromium.org alexilin@chromium.org
Owner: michae...@chromium.org
Status: Assigned (was: Untriaged)
Hey michaelpg@ I'm assigning this to you as you added the test in question.

I'm CCing fodoray@ as the owner of chrome/browser/resource_coordinator, as well as alexilin@ an owner of chrome/browser/predictors which shows up in the error stack trace.

It appears that the flakes began around May 3rd.

With what appears to be a disk I/O error:
FATAL:connection.cc(1903)] disk I/O error
#0 0x00000593fc1c base::debug::StackTrace::StackTrace()
#1 0x0000058bcacb logging::LogMessage::~LogMessage()
#2 0x0000063ee289 sql::Connection::OnSqliteError()
#3 0x0000063eb6a9 sql::Connection::Execute()
#4 0x0000063eb47a sql::Connection::SetMmapAltStatus()
#5 0x0000063ec17c sql::Connection::GetAppropriateMmapSize()
#6 0x0000063ea652 sql::Connection::OpenInternal()
#7 0x0000063e9c80 sql::Connection::Open()
#8 0x000005c650c5 predictors::PredictorDatabaseInternal::Initialize()
#9 0x0000014389ee _ZN4base8internal7InvokerINS0_9BindStateIMN10extensions12image_writer9OperationEFvvEJ13scoped_refptrINS4_26DestroyPartitionsOperationEEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#10 0x0000058a9845 base::debug::TaskAnnotator::RunTask()
#11 0x0000059071c5 base::internal::TaskTracker::RunOrSkipTask()
#12 0x00000594f3c8 base::internal::TaskTrackerPosix::RunOrSkipTask()
#13 0x000004f47e85 base::test::ScopedTaskEnvironment::TestTaskTracker::RunOrSkipTask()
#14 0x00000590665f base::internal::TaskTracker::RunAndPopNextTask()
#15 0x00000596dba8 base::internal::SchedulerWorker::ThreadMain()
#16 0x00000594fa6f base::(anonymous namespace)::ThreadFunc()
#17 0x7f9c0e200184 start_thread
#18 0x7f9c0af3603d clone

I'll put together a patch to disable the test on chrome os until a cause can be found
 Issue 839916  has been merged into this issue.
Cc: -alexilin@chromium.org michae...@chromium.org
Owner: alexilin@chromium.org
Assigning to alexilin since michaelpg@ is traveling. 
Talked to trooper martiniss@ about potential infra issue that could cause this. They checked it out and said they guess that low disk space could be an issue but it seems fine. 

Project Member

Comment 6 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/65c948ac50c3c53d902716212057b48c5a32b102

commit 65c948ac50c3c53d902716212057b48c5a32b102
Author: jonross <jonross@chromium.org>
Date: Fri May 04 17:59:58 2018

Disable Flaky ForegroundedOrClosedTest.SingleTab test

ForegroundedOrClosedTest.SingleTab has started to flake on ChromeOS. This
disables the test until the cause can be found.

TEST=ForegroundedOrClosedTest.SingleTab
TBR=fdoray@chromium.org

Bug:  839886 
Change-Id: I650bc975a9e4795218eb205a628d3c604f7fcdcf
Reviewed-on: https://chromium-review.googlesource.com/1044533
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556112}
[modify] https://crrev.com/65c948ac50c3c53d902716212057b48c5a32b102/chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc

Cc: pasko@chromium.org pwnall@chromium.org
There are several tests being flaky because of a disk I/O error in the PredictorsDatabaseInternal. We've started seeing them right after I enabled this feature by default: https://crrev.com/c/1042388.

It'd be better to disable the feature rather than disabling all flaky tests. I've sent a CL: https://chromium-review.googlesource.com/c/chromium/src/+/1044326

We need to figure out why disk I/O is failing on Chrome OS test devices. I see similar issues with another sqlite database: https://crbug.com/839371, https://crbug.com/836332

https://crbug.com/839186 may be related. pwnall@ do you think these are real disk I/O errors? What could we do to reduce flakiness?
Project Member

Comment 8 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc89d6bebc0eb8d95e8aa72b4d9852020d4257da

commit cc89d6bebc0eb8d95e8aa72b4d9852020d4257da
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri May 04 18:39:45 2018

Revert "predictors: Enable the new preconnect predictor by default"

This reverts commit 5f1039f1025b5294133baab17a578d45c4431063.

Reason for revert: multiple tests on Chrome OS became flaky because of disk I/O errors.

Original change's description:
> predictors: Enable the new preconnect predictor by default
> 
> The old predictor is automatically disabled when the new one is enabled.
> 
> Change-Id: Ia5c1a79dbeb0e096720fc3d50da3dc707b3761a6
> Reviewed-on: https://chromium-review.googlesource.com/1042388
> Reviewed-by: Egor Pasko <pasko@chromium.org>
> Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#555766}

TBR=pasko@chromium.org,alexilin@chromium.org
Bug:  839886 

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I8c286f29cee249990c57486851842e4d9dd900ac
Reviewed-on: https://chromium-review.googlesource.com/1044326
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556130}
[modify] https://crrev.com/cc89d6bebc0eb8d95e8aa72b4d9852020d4257da/chrome/browser/predictors/loading_predictor_config.cc
[modify] https://crrev.com/cc89d6bebc0eb8d95e8aa72b4d9852020d4257da/chrome/browser/predictors/loading_predictor_config_unittest.cc

Comment 9 Deleted

 Issue 839929  has been merged into this issue.
Cc: charleszhao@chromium.org
If these test failures were due to I/O flakes, let's re-enable the test next week.
ToolbarActionsBarUnitTest also failed due to disk I/O error in the same period of time. ( Issue 840077 )
Labels: -Sheriff-Chromium
Removing Sheriff-Chromium since the flakiness seems to be addressed by the revert in #8.
Cc: -michae...@chromium.org -fdoray@chromium.org
Removing myself and michaelpg@ from cc, given that we are suspecting limited disk space rather than an error in chrome/browser/resource_coordinator/ code.
Project Member

Comment 15 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e78b52df303aadc23b4b9303dcb829d66aac7b9

commit 6e78b52df303aadc23b4b9303dcb829d66aac7b9
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed May 16 01:50:17 2018

Reland "predictors: Enable the new preconnect predictor by default"

This reverts commit cc89d6bebc0eb8d95e8aa72b4d9852020d4257da.

Reason for revert: disk I/O errors were observed on other sqlite
databases on Chrome OS, so the issue is not in the predictors code.
Enabling the feature for all platforms but Chrome OS until database
flakes are fixed.

Original change's description:
> Revert "predictors: Enable the new preconnect predictor by default"
>
> This reverts commit 5f1039f1025b5294133baab17a578d45c4431063.
>
> Reason for revert: multiple tests on Chrome OS became flaky because of disk I/O errors.
>
> Original change's description:
> > predictors: Enable the new preconnect predictor by default
> >
> > The old predictor is automatically disabled when the new one is enabled.
> >
> > Change-Id: Ia5c1a79dbeb0e096720fc3d50da3dc707b3761a6
> > Reviewed-on: https://chromium-review.googlesource.com/1042388
> > Reviewed-by: Egor Pasko <pasko@chromium.org>
> > Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#555766}
>
> TBR=pasko@chromium.org,alexilin@chromium.org
> Bug:  839886 
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Change-Id: I8c286f29cee249990c57486851842e4d9dd900ac
> Reviewed-on: https://chromium-review.googlesource.com/1044326
> Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
> Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556130}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  839886 
Change-Id: Ib7e2070c546cdf443e511fc9ae72e14eb5bd520f
Reviewed-on: https://chromium-review.googlesource.com/1059189
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558920}
[modify] https://crrev.com/6e78b52df303aadc23b4b9303dcb829d66aac7b9/chrome/browser/predictors/loading_predictor_config.cc
[modify] https://crrev.com/6e78b52df303aadc23b4b9303dcb829d66aac7b9/chrome/browser/predictors/loading_predictor_config_unittest.cc

I discovered that this error is caused by the database directory being destroyed before the database opens. It happens because of race condition in testing code. Actually, all platforms are affected but I don't know why the problem manifests itself only on ChromeOS.

I've created a discussion thread on chromium-dev:
https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!topic/chromium-dev/-oH0n_O9Tjc
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c950d6a3071d83341203d509646ae13c95926e47

commit c950d6a3071d83341203d509646ae13c95926e47
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Mon Jun 11 13:13:11 2018

predictors: Don't initialize database after shutdown

PredictorDatabaseInternal::Initialize() can be called after the shutdown of
PredictorDatabase. In testing scenario it could lead to accessing a database
file in a directory that doesn't exist, because profile directories are deleted
after test.

This CL checks a cancellation flag that is set on PredictorDatabase shutdown
before opening a database file.

This CL also enables the predictor by default on ChromeOS since the disk I/O
issue should be resolved.

Bug:  839886 
Change-Id: I27fbdda2cac282a44bd1bac962b27bf11de8520b
Reviewed-on: https://chromium-review.googlesource.com/1092851
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565983}
[modify] https://crrev.com/c950d6a3071d83341203d509646ae13c95926e47/chrome/browser/predictors/loading_predictor_config.cc
[modify] https://crrev.com/c950d6a3071d83341203d509646ae13c95926e47/chrome/browser/predictors/predictor_database.cc
[modify] https://crrev.com/c950d6a3071d83341203d509646ae13c95926e47/chrome/browser/predictors/predictor_table_base.cc
[modify] https://crrev.com/c950d6a3071d83341203d509646ae13c95926e47/chrome/browser/predictors/predictor_table_base.h

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cdff37ea1518dd8339acfce12bc4fb4f7c23527e

commit cdff37ea1518dd8339acfce12bc4fb4f7c23527e
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Mon Jul 02 16:06:57 2018

Enable ForegroundedOrClosedTest.SingleTab test

This reverts commit 65c948ac50c3c53d902716212057b48c5a32b102.

Reason for revert: The cause of flakiness was fixed in
https://crrev.com/c/1092851.

Original change's description:
> Disable Flaky ForegroundedOrClosedTest.SingleTab test
> 
> ForegroundedOrClosedTest.SingleTab has started to flake on ChromeOS. This
> disables the test until the cause can be found.
> 
> TEST=ForegroundedOrClosedTest.SingleTab
> TBR=fdoray@chromium.org
> 
> Bug:  839886 
> Change-Id: I650bc975a9e4795218eb205a628d3c604f7fcdcf
> Reviewed-on: https://chromium-review.googlesource.com/1044533
> Reviewed-by: Jonathan Ross <jonross@chromium.org>
> Commit-Queue: Jonathan Ross <jonross@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556112}

TBR=fdoray@chromium.org,jonross@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  839886 
Change-Id: I946125acee4ec518f25d6c1992ff3b3ecf9f6b0e
Reviewed-on: https://chromium-review.googlesource.com/1122797
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571912}
[modify] https://crrev.com/cdff37ea1518dd8339acfce12bc4fb4f7c23527e/chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc

Status: Fixed (was: Assigned)
All flaky tests are back to normal, closing this issue.
The problem mentioned in c#16 still exists, I've created Issue 860521 to track that.

Sign in to add a comment