diff --git a/sql/database.cc b/sql/database.cc index db46be604af6a7..59f1959d30480c 100644 --- a/sql/database.cc +++ b/sql/database.cc @@ -233,19 +233,15 @@ void Database::StatementRef::Close(bool forced) { was_valid_ = was_valid_ && forced; } -static_assert( - Database::kDefaultPageSize == SQLITE_DEFAULT_PAGE_SIZE, - "Database::kDefaultPageSize must match the value configured into SQLite"); +static_assert(DatabaseOptions::kDefaultPageSize == SQLITE_DEFAULT_PAGE_SIZE, + "DatabaseOptions::kDefaultPageSize must match the value " + "configured into SQLite"); -constexpr int Database::kDefaultPageSize; +Database::Database() : Database({.exclusive_locking = false}) {} -Database::Database() +Database::Database(DatabaseOptions options) : db_(nullptr), - page_size_(kDefaultPageSize), - cache_size_(0), - exclusive_locking_(false), - want_wal_mode_( - base::FeatureList::IsEnabled(features::kEnableWALModeByDefault)), + options_(options), transaction_nesting_(0), needs_rollback_(false), in_memory_(false), @@ -254,7 +250,12 @@ Database::Database() mmap_disabled_(!enable_mmap_by_default_), mmap_enabled_(false), total_changes_at_last_release_(0), - stats_histogram_(nullptr) {} + stats_histogram_(nullptr) { + DCHECK_GE(options.page_size, 512); + DCHECK_LE(options.page_size, 65536); + DCHECK(!(options.page_size & (options.page_size - 1))) + << "page_size must be a power of two"; +} Database::~Database() { Close(); @@ -821,7 +822,7 @@ bool Database::Raze() { } const std::string page_size_sql = - base::StringPrintf("PRAGMA page_size=%d", page_size_); + base::StringPrintf("PRAGMA page_size=%d", options_.page_size); if (!null_db.Execute(page_size_sql.c_str())) return false; @@ -1519,7 +1520,7 @@ bool Database::OpenInternal(const std::string& file_name, // enabled. // // TODO(crbug.com/1120969): Remove support for non-exclusive mode. - if (!exclusive_locking_) { + if (!options_.exclusive_locking) { err = ExecuteAndReturnErrorCode("PRAGMA locking_mode=NORMAL"); if (err != SQLITE_OK) return false; @@ -1562,7 +1563,7 @@ bool Database::OpenInternal(const std::string& file_name, // 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_); + base::StringPrintf("PRAGMA page_size=%d", options_.page_size); ignore_result(ExecuteWithTimeout(page_size_sql.c_str(), kBusyTimeout)); // http://www.sqlite.org/pragma.html#pragma_journal_mode @@ -1594,9 +1595,9 @@ bool Database::OpenInternal(const std::string& file_name, ignore_result(Execute("PRAGMA journal_mode=TRUNCATE")); } - if (cache_size_ != 0) { + if (options_.cache_size != 0) { const std::string cache_size_sql = - base::StringPrintf("PRAGMA cache_size=%d", cache_size_); + base::StringPrintf("PRAGMA cache_size=%d", options_.cache_size); ignore_result(ExecuteWithTimeout(cache_size_sql.c_str(), kBusyTimeout)); } @@ -1822,9 +1823,9 @@ bool Database::UseWALMode() const { // 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_; + return options_.wal_mode && options_.exclusive_locking; #else - return want_wal_mode_; + return options_.wal_mode; #endif // defined(OS_FUCHSIA) } diff --git a/sql/database.h b/sql/database.h index 6e4334ea776c23..c8f365eba234a2 100644 --- a/sql/database.h +++ b/sql/database.h @@ -17,6 +17,7 @@ #include "base/compiler_specific.h" #include "base/component_export.h" #include "base/containers/flat_map.h" +#include "base/feature_list.h" #include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/memory/ref_counted.h" @@ -24,6 +25,7 @@ #include "base/sequence_checker.h" #include "base/threading/scoped_blocking_call.h" #include "sql/internal_api_token.h" +#include "sql/sql_features.h" #include "sql/statement_id.h" struct sqlite3; @@ -46,6 +48,46 @@ namespace test { class ScopedErrorExpecter; } // namespace test +struct COMPONENT_EXPORT(SQL) DatabaseOptions { + // Default page size for newly created databases. + // + // Guaranteed to match SQLITE_DEFAULT_PAGE_SIZE. + static constexpr int kDefaultPageSize = 4096; + + // If true, the database can only be opened by one process at a time. + // + // Exclusive mode is strongly recommended. It reduces the I/O cost of setting + // up a transaction. It also removes the need of handling transaction failures + // due to lock contention. + bool exclusive_locking = true; + + // If true, enables SQLite's Write-Ahead Logging (WAL). + // + // WAL integration is under development, and should not be used in shipping + // Chrome features yet. In particular, our custom database recovery code does + // not support the WAL log file. + // + // More details at https://www.sqlite.org/wal.html + bool wal_mode = + base::FeatureList::IsEnabled(sql::features::kEnableWALModeByDefault); + + // Database page size. + // + // Larger page sizes result in shallower B-trees, because they allow an inner + // page to hold more keys. On the flip side, larger page sizes may result in + // more I/O when making small changes to existing records. + int page_size = kDefaultPageSize; + + // The size of in-memory cache, in pages. + // + // SQLite's database cache will take up at most (`page_size` * `cache_size`) + // bytes of RAM. + // + // 0 invokes SQLite's default, which is currently to size up the cache to use + // exactly 2,048,000 bytes of RAM. + int cache_size = 0; +}; + // Handle to an open SQLite database. // // Instances of this class are thread-unsafe and DCHECK that they are accessed @@ -57,7 +99,13 @@ class COMPONENT_EXPORT(SQL) Database { public: // The database is opened by calling Open[InMemory](). Any uncommitted // transactions will be rolled back when this object is deleted. + // + // This constructor is deprecated. + // TODO(crbug.com/1126968): Remove this constructor after migrating all + // uses to the explicit constructor below. Database(); + // |options| only affects newly created databases. + explicit Database(DatabaseOptions options); ~Database(); // Allows mmapping to be disabled globally by default in the calling process. @@ -79,11 +127,11 @@ class COMPONENT_EXPORT(SQL) Database { DCHECK(!(page_size & (page_size - 1))) << "page_size must be a power of two"; - page_size_ = page_size; + options_.page_size = page_size; } // The page size that will be used when creating a new database. - int page_size() const { return page_size_; } + int page_size() const { return options_.page_size; } // Sets the number of pages that will be cached in memory by sqlite. The // total cache size in bytes will be page_size * cache_size. This must be @@ -91,7 +139,7 @@ class COMPONENT_EXPORT(SQL) Database { void set_cache_size(int cache_size) { DCHECK_GE(cache_size, 0); - cache_size_ = cache_size; + options_.cache_size = cache_size; } // Returns whether a database will be opened in WAL mode. @@ -109,7 +157,7 @@ class COMPONENT_EXPORT(SQL) Database { // no-ops. // // This must be called before Open() to have an effect. - void want_wal_mode(bool enabled) { want_wal_mode_ = enabled; } + void want_wal_mode(bool enabled) { options_.wal_mode = enabled; } // Makes database accessible by only one process at a time. // @@ -130,7 +178,7 @@ class COMPONENT_EXPORT(SQL) Database { // // SQLite's locking protocol is summarized at // https://www.sqlite.org/c3ref/io_methods.html - void set_exclusive_locking() { exclusive_locking_ = true; } + void set_exclusive_locking() { options_.exclusive_locking = true; } // Call to use alternative status-tracking for mmap. Usually this is tracked // in the meta table, but some databases have no meta table. @@ -291,7 +339,7 @@ class COMPONENT_EXPORT(SQL) Database { // these all return false, since it is unlikely that the caller // could fix them. // - // The database's page size is taken from |page_size_|. The + // The database's page size is taken from |options_.page_size|. The // existing database's |auto_vacuum| setting is lost (the // possibility of corruption makes it unreliable to pull it from the // existing database). To re-enable on the empty database requires @@ -530,11 +578,6 @@ class COMPONENT_EXPORT(SQL) Database { // the existence of specific files. static base::FilePath SharedMemoryFilePath(const base::FilePath& db_path); - // Default page size for newly created databases. - // - // Guaranteed to match SQLITE_DEFAULT_PAGE_SIZE. - static constexpr int kDefaultPageSize = 4096; - // Internal state accessed by other classes in //sql. sqlite3* db(InternalApiToken) const { return db_; } bool poisoned(InternalApiToken) const { return poisoned_; } @@ -738,13 +781,9 @@ class COMPONENT_EXPORT(SQL) Database { // Init resulted in an error. sqlite3* db_; - // Parameters we'll configure in sqlite before doing anything else. Zero means - // use the default value. - int page_size_; - int cache_size_; - - bool exclusive_locking_; - bool want_wal_mode_; + // TODO(shuagga@microsoft.com): Make `options_` const after removing all + // setters. + DatabaseOptions options_; // Holds references to all cached statements so they remain active. // diff --git a/sql/database_unittest.cc b/sql/database_unittest.cc index 600f7cc2ac6343..b7264a42700ed0 100644 --- a/sql/database_unittest.cc +++ b/sql/database_unittest.cc @@ -445,29 +445,32 @@ void TestPageSize(const base::FilePath& db_prefix, const base::FilePath db_path = db_prefix.InsertBeforeExtensionASCII( base::NumberToString(initial_page_size)); sql::Database::Delete(db_path); - sql::Database db; - db.set_page_size(initial_page_size); + sql::Database db({.page_size = initial_page_size}); ASSERT_TRUE(db.Open(db_path)); ASSERT_TRUE(db.Execute(kCreateSql)); ASSERT_TRUE(db.Execute(kInsertSql1)); ASSERT_TRUE(db.Execute(kInsertSql2)); ASSERT_EQ(expected_initial_page_size, ExecuteWithResult(&db, "PRAGMA page_size")); + db.Close(); + // Re-open the database while setting a new |options.page_size| in the object. + sql::Database razed_db({.page_size = final_page_size}); + ASSERT_TRUE(razed_db.Open(db_path)); // Raze will use the page size set in the connection object, which may not // match the file's page size. - db.set_page_size(final_page_size); - ASSERT_TRUE(db.Raze()); + ASSERT_TRUE(razed_db.Raze()); // SQLite 3.10.2 (at least) has a quirk with the sqlite3_backup() API (used by // Raze()) which causes the destination database to remember the previous // page_size, even if the overwriting database changed the page_size. Access // the actual database to cause the cached value to be updated. - EXPECT_EQ("0", ExecuteWithResult(&db, "SELECT COUNT(*) FROM sqlite_master")); + EXPECT_EQ("0", + ExecuteWithResult(&razed_db, "SELECT COUNT(*) FROM sqlite_master")); EXPECT_EQ(expected_final_page_size, - ExecuteWithResult(&db, "PRAGMA page_size")); - EXPECT_EQ("1", ExecuteWithResult(&db, "PRAGMA page_count")); + ExecuteWithResult(&razed_db, "PRAGMA page_size")); + EXPECT_EQ("1", ExecuteWithResult(&razed_db, "PRAGMA page_count")); } // Verify that sql::Recovery maintains the page size, and the virtual table @@ -492,8 +495,9 @@ TEST_F(SQLDatabaseTest, RazePageSize) { // Databases with no page size specified should result in the default // page size. 2k has never been the default page size. ASSERT_NE("2048", default_page_size); - EXPECT_NO_FATAL_FAILURE(TestPageSize( - db_path(), 2048, "2048", Database::kDefaultPageSize, default_page_size)); + EXPECT_NO_FATAL_FAILURE(TestPageSize(db_path(), 2048, "2048", + DatabaseOptions::kDefaultPageSize, + default_page_size)); } // Test that Raze() results are seen in other connections. @@ -567,6 +571,8 @@ TEST_F(SQLDatabaseTest, RazeEmptyDB) { } // Verify that Raze() can handle a file of junk. +// Need exclusive mode off here as there are some subtleties (by design) around +// how the cache is used with it on which causes the test to fail. TEST_F(SQLDatabaseTest, RazeNOTADB) { db().Close(); sql::Database::Delete(db_path()); @@ -809,17 +815,19 @@ TEST_F(SQLDatabaseTest, SetTempDirForSQL) { class JournalModeTest : public SQLDatabaseTest, public testing::WithParamInterface { public: - void SetUp() override { + JournalModeTest() : SQLDatabaseTest(GetDBOptions()) {} + + sql::DatabaseOptions GetDBOptions() { + sql::DatabaseOptions options; + options.wal_mode = IsWALEnabled(); #if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode // on Fuchsia if (IsWALEnabled()) { - db().set_exclusive_locking(); + options.exclusive_locking = true; } #endif // defined(OS_FUCHSIA) - db().want_wal_mode(IsWALEnabled()); - SQLDatabaseTest::SetUp(); + return options; } - bool IsWALEnabled() { return GetParam(); } }; @@ -884,10 +892,13 @@ TEST_P(JournalModeTest, PosixFilePermissions) { EXPECT_TRUE(base::GetPosixFilePermissions(wal_path, &mode)); ASSERT_EQ(mode, 0600); - base::FilePath shm_path = sql::Database::SharedMemoryFilePath(db_path()); - ASSERT_TRUE(GetPathExists(shm_path)); - EXPECT_TRUE(base::GetPosixFilePermissions(shm_path, &mode)); - ASSERT_EQ(mode, 0600); + // The shm file doesn't exist in exclusive locking mode. + if (ExecuteWithResult(&db(), "PRAGMA locking_mode") == "normal") { + base::FilePath shm_path = sql::Database::SharedMemoryFilePath(db_path()); + 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; @@ -1274,11 +1285,13 @@ TEST_F(SQLDatabaseTest, GetMemoryUsage) { << "Page cache usage should go down after calling TrimMemory()"; } -TEST_F(SQLDatabaseTest, LockingModeExclusive) { - db().Close(); - db().set_exclusive_locking(); - ASSERT_TRUE(db().Open(db_path())); +class SQLDatabaseTestExclusiveMode : public SQLDatabaseTest { + public: + SQLDatabaseTestExclusiveMode() + : SQLDatabaseTest({.exclusive_locking = true}) {} +}; +TEST_F(SQLDatabaseTestExclusiveMode, LockingModeExclusive) { EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA locking_mode"), "exclusive"); } @@ -1286,53 +1299,14 @@ TEST_F(SQLDatabaseTest, LockingModeNormal) { EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA locking_mode"), "normal"); } -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_P(JournalModeTest, OpenedInCorrectMode) { + std::string expected_mode = IsWALEnabled() ? "wal" : "truncate"; + EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA journal_mode"), expected_mode); } -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"); - } +TEST_P(JournalModeTest, CheckpointDatabase) { + if (!IsWALEnabled()) + return; base::FilePath wal_path = sql::Database::WriteAheadLogPath(db_path()); diff --git a/sql/recovery.cc b/sql/recovery.cc index a6af79ed48d77c..4e6d1519991389 100644 --- a/sql/recovery.cc +++ b/sql/recovery.cc @@ -183,10 +183,9 @@ void Recovery::Rollback(std::unique_ptr r) { r->Shutdown(POISON); } -Recovery::Recovery(Database* connection) : db_(connection), recover_db_() { - // Result should keep the page size specified earlier. - recover_db_.set_page_size(db_->page_size()); - +Recovery::Recovery(Database* connection) + : db_(connection), + recover_db_({.exclusive_locking = false, .page_size = db_->page_size()}) { // Files with I/O errors cannot be safely memory-mapped. recover_db_.set_mmap_disabled(); diff --git a/sql/recovery_unittest.cc b/sql/recovery_unittest.cc index 17b8e8a511998f..afd386574f3619 100644 --- a/sql/recovery_unittest.cc +++ b/sql/recovery_unittest.cc @@ -29,8 +29,8 @@ namespace { -using sql::test::ExecuteWithResults; using sql::test::ExecuteWithResult; +using sql::test::ExecuteWithResults; // Dump consistent human-readable representation of the database // schema. For tables or indices, this will contain the sql command @@ -943,30 +943,33 @@ void TestPageSize(const base::FilePath& db_prefix, const base::FilePath db_path = db_prefix.InsertBeforeExtensionASCII( base::NumberToString(initial_page_size)); sql::Database::Delete(db_path); - sql::Database db; - db.set_page_size(initial_page_size); + sql::Database db({.page_size = initial_page_size}); ASSERT_TRUE(db.Open(db_path)); ASSERT_TRUE(db.Execute(kCreateSql)); ASSERT_TRUE(db.Execute(kInsertSql1)); ASSERT_TRUE(db.Execute(kInsertSql2)); ASSERT_EQ(expected_initial_page_size, ExecuteWithResult(&db, "PRAGMA page_size")); + db.Close(); - // Recovery will use the page size set in the connection object, which may not + // Re-open the database while setting a new |options.page_size| in the object. + sql::Database recover_db({.page_size = final_page_size}); + ASSERT_TRUE(recover_db.Open(db_path)); + // Recovery will use the page size set in the database object, which may not // match the file's page size. - db.set_page_size(final_page_size); - sql::Recovery::RecoverDatabase(&db, db_path); + sql::Recovery::RecoverDatabase(&recover_db, db_path); // Recovery poisoned the handle, must re-open. - db.Close(); + recover_db.Close(); // Make sure the page size is read from the file. - db.set_page_size(sql::Database::kDefaultPageSize); - ASSERT_TRUE(db.Open(db_path)); + sql::Database recovered_db( + {.page_size = sql::DatabaseOptions::kDefaultPageSize}); + ASSERT_TRUE(recovered_db.Open(db_path)); ASSERT_EQ(expected_final_page_size, - ExecuteWithResult(&db, "PRAGMA page_size")); + ExecuteWithResult(&recovered_db, "PRAGMA page_size")); EXPECT_EQ("That was a test\nThis is a test", - ExecuteWithResults(&db, kSelectSql, "|", "\n")); + ExecuteWithResults(&recovered_db, kSelectSql, "|", "\n")); } // Verify that sql::Recovery maintains the page size, and the virtual table @@ -978,8 +981,8 @@ TEST_F(SQLRecoveryTest, PageSize) { // Check the default page size first. EXPECT_NO_FATAL_FAILURE(TestPageSize( - db_path(), sql::Database::kDefaultPageSize, default_page_size, - sql::Database::kDefaultPageSize, default_page_size)); + db_path(), sql::DatabaseOptions::kDefaultPageSize, default_page_size, + sql::DatabaseOptions::kDefaultPageSize, default_page_size)); // Sync uses 32k pages. EXPECT_NO_FATAL_FAILURE( @@ -995,7 +998,7 @@ TEST_F(SQLRecoveryTest, PageSize) { // page size. 2k has never been the default page size. ASSERT_NE("2048", default_page_size); EXPECT_NO_FATAL_FAILURE(TestPageSize(db_path(), 2048, "2048", - sql::Database::kDefaultPageSize, + sql::DatabaseOptions::kDefaultPageSize, default_page_size)); } diff --git a/sql/test/sql_test_base.cc b/sql/test/sql_test_base.cc index 6598bbb18fa011..ef32741cb0d465 100644 --- a/sql/test/sql_test_base.cc +++ b/sql/test/sql_test_base.cc @@ -11,6 +11,8 @@ namespace sql { SQLTestBase::SQLTestBase() = default; +SQLTestBase::SQLTestBase(sql::DatabaseOptions options) : db_(options) {} + SQLTestBase::~SQLTestBase() = default; base::FilePath SQLTestBase::db_path() { diff --git a/sql/test/sql_test_base.h b/sql/test/sql_test_base.h index 13658e0c8f526d..cf35f1e32fd0c5 100644 --- a/sql/test/sql_test_base.h +++ b/sql/test/sql_test_base.h @@ -25,6 +25,7 @@ namespace sql { class SQLTestBase : public testing::Test { public: SQLTestBase(); + explicit SQLTestBase(sql::DatabaseOptions options); ~SQLTestBase() override; enum WriteJunkType {