From be4f97ce11511c5581fdc8c60465c0420a36db62 Mon Sep 17 00:00:00 2001 From: Shubham Aggarwal Date: Fri, 19 Jun 2020 15:58:57 +0000 Subject: [PATCH] SQLite: Add support for WAL mode As per issue 78507, we are looking to add support for SQLite databases to use Write-ahead logging (https://www.sqlite.org/wal.html) mode in Chromium. WAL mode should give us significant performance gains across almost all use-cases. This change is a first step towards achieving this. It adds opt-in support to enable WAL mode for a database connection and perform a checkpoint. It also adds a feature flag to enable WAL mode for all databases by default to investigate its feasibility and impact on performance. Bug: 78507 Change-Id: I7fc5edcc39b50d2a13755d587cf342bded1af60a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095927 Commit-Queue: Shubham Aggarwal Reviewed-by: Brandon Maslen Reviewed-by: Victor Costan Reviewed-by: Chris Mumford Cr-Commit-Position: refs/heads/master@{#780318} --- sql/database.cc | 130 +++++++++++++++++++++++------ sql/database.h | 27 ++++++ sql/database_unittest.cc | 172 ++++++++++++++++++++++++++++++++------- sql/sql_features.cc | 4 + sql/sql_features.h | 2 + 5 files changed, 281 insertions(+), 54 deletions(-) diff --git a/sql/database.cc b/sql/database.cc index eeab5e626c2b5b..64c7e9cedfeb40 100644 --- a/sql/database.cc +++ b/sql/database.cc @@ -9,6 +9,7 @@ #include #include +#include "base/feature_list.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/format_macros.h" @@ -241,6 +242,8 @@ Database::Database() page_size_(kDefaultPageSize), cache_size_(0), exclusive_locking_(false), + want_wal_mode_( + base::FeatureList::IsEnabled(features::kEnableWALModeByDefault)), transaction_nesting_(0), needs_rollback_(false), in_memory_(false), @@ -813,8 +816,9 @@ bool Database::Raze() { return false; } - const std::string sql = base::StringPrintf("PRAGMA page_size=%d", page_size_); - if (!null_db.Execute(sql.c_str())) + const std::string page_size_sql = + base::StringPrintf("PRAGMA page_size=%d", page_size_); + if (!null_db.Execute(page_size_sql.c_str())) return false; #if defined(OS_ANDROID) @@ -894,9 +898,39 @@ bool Database::Raze() { DCHECK_EQ(rc, SQLITE_DONE) << "Failed retrying Raze()."; } + // Page size of |db_| and |null_db| differ. + if (rc == SQLITE_READONLY) { + // Enter TRUNCATE mode to change page size. + // TODO(shuagga@microsoft.com): Need a guarantee here that there is no other + // database connection open. + ignore_result(Execute("PRAGMA journal_mode=TRUNCATE;")); + if (!Execute(page_size_sql.c_str())) { + return false; + } + // Page size isn't changed until the database is vacuumed. + ignore_result(Execute("VACUUM")); + // Re-enter WAL mode. + if (UseWALMode()) { + ignore_result(Execute("PRAGMA journal_mode=WAL;")); + } + + rc = BackupDatabase(null_db.db_, db_, kMain); + base::UmaHistogramSparse("Sqlite.RazeDatabase2", rc); + + DCHECK_EQ(rc, SQLITE_DONE) << "Failed retrying Raze()."; + } + // TODO(shess): Figure out which other cases can happen. DCHECK_EQ(rc, SQLITE_DONE) << "Unable to copy entire null database."; + // Checkpoint to propagate transactions to the database file and empty the WAL + // file. + // The database can still contain old data if the Checkpoint fails so fail the + // Raze. + if (!CheckpointDatabase()) { + return false; + } + // The entire database should have been backed up. return rc == SQLITE_DONE; } @@ -1449,6 +1483,24 @@ bool Database::OpenInternal(const std::string& file_name, return false; } + // If indicated, lock up the database before doing anything else, so + // that the following code doesn't have to deal with locking. + // + // Needs to happen before any other operation is performed in WAL mode so that + // no operation relies on shared memory if exclusive locking is turned on. + // + // TODO(shess): This code is brittle. Find the cases where code + // doesn't request |exclusive_locking_| and audit that it does the + // right thing with SQLITE_BUSY, and that it doesn't make + // assumptions about who might change things in the database. + // http://crbug.com/56559 + if (exclusive_locking_) { + // TODO(shess): This should probably be a failure. Code which + // requests exclusive locking but doesn't get it is almost certain + // to be ill-tested. + ignore_result(Execute("PRAGMA locking_mode=EXCLUSIVE")); + } + // Enable extended result codes to provide more color on I/O errors. // Not having extended result codes is not a fatal problem, as // Chromium code does not attempt to handle I/O errors anyhow. The @@ -1480,35 +1532,43 @@ bool Database::OpenInternal(const std::string& file_name, } } - // If indicated, lock up the database before doing anything else, so - // that the following code doesn't have to deal with locking. - // TODO(shess): This code is brittle. Find the cases where code - // doesn't request |exclusive_locking_| and audit that it does the - // right thing with SQLITE_BUSY, and that it doesn't make - // assumptions about who might change things in the database. - // http://crbug.com/56559 - if (exclusive_locking_) { - // TODO(shess): This should probably be a failure. Code which - // requests exclusive locking but doesn't get it is almost certain - // to be ill-tested. - ignore_result(Execute("PRAGMA locking_mode=EXCLUSIVE")); - } + const base::TimeDelta kBusyTimeout = + base::TimeDelta::FromSeconds(kBusyTimeoutSeconds); + + // Needs to happen before entering WAL mode. Will only work if this the first + // time the database is being opened in WAL mode. + const std::string page_size_sql = + base::StringPrintf("PRAGMA page_size=%d", page_size_); + ignore_result(ExecuteWithTimeout(page_size_sql.c_str(), kBusyTimeout)); // http://www.sqlite.org/pragma.html#pragma_journal_mode + // WAL - Use a write-ahead log instead of a journal file. // DELETE (default) - delete -journal file to commit. // TRUNCATE - truncate -journal file to commit. // PERSIST - zero out header of -journal file to commit. // TRUNCATE should be faster than DELETE because it won't need directory // changes for each transaction. PERSIST may break the spirit of using // secure_delete. - ignore_result(Execute("PRAGMA journal_mode=TRUNCATE")); - - const base::TimeDelta kBusyTimeout = - base::TimeDelta::FromSeconds(kBusyTimeoutSeconds); - - const std::string page_size_sql = - base::StringPrintf("PRAGMA page_size=%d", page_size_); - ignore_result(ExecuteWithTimeout(page_size_sql.c_str(), kBusyTimeout)); + // + // Needs to be performed after setting exclusive locking mode. Otherwise can + // fail if underlying VFS doesn't support shared memory. + if (UseWALMode()) { + // Set the synchronous flag to NORMAL. This means that writers don't flush + // the WAL file after every write. The WAL file is only flushed on a + // checkpoint. In this case, transcations might lose durability on a power + // loss (but still durable after an application crash). + // TODO(shuagga@microsoft.com): Evaluate if this loss of durability is a + // concern. + ignore_result(Execute("PRAGMA synchronous=NORMAL")); + + // Opening the db in WAL mode can fail (eg if the underlying VFS doesn't + // support shared memory and we are not in exclusive locking mode). + // + // TODO(shuagga@microsoft.com): We should probably catch a failure here. + ignore_result(Execute("PRAGMA journal_mode=WAL")); + } else { + ignore_result(Execute("PRAGMA journal_mode=TRUNCATE")); + } if (cache_size_ != 0) { const std::string cache_size_sql = @@ -1732,4 +1792,28 @@ bool Database::ReportMemoryUsage(base::trace_event::ProcessMemoryDump* pmd, memory_dump_provider_->ReportMemoryUsage(pmd, dump_name); } +bool Database::UseWALMode() const { +#if defined(OS_FUCHSIA) + // WAL mode is only enabled on Fuchsia for databases with exclusive + // locking, because this case does not require shared memory support. + // At the time this was implemented (May 2020), Fuchsia's shared + // memory support was insufficient for SQLite's needs. + return want_wal_mode_ && exclusive_locking_; +#else + return want_wal_mode_; +#endif // defined(OS_FUCHSIA) +} + +bool Database::CheckpointDatabase() { + base::Optional scoped_blocking_call; + InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call); + + static const char* kMainDb = "main"; + int rc = sqlite3_wal_checkpoint_v2(db_, kMainDb, SQLITE_CHECKPOINT_PASSIVE, + /*pnLog=*/nullptr, + /*pnCkpt=*/nullptr); + + return rc == SQLITE_OK; +} + } // namespace sql diff --git a/sql/database.h b/sql/database.h index 18ccdcd7d3d8ad..2bf8652f33c1cb 100644 --- a/sql/database.h +++ b/sql/database.h @@ -88,6 +88,23 @@ class COMPONENT_EXPORT(SQL) Database { cache_size_ = cache_size; } + // Returns whether a database will be opened in WAL mode. + bool UseWALMode() const; + + // Enables/disables WAL mode (https://www.sqlite.org/wal.html) when + // opening a new database. + // + // WAL mode is currently not fully supported on FuchsiaOS. It will only be + // turned on if the database is also using exclusive locking mode. + // (https://crbug.com/1082059) + // + // Note: Changing page size is not supported when in WAL mode. So running + // 'PRAGMA page_size = ' or using set_page_size will result in + // no-ops. + // + // This must be called before Open() to have an effect. + void want_wal_mode(bool enabled) { want_wal_mode_ = enabled; } + // Call to put the database in exclusive locking mode. There is no "back to // normal" flag because of some additional requirements sqlite puts on this // transaction (requires another access to the DB) and because we don't @@ -396,6 +413,14 @@ class COMPONENT_EXPORT(SQL) Database { // See GetCachedStatement above for examples and error information. scoped_refptr GetUniqueStatement(const char* sql); + // Performs a passive checkpoint on the main attached database if it is in + // WAL mode. Returns true if the checkpoint was successful and false in case + // of an error. It is a no-op if the database is not in WAL mode. + // + // Note: Checkpointing is a very slow operation and will block any writes + // until it is finished. Please use with care. + bool CheckpointDatabase(); + // Info querying ------------------------------------------------------------- // Returns true if the given structure exists. Instead of test-then-create, @@ -693,7 +718,9 @@ class COMPONENT_EXPORT(SQL) Database { // use the default value. int page_size_; int cache_size_; + bool exclusive_locking_; + bool want_wal_mode_; // Holds references to all cached statements so they remain active. // diff --git a/sql/database_unittest.cc b/sql/database_unittest.cc index 7b3e04f49106a1..401c2b13d8b0e9 100644 --- a/sql/database_unittest.cc +++ b/sql/database_unittest.cc @@ -20,6 +20,7 @@ #include "sql/database.h" #include "sql/database_memory_dump_provider.h" #include "sql/meta_table.h" +#include "sql/sql_features.h" #include "sql/statement.h" #include "sql/test/database_test_peer.h" #include "sql/test/error_callback_support.h" @@ -538,14 +539,18 @@ TEST_F(SQLDatabaseTest, RazeLocked) { // An unfinished read transaction in the other connection also // blocks raze. - const char* kQuery = "SELECT COUNT(*) FROM foo"; - sql::Statement s(other_db.GetUniqueStatement(kQuery)); - ASSERT_TRUE(s.Step()); - ASSERT_FALSE(db().Raze()); + // This doesn't happen in WAL mode because reads are no longer blocked by + // write operations when using a WAL. + if (!base::FeatureList::IsEnabled(sql::features::kEnableWALModeByDefault)) { + const char* kQuery = "SELECT COUNT(*) FROM foo"; + sql::Statement s(other_db.GetUniqueStatement(kQuery)); + ASSERT_TRUE(s.Step()); + ASSERT_FALSE(db().Raze()); - // Complete the statement unlocks the database. - ASSERT_FALSE(s.Step()); - ASSERT_TRUE(db().Raze()); + // Completing the statement unlocks the database. + ASSERT_FALSE(s.Step()); + ASSERT_TRUE(db().Raze()); + } } // Verify that Raze() can handle an empty file. SQLite should treat @@ -769,6 +774,11 @@ TEST_F(SQLDatabaseTest, RazeTruncate) { ASSERT_TRUE( db().Execute("INSERT INTO foo (value) VALUES (randomblob(1024))")); } + + // In WAL mode, writes don't reach the database file until a checkpoint + // happens. + ASSERT_TRUE(db().CheckpointDatabase()); + int64_t db_size; ASSERT_TRUE(base::GetFileSize(db_path(), &db_size)); ASSERT_GT(db_size, expected_size); @@ -796,23 +806,55 @@ TEST_F(SQLDatabaseTest, SetTempDirForSQL) { } #endif // defined(OS_ANDROID) -TEST_F(SQLDatabaseTest, DeleteNonWal) { +// Contained param indicates whether WAL mode is switched on or not. +class JournalModeTest : public SQLDatabaseTest, + public testing::WithParamInterface { + public: + void SetUp() override { +#if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode + // on Fuchsia + if (IsWALEnabled()) { + db().set_exclusive_locking(); + } +#endif // defined(OS_FUCHSIA) + db().want_wal_mode(IsWALEnabled()); + SQLDatabaseTest::SetUp(); + } + + bool IsWALEnabled() { return GetParam(); } +}; + +TEST_P(JournalModeTest, Delete) { EXPECT_TRUE(db().Execute("CREATE TABLE x (x)")); db().Close(); - // Should have both a main database file and a journal file because - // of journal_mode TRUNCATE. base::FilePath journal_path = sql::Database::JournalPath(db_path()); + base::FilePath wal_path = sql::Database::WriteAheadLogPath(db_path()); + + // Should have both a main database file and a journal file if + // journal_mode is TRUNCATE. There is no WAL file as it is deleted on Close. ASSERT_TRUE(GetPathExists(db_path())); - ASSERT_TRUE(GetPathExists(journal_path)); + if (!IsWALEnabled()) { // TRUNCATE mode + ASSERT_TRUE(GetPathExists(journal_path)); + } sql::Database::Delete(db_path()); EXPECT_FALSE(GetPathExists(db_path())); EXPECT_FALSE(GetPathExists(journal_path)); + EXPECT_FALSE(GetPathExists(wal_path)); } +// WAL mode is currently not supported on Fuchsia +#if !defined(OS_FUCHSIA) +INSTANTIATE_TEST_SUITE_P(SQLDatabaseTest, JournalModeTest, testing::Bool()); +#else +INSTANTIATE_TEST_SUITE_P(SQLDatabaseTest, + JournalModeTest, + testing::Values(false)); +#endif + #if defined(OS_POSIX) // This test operates on POSIX file permissions. -TEST_F(SQLDatabaseTest, PosixFilePermissions) { +TEST_P(JournalModeTest, PosixFilePermissions) { db().Close(); sql::Database::Delete(db_path()); ASSERT_FALSE(GetPathExists(db_path())); @@ -834,26 +876,10 @@ TEST_F(SQLDatabaseTest, PosixFilePermissions) { EXPECT_TRUE(base::GetPosixFilePermissions(db_path(), &mode)); ASSERT_EQ(mode, 0600); - { - base::FilePath journal_path = sql::Database::JournalPath(db_path()); - DLOG(ERROR) << "journal_path: " << journal_path; - ASSERT_TRUE(GetPathExists(journal_path)); - EXPECT_TRUE(base::GetPosixFilePermissions(journal_path, &mode)); - ASSERT_EQ(mode, 0600); - } + if (IsWALEnabled()) { // WAL mode + // The WAL file is created lazily on first change. + ASSERT_TRUE(db().Execute("CREATE TABLE foo (a, b)")); - // Reopen the database and turn on WAL mode. - db().Close(); - sql::Database::Delete(db_path()); - ASSERT_FALSE(GetPathExists(db_path())); - ASSERT_TRUE(db().Open(db_path())); - ASSERT_TRUE(db().Execute("PRAGMA journal_mode = WAL")); - ASSERT_EQ("wal", ExecuteWithResult(&db(), "PRAGMA journal_mode")); - - // The WAL file is created lazily on first change. - ASSERT_TRUE(db().Execute("CREATE TABLE foo (a, b)")); - - { base::FilePath wal_path = sql::Database::WriteAheadLogPath(db_path()); ASSERT_TRUE(GetPathExists(wal_path)); EXPECT_TRUE(base::GetPosixFilePermissions(wal_path, &mode)); @@ -863,6 +889,12 @@ TEST_F(SQLDatabaseTest, PosixFilePermissions) { ASSERT_TRUE(GetPathExists(shm_path)); EXPECT_TRUE(base::GetPosixFilePermissions(shm_path, &mode)); ASSERT_EQ(mode, 0600); + } else { // Truncate mode + base::FilePath journal_path = sql::Database::JournalPath(db_path()); + DLOG(ERROR) << "journal_path: " << journal_path; + ASSERT_TRUE(GetPathExists(journal_path)); + EXPECT_TRUE(base::GetPosixFilePermissions(journal_path, &mode)); + ASSERT_EQ(mode, 0600); } } #endif // defined(OS_POSIX) @@ -1220,6 +1252,84 @@ TEST_F(SQLDatabaseTest, GetAppropriateMmapSizeAltStatus) { ExecuteWithResult(&db(), "SELECT * FROM MmapStatus")); } +TEST_F(SQLDatabaseTest, EnableWALMode) { + db().want_wal_mode(true); +#if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode + // on Fuchsia + db().set_exclusive_locking(); +#endif // defined(OS_FUCHSIA) + ASSERT_TRUE(Reopen()); + + EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA journal_mode"), "wal"); +} + +TEST_F(SQLDatabaseTest, DisableWALMode) { + db().want_wal_mode(true); +#if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode + // on Fuchsia + db().set_exclusive_locking(); +#endif // defined(OS_FUCHSIA) + ASSERT_TRUE(Reopen()); + ASSERT_EQ(ExecuteWithResult(&db(), "PRAGMA journal_mode"), "wal"); + + // Add some data to ensure that disabling WAL mode correctly handles a + // populated WAL file. + ASSERT_TRUE( + db().Execute("CREATE TABLE foo (id INTEGER UNIQUE, value INTEGER)")); + ASSERT_TRUE(db().Execute("INSERT INTO foo VALUES (1, 1)")); + ASSERT_TRUE(db().Execute("INSERT INTO foo VALUES (2, 2)")); + + db().want_wal_mode(false); + ASSERT_TRUE(Reopen()); + EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA journal_mode"), "truncate"); + // Check that data is preserved + EXPECT_EQ(ExecuteWithResult(&db(), "SELECT SUM(value) FROM foo WHERE id < 3"), + "3"); +} + +TEST_F(SQLDatabaseTest, CheckpointDatabase) { + if (!db().UseWALMode()) { + db().Close(); + sql::Database::Delete(db_path()); + db().want_wal_mode(true); +#if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode + // on Fuchsia + db().set_exclusive_locking(); +#endif // defined(OS_FUCHSIA) + ASSERT_TRUE(db().Open(db_path())); + ASSERT_EQ(ExecuteWithResult(&db(), "PRAGMA journal_mode"), "wal"); + } + + base::FilePath wal_path = sql::Database::WriteAheadLogPath(db_path()); + + int64_t wal_size = 0; + // WAL file initially empty. + EXPECT_TRUE(GetPathExists(wal_path)); + base::GetFileSize(wal_path, &wal_size); + EXPECT_EQ(wal_size, 0); + + ASSERT_TRUE( + db().Execute("CREATE TABLE foo (id INTEGER UNIQUE, value INTEGER)")); + ASSERT_TRUE(db().Execute("INSERT INTO foo VALUES (1, 1)")); + ASSERT_TRUE(db().Execute("INSERT INTO foo VALUES (2, 2)")); + + // Writes reach WAL file but not db file. + base::GetFileSize(wal_path, &wal_size); + EXPECT_GT(wal_size, 0); + + int64_t db_size = 0; + base::GetFileSize(db_path(), &db_size); + EXPECT_EQ(db_size, db().page_size()); + + // Checkpoint database to immediately propagate writes to DB file. + EXPECT_TRUE(db().CheckpointDatabase()); + + base::GetFileSize(db_path(), &db_size); + EXPECT_GT(db_size, db().page_size()); + EXPECT_EQ(ExecuteWithResult(&db(), "SELECT value FROM foo where id=1"), "1"); + EXPECT_EQ(ExecuteWithResult(&db(), "SELECT value FROM foo where id=2"), "2"); +} + // To prevent invalid SQL from accidentally shipping to production, prepared // statements which fail to compile with SQLITE_ERROR call DLOG(DCHECK). This // case cannot be suppressed with an error callback. diff --git a/sql/sql_features.cc b/sql/sql_features.cc index 833c43e00b0085..7069737a22f164 100644 --- a/sql/sql_features.cc +++ b/sql/sql_features.cc @@ -20,6 +20,10 @@ namespace features { const base::Feature kSqlSkipPreload{"SqlSkipPreload", base::FEATURE_DISABLED_BY_DEFAULT}; +// Enable WAL mode for all SQLite databases. +const base::Feature kEnableWALModeByDefault{"EnableWALModeByDefault", + base::FEATURE_DISABLED_BY_DEFAULT}; + } // namespace features } // namespace sql diff --git a/sql/sql_features.h b/sql/sql_features.h index eee57706634ade..76c4a63c07e23d 100644 --- a/sql/sql_features.h +++ b/sql/sql_features.h @@ -14,6 +14,8 @@ namespace features { COMPONENT_EXPORT(SQL) extern const base::Feature kSqlSkipPreload; +COMPONENT_EXPORT(SQL) extern const base::Feature kEnableWALModeByDefault; + } // namespace features } // namespace sql