High commit error rate on windows with new localstorage implementation |
||
Issue descriptionThe commit error rate on windows is high, and rising (currently at almost 50%....) Not entirely clear what is causing this (and other platforms don't seem to be effected), but one plausible explanation is the use of base::Move rather than base::Replace file when renaming files (which also fails fairly frequently), which then seems to leave the DB in a corrupt state where commits will keep failing indefinitely. Two things are needed: bug 712399 to recover from corruption, and somehow figure out how to reduce the error rate in the first place.
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb54972f13c11976dea9f1844e063506d4fc6384 commit eb54972f13c11976dea9f1844e063506d4fc6384 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Tue Jun 13 20:26:52 2017 Use base::ReplaceFile rather than base::Move in mojo leveldb env. This should hopefully reduce the large error rate in rename file operations on windows in the mojo leveldb env, as ReplaceFile should work better for these kinds of operations. Bug: 732863 Change-Id: I7abe9ff025eabfb13e1e2716aa091dd01ed9fccc Reviewed-on: https://chromium-review.googlesource.com/533679 Reviewed-by: Elliot Glaysher <erg@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#479131} [modify] https://crrev.com/eb54972f13c11976dea9f1844e063506d4fc6384/components/filesystem/directory_impl.cc [modify] https://crrev.com/eb54972f13c11976dea9f1844e063506d4fc6384/components/filesystem/directory_impl.h [modify] https://crrev.com/eb54972f13c11976dea9f1844e063506d4fc6384/components/filesystem/public/interfaces/directory.mojom [modify] https://crrev.com/eb54972f13c11976dea9f1844e063506d4fc6384/components/leveldb/leveldb_mojo_proxy.cc
,
Jun 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/469805742123ec65bdeea766d8960c5abd7f82e8 commit 469805742123ec65bdeea766d8960c5abd7f82e8 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Thu Jun 15 19:33:41 2017 If the file to rename doesn't exist, return OK immediately. This matches the behavior of ChromiumEnv::RenameFile and should prevent at least some unnecesary retries of renames. Bug: 732701 , 732863 Change-Id: I76637ba5f1f081f3709add94f4a085f54be0e096 Reviewed-on: https://chromium-review.googlesource.com/537134 Reviewed-by: Michael Nordman <michaeln@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#479785} [modify] https://crrev.com/469805742123ec65bdeea766d8960c5abd7f82e8/components/leveldb/env_mojo.cc
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/750631ece7666c65b1b1e51022933d5e88b7d7ba commit 750631ece7666c65b1b1e51022933d5e88b7d7ba Author: Marijn Kruisselbrink <mek@chromium.org> Date: Fri Jun 16 20:26:41 2017 Log results of SyncParent call in both mojo and non mojo leveldb envs. Bug: 732863 Change-Id: Ia8dfe5480cf16b0a333b14beb064b21a34f6d2d9 Reviewed-on: https://chromium-review.googlesource.com/538110 Reviewed-by: Chris Mumford <cmumford@chromium.org> Reviewed-by: Mark Pearson <mpearson@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#480150} [modify] https://crrev.com/750631ece7666c65b1b1e51022933d5e88b7d7ba/components/leveldb/env_mojo.cc [modify] https://crrev.com/750631ece7666c65b1b1e51022933d5e88b7d7ba/third_party/leveldatabase/env_chromium.cc [modify] https://crrev.com/750631ece7666c65b1b1e51022933d5e88b7d7ba/tools/metrics/histograms/histograms.xml
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0c2987eee004d427ace7113e539b93f5a1946979 commit 0c2987eee004d427ace7113e539b93f5a1946979 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Fri Jun 16 21:21:10 2017 Only implement DirectoryImpl::Flush on non-windows platforms. On other platforms attempting to open the directory as a file to then call Flush on it doesn't make sense, and in fact might cause this method to always fail on windows. Bug: 732863 Change-Id: I314d6e3f59ef2200b6accffe1f292bf2dfd3e40c Reviewed-on: https://chromium-review.googlesource.com/538078 Reviewed-by: Elliot Glaysher <erg@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#480174} [modify] https://crrev.com/0c2987eee004d427ace7113e539b93f5a1946979/components/filesystem/directory_impl.cc
,
Jun 20 2017
According to the UMA data this seems to be fixed now. Commit errors rates are < 0.05% on all platforms in versions after the commit from comment 5. |
||
►
Sign in to add a comment |
||
Comment 1 by mek@chromium.org
, Jun 13 2017Blocking: 586194