Add F_BARRIERFSYNC remote config kill switch

This commit is contained in:
Evan Hahn 2023-04-21 16:10:35 -05:00 committed by GitHub
parent a597f56c9c
commit d6b60ac2cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 73 additions and 5 deletions

View File

@ -380,9 +380,7 @@ public class GRDBDatabaseStorageAdapter: NSObject {
try db.execute(sql: "PRAGMA key = \"\(keyspec)\"")
try db.execute(sql: "PRAGMA cipher_plaintext_header_size = 32")
try db.execute(sql: "PRAGMA checkpoint_fullfsync = ON")
// This will use F_BARRIERFSYNC because of our fork of SQLCipher.
try db.execute(sql: "PRAGMA fullfsync = ON")
try SqliteUtil.setBarrierFsync(db: db, enabled: true)
if !CurrentAppContext().isMainApp {
let perConnectionCacheSizeInKibibytes = 2000 / (GRDBStorage.maximumReaderCountInExtensions + 1)

View File

@ -448,8 +448,7 @@ private struct Flags {
// as soon as we fetch an update to the remote config. They will not
// wait for an app restart.
enum HotSwappableIsEnabledFlags: String, FlagType {
// This can't be empty, so we define a bogus case. Remove this if you add a flag here.
case __noHotSwappableIsEnabledFlags
case barrierFsyncKillSwitch
}
// We filter the received config down to just the supported flags.
@ -458,6 +457,7 @@ private struct Flags {
// a sticky flag to 100% in beta then turn it back to 0% before going
// to production.
enum SupportedIsEnabledFlags: String, FlagType {
case barrierFsyncKillSwitch
case uuidSafetyNumbers
case automaticSessionResetKillSwitch
case paymentsResetKillSwitch
@ -661,9 +661,14 @@ public class ServiceRemoteConfigManager: RemoteConfigManager {
private func cacheCurrent() {
var isEnabledFlags = [String: Bool]()
var valueFlags = [String: AnyObject]()
var isUsingBarrierFsync = false
db.read { transaction in
isEnabledFlags = self.keyValueStore.getRemoteConfigIsEnabledFlags(transaction: transaction) ?? [:]
valueFlags = self.keyValueStore.getRemoteConfigValueFlags(transaction: transaction) ?? [:]
isUsingBarrierFsync = SqliteUtil.isUsingBarrierFsync(
db: SDSDB.shimOnlyBridge(transaction).unwrapGrdbRead.database
)
}
if !isEnabledFlags.isEmpty || !valueFlags.isEmpty {
@ -673,6 +678,22 @@ public class ServiceRemoteConfigManager: RemoteConfigManager {
Logger.info("no stored remote config")
}
// This will be tripped in the unlikely event that the kill switch is enabled,
// but typically won't result in a write.
let shouldUseBarrierFsync: Bool = {
let rawFlag = Flags.HotSwappableIsEnabledFlags.barrierFsyncKillSwitch.rawValue
let isKilled = isEnabledFlags[rawFlag] ?? false
return !isKilled
}()
if shouldUseBarrierFsync != isUsingBarrierFsync {
self.db.write { tx in
try? SqliteUtil.setBarrierFsync(
db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database,
enabled: shouldUseBarrierFsync
)
}
}
checkClientExpiration(valueFlags: valueFlags)
hasWarmedCache.set(true)
@ -793,9 +814,19 @@ public class ServiceRemoteConfigManager: RemoteConfigManager {
}
}
try? SqliteUtil.setBarrierFsync(
db: SDSDB.shimOnlyBridge(transaction).unwrapGrdbWrite.database,
enabled: {
let rawFlag = Flags.HotSwappableIsEnabledFlags.barrierFsyncKillSwitch.rawValue
let isKilled = isEnabledFlags[rawFlag] ?? false
return !isKilled
}()
)
self.keyValueStore.setRemoteConfigIsEnabledFlags(isEnabledFlags, transaction: transaction)
self.keyValueStore.setRemoteConfigValueFlags(valueFlags, transaction: transaction)
self.keyValueStore.setLastFetched(Date(), transaction: transaction)
self.checkClientExpiration(valueFlags: valueFlags)
}

View File

@ -40,6 +40,28 @@ public enum SqliteUtil {
sqlName.range(of: "^[a-zA-Z][a-zA-Z0-9_]*$", options: .regularExpression) != nil
}
/// Are we using `F_BARRIERFSYNC`?
///
/// Under the hood, this calls [`PRAGMA fullfsync`][0]. You'd think that this would affect
/// `FULLFSYNC` instead, but we modify SQLCipher to replace `FULLFSYNC` with `FULLFSYNC` with
/// `BARRIERFSYNC`. This helps us balance reliability and performance.
///
/// [0]: https://www.sqlite.org/pragma.html#pragma_fullfsync
public static func isUsingBarrierFsync(db: Database) -> Bool {
return (try? Bool.fetchOne(db, sql: "PRAGMA fullfsync")) ?? false
}
/// Enable or disable `F_BARRIERFSYNC`.
///
/// Under the hood, this calls [`PRAGMA fullfsync`][0]. You'd think that this would affect
/// `FULLFSYNC` instead, but we modify SQLCipher to replace `FULLFSYNC` with `FULLFSYNC` with
/// `BARRIERFSYNC`. This helps us balance reliability and performance.
///
/// [0]: https://www.sqlite.org/pragma.html#pragma_fullfsync
public static func setBarrierFsync(db: Database, enabled: Bool) throws {
try db.execute(sql: "PRAGMA fullfsync = \(enabled ? "ON" : "OFF")")
}
public enum IntegrityCheckResult {
case ok
case notOk

View File

@ -31,6 +31,23 @@ final class SqliteUtilTest: XCTestCase {
}
}
func testBarrierFsync() throws {
let databasePath = OWSFileSystem.temporaryFilePath(fileExtension: "sqlite")
let databaseQueue = try DatabaseQueue(path: databasePath)
defer { try? databaseQueue.close() }
try databaseQueue.write { db in
XCTAssertFalse(SqliteUtil.isUsingBarrierFsync(db: db))
try SqliteUtil.setBarrierFsync(db: db, enabled: true)
XCTAssertTrue(SqliteUtil.isUsingBarrierFsync(db: db))
try SqliteUtil.setBarrierFsync(db: db, enabled: false)
XCTAssertFalse(SqliteUtil.isUsingBarrierFsync(db: db))
}
}
func testIntegrityCheckResult() {
let ok = SqliteUtil.IntegrityCheckResult.ok
let notOk = SqliteUtil.IntegrityCheckResult.notOk