From 4015f1a110af3dc3b3f53a08c53cdbbc8fddadb3 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 16 Sep 2024 18:20:51 +1200 Subject: [PATCH] NEW Allow database read-only replicas --- cli-script.php | 3 + src/Control/Director.php | 19 ++++ src/Core/CoreKernel.php | 122 +++++++++++++++++------ src/ORM/DB.php | 146 +++++++++++++++++++++++++++- src/ORM/DataObject.php | 8 ++ src/ORM/DataQuery.php | 7 +- src/Security/Group.php | 2 + src/Security/LoginAttempt.php | 2 + src/Security/Member.php | 2 + src/Security/MemberPassword.php | 2 + src/Security/Permission.php | 2 + src/Security/PermissionRole.php | 2 + src/Security/PermissionRoleCode.php | 2 + src/Security/RememberLoginHash.php | 2 + src/Security/Security.php | 8 ++ 15 files changed, 294 insertions(+), 35 deletions(-) diff --git a/cli-script.php b/cli-script.php index 879b2de6546..93b5813ab1d 100755 --- a/cli-script.php +++ b/cli-script.php @@ -16,6 +16,9 @@ die(); } +// CLI scripts must only use the primary database connection and not replicas +DB::setMustUsePrimary(); + // Build request and detect flush $request = CLIRequestBuilder::createFromEnvironment(); diff --git a/src/Control/Director.php b/src/Control/Director.php index 119d4e746d0..3c9ab914ef4 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -16,6 +16,7 @@ use SilverStripe\View\Requirements; use SilverStripe\View\Requirements_Backend; use SilverStripe\View\TemplateGlobalProvider; +use SilverStripe\ORM\DB; /** * Director is responsible for processing URLs, and providing environment information. @@ -83,6 +84,13 @@ class Director implements TemplateGlobalProvider */ private static $default_base_url = '`SS_BASE_URL`'; + /** + * List of rules that must only use the primary database and not a replica + */ + private static array $must_use_primary_db_rules = [ + 'Security' + ]; + public function __construct() { } @@ -295,6 +303,17 @@ public function handleRequest(HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); + // Check if primary database must be used based on request rules + // Note this check must happend before the rules are processed as + // $shiftOnSuccess is true during $request->match() below + $primaryDbOnlyRules = Director::config()->uninherited('must_use_primary_db_rules'); + foreach ($primaryDbOnlyRules as $rule) { + if ($request->match($rule)) { + DB::setMustUsePrimary(); + break; + } + } + $rules = Director::config()->uninherited('rules'); $this->extend('updateRules', $rules); diff --git a/src/Core/CoreKernel.php b/src/Core/CoreKernel.php index 7f968c8a3f4..d1ee5dfa8de 100644 --- a/src/Core/CoreKernel.php +++ b/src/Core/CoreKernel.php @@ -66,31 +66,40 @@ protected function bootDatabaseGlobals() global $databaseConfig; global $database; - // Case 1: $databaseConfig global exists. Merge $database in as needed - if (!empty($databaseConfig)) { - if (!empty($database)) { - $databaseConfig['database'] = $this->getDatabasePrefix() . $database . $this->getDatabaseSuffix(); + for ($i = 0; $i <= 99; $i++) { + if ($i === 0) { + $key = 'default'; + } else { + $key = DB::getReplicaConfigKey($i); + if (!DB::hasConfig($key)) { + break; + } } - // Only set it if its valid, otherwise ignore $databaseConfig entirely - if (!empty($databaseConfig['database'])) { - DB::setConfig($databaseConfig); - - return; + // Case 1: $databaseConfig global exists. Merge $database in as needed + if (!empty($databaseConfig)) { + if (!empty($database)) { + $databaseConfig['database'] = $this->getDatabasePrefix() . $database . $this->getDatabaseSuffix(); + } + + // Only set it if its valid, otherwise ignore $databaseConfig entirely + if (!empty($databaseConfig['database'])) { + DB::setConfig($databaseConfig, $key); + return; + } } - } - - // Case 2: $database merged into existing config - if (!empty($database)) { - $existing = DB::getConfig(); - $existing['database'] = $this->getDatabasePrefix() . $database . $this->getDatabaseSuffix(); - DB::setConfig($existing); + // Case 2: $database merged into existing config + if (!empty($database)) { + $existing = DB::getConfig($key); + $existing['database'] = $this->getDatabasePrefix() . $database . $this->getDatabaseSuffix(); + DB::setConfig($existing, $key); + } } } /** - * Load default database configuration from environment variable + * Load default database configuration from environment variables */ protected function bootDatabaseEnvVars() { @@ -98,20 +107,58 @@ protected function bootDatabaseEnvVars() $databaseConfig = $this->getDatabaseConfig(); $databaseConfig['database'] = $this->getDatabaseName(); DB::setConfig($databaseConfig); + + // Set database replicas config + for ($i = 1; $i <= 99; $i++) { + $envKey = $this->getReplicaEnvKey('SS_DATABASE_SERVER', $i); + if (!Environment::hasEnv($envKey)) { + break; + } + $replicaDatabaseConfig = $this->getDatabaseReplicaConfig($i); + $configKey = DB::getReplicaConfigKey($i); + DB::setConfig($replicaDatabaseConfig, $configKey); + } } /** - * Load database config from environment + * Convert a database key to a replica key + * e.g. SS_DATABASE_SERVER -> SS_DATABASE_SERVER_REPLICA_01 + */ + private function getReplicaEnvKey(string $key, int $replica): string + { + // Left pad replica number with a zero if it's less than 10 + return $key . '_REPLICA_' . str_pad($replica, 2, '0', STR_PAD_LEFT); + } + + /** + * Reads a single database configuration variable from the environment + * For replica databases, it will first attempt to find replica-specific configuration + * before falling back to the default configuration. * - * @return array + * Replicate specific configuration has `_REPLICA_01` appended to the key + * where 01 is the replica number. */ - protected function getDatabaseConfig() + private function getDatabaseConfigVariable(string $key, int $replica): string + { + if ($replica > 0) { + $replicaKey = $this->getReplicaEnvKey($key, $replica); + if (Environment::hasEnv($replicaKey)) { + return Environment::getEnv($replicaKey); + } + } + if (Environment::hasEnv($key)) { + return Environment::getEnv($key); + } + return ''; + } + + private function getSingleDataBaseConfig(int $replica): array { $databaseConfig = [ - "type" => Environment::getEnv('SS_DATABASE_CLASS') ?: 'MySQLDatabase', - "server" => Environment::getEnv('SS_DATABASE_SERVER') ?: 'localhost', - "username" => Environment::getEnv('SS_DATABASE_USERNAME') ?: null, - "password" => Environment::getEnv('SS_DATABASE_PASSWORD') ?: null, + "type" => $this->getDatabaseConfigVariable('SS_DATABASE_CLASS', $replica) ?: 'MySQLDatabase', + "server" => $this->getDatabaseConfigVariable('SS_DATABASE_SERVER', $replica) ?: 'localhost', + "username" => $this->getDatabaseConfigVariable('SS_DATABASE_USERNAME', $replica) ?: null, + "password" => $this->getDatabaseConfigVariable('SS_DATABASE_PASSWORD', $replica) ?: null, ]; // Only add SSL keys in the array if there is an actual value associated with them @@ -122,7 +169,7 @@ protected function getDatabaseConfig() 'ssl_cipher' => 'SS_DATABASE_SSL_CIPHER', ]; foreach ($sslConf as $key => $envVar) { - $envValue = Environment::getEnv($envVar); + $envValue = $this->getDatabaseConfigVariable($envVar, $replica); if ($envValue) { $databaseConfig[$key] = $envValue; } @@ -138,25 +185,25 @@ protected function getDatabaseConfig() } // Set the port if called for - $dbPort = Environment::getEnv('SS_DATABASE_PORT'); + $dbPort = $this->getDatabaseConfigVariable('SS_DATABASE_PORT', $replica); if ($dbPort) { $databaseConfig['port'] = $dbPort; } // Set the timezone if called for - $dbTZ = Environment::getEnv('SS_DATABASE_TIMEZONE'); + $dbTZ = $this->getDatabaseConfigVariable('SS_DATABASE_TIMEZONE', $replica); if ($dbTZ) { $databaseConfig['timezone'] = $dbTZ; } // For schema enabled drivers: - $dbSchema = Environment::getEnv('SS_DATABASE_SCHEMA'); + $dbSchema = $this->getDatabaseConfigVariable('SS_DATABASE_SCHEMA', $replica); if ($dbSchema) { $databaseConfig["schema"] = $dbSchema; } // For SQlite3 memory databases (mainly for testing purposes) - $dbMemory = Environment::getEnv('SS_DATABASE_MEMORY'); + $dbMemory = $this->getDatabaseConfigVariable('SS_DATABASE_MEMORY', $replica); if ($dbMemory) { $databaseConfig["memory"] = $dbMemory; } @@ -166,6 +213,22 @@ protected function getDatabaseConfig() return $databaseConfig; } + /** + * Load database config from environment + * + * @return array + */ + protected function getDatabaseConfig() + { + return $this->getSingleDataBaseConfig(0); + } + + protected function getDatabaseReplicaConfig(int $replica) + { + $replica = 1; + return $this->getSingleDataBaseConfig($replica); + } + /** * @return string */ @@ -184,6 +247,7 @@ protected function getDatabaseSuffix() /** * Get name of database + * Note that any replicas must have the same database name as the primary database * * @return string */ diff --git a/src/ORM/DB.php b/src/ORM/DB.php index 054b858d02f..4af7080d742 100644 --- a/src/ORM/DB.php +++ b/src/ORM/DB.php @@ -9,6 +9,7 @@ use SilverStripe\Core\Convert; use SilverStripe\Core\Environment; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Dev\Install\DatabaseAdapterRegistry; use SilverStripe\ORM\Connect\Database; use SilverStripe\ORM\Connect\DBConnector; use SilverStripe\ORM\Connect\DBSchemaManager; @@ -96,10 +97,21 @@ public static function set_conn(Database $connection, $name = 'default') */ public static function get_conn($name = 'default') { + // Allow default to connect to replica if configured + if ($name === 'default') { + $name = DB::getDefaultConnectionName(); + } + if (isset(DB::$connections[$name])) { return DB::$connections[$name]; } + // Ensure that primary config is set, clone it from default config + if ($name === 'primary' && !static::hasConfig('primary')) { + $config = static::getConfig('default'); + static::setConfig($config, 'primary'); + } + // lazy connect $config = static::getConfig($name); if ($config) { @@ -109,6 +121,102 @@ public static function get_conn($name = 'default') return null; } + /** + * Only use the primary database connection for the rest of the current request + * + * @internal + */ + private static bool $mustUsePrimary = false; + + /** + * Temporarily only use the primary database connection - used in DB::withPrimary() + * + * @internal + */ + private static bool $usePrimary = false; + + /** + * The key of the previously choosen random replica config + * + * @internal + */ + private static string $randomReplicaConfigKey = ''; + + /** + * Set the flag to only use the primary database connection for the current request + * meaning that replia connections will no longer be used + * + * This intentioally does not have a parameter to set this back to false, as this it to prevent + * accidentally attempting writing to a replica, or reading from an out of date replica + * after a write + */ + public static function setMustUsePrimary(): void + { + DB::$mustUsePrimary = true; + } + + /** + * Only use the primary database connection when calling $callback + * Use this when doing non-mutable queries on the primary database where querying + * an out of sync replica could cause issues + * There's no need to use this with mutable queries, or after calling a mutable query + * as the primary database connection will be automatically used + */ + public static function withPrimary(callable $callback): mixed + { + DB::$usePrimary = true; + $result = $callback(); + DB::$usePrimary = false; + return $result; + } + + /** + * Get the name of the database connection to use for the given SQL query + * The 'default' connection can be either the primary or a replica connection if configured + */ + private static function getDefaultConnectionName(string $sql = ''): string + { + if (DB::$mustUsePrimary || DB::$usePrimary || !DB::hasReplicaConfig()) { + return 'primary'; + } + if ($sql) { + $primaryDbClass = DB::getConfig('default')['type']; + $primaryDbObj = Injector::inst()->create($primaryDbClass); + $primaryConnector = $primaryDbObj->getConnector(); + if ($primaryConnector->isQueryMutable($sql)) { + DB::$mustUsePrimary = true; + return 'primary'; + } + } + if (DB::$randomReplicaConfigKey) { + return DB::$randomReplicaConfigKey; + } + $name = DB::getRandomReplicaConfigKey(); + DB::$randomReplicaConfigKey = $name; + return $name; + } + + /** + * Get a random replica database configuration key from the available replica configurations + * The replica choosen will be used for the rest of the request, unless the primary connection + * is forced + */ + private static function getRandomReplicaConfigKey(): string + { + $replicaCount = 0; + for ($i = 1; $i < 99; $i++) { + $replicaKey = DB::getReplicaConfigKey($i); + if (DB::hasConfig($replicaKey)) { + $replicaCount++; + } else { + break; + } + } + // Choose a random replica + $replicaNumber = rand(1, $replicaCount); + return DB::getReplicaConfigKey($replicaNumber); + } + /** * Retrieves the schema manager for the current database * @@ -311,11 +419,39 @@ public static function setConfig($databaseConfig, $name = 'default') */ public static function getConfig($name = 'default') { - if (isset(static::$configs[$name])) { + if (static::hasConfig($name)) { return static::$configs[$name]; } } + /** + * Check if a named connection config exists + */ + public static function hasConfig($name = 'default'): bool + { + return array_key_exists($name, static::$configs); + } + + /** + * Get a replica database configuration key + * e.g. replica_01 + */ + public static function getReplicaConfigKey(int $replica): string + { + return 'replica_' . str_pad($replica, 2, '0', STR_PAD_LEFT); + } + + /** + * Check if there are any replica configurations + */ + public static function hasReplicaConfig(): bool + { + $configKeys = array_keys(static::$configs); + return !empty(array_filter($configKeys, function (string $key) { + return (bool) preg_match('#^replica_[0-9]+$#', $key); + })); + } + /** * Returns true if a database connection has been attempted. * In particular, it lets the caller know if we're still so early in the execution pipeline that @@ -335,8 +471,8 @@ public static function connection_attempted() public static function query($sql, $errorLevel = E_USER_ERROR) { DB::$lastQuery = $sql; - - return DB::get_conn()->query($sql, $errorLevel); + $name = DB::getDefaultConnectionName($sql); + return DB::get_conn($name)->query($sql, $errorLevel); } /** @@ -427,8 +563,8 @@ public static function inline_parameters($sql, $parameters) public static function prepared_query($sql, $parameters, $errorLevel = E_USER_ERROR) { DB::$lastQuery = $sql; - - return DB::get_conn()->preparedQuery($sql, $parameters, $errorLevel); + $name = DB::getDefaultConnectionName($sql); + return DB::get_conn($name)->preparedQuery($sql, $parameters, $errorLevel); } /** diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index c496f631d9f..c30fc7ae6c2 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -141,6 +141,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ private static $default_classname = null; + /** + * Whether this DataObject class must only use the primary database and not a replica + * Note that this will be only be enforced when using DataQuery::execute() or + * another method that uses calls DataQuery::execute() internally e.g. as DataObject::get() + * This will not be enforced when using another method to query data e.g. SQLSelect or DB::query() + */ + private static bool $must_use_primary_db = false; + /** * Data stored in this objects database record. An array indexed by fieldname. * diff --git a/src/ORM/DataQuery.php b/src/ORM/DataQuery.php index c9230986947..1eb745b931b 100644 --- a/src/ORM/DataQuery.php +++ b/src/ORM/DataQuery.php @@ -10,6 +10,7 @@ use SilverStripe\ORM\Queries\SQLConditionGroup; use SilverStripe\ORM\Queries\SQLSelect; use InvalidArgumentException; +use SilverStripe\Core\Config\Config; /** * An object representing a query of data from the DataObject's supporting database. @@ -449,7 +450,11 @@ protected function ensureSelectContainsOrderbyColumns($query, $originalSelect = */ public function execute() { - return $this->getFinalisedQuery()->execute(); + $callback = fn() => $this->getFinalisedQuery()->execute(); + if (Config::inst()->get($this->dataClass(), 'must_use_primary_db')) { + return DB::withPrimary($callback); + } + return $callback(); } /** diff --git a/src/Security/Group.php b/src/Security/Group.php index dff5e81b84e..69efee2c45d 100755 --- a/src/Security/Group.php +++ b/src/Security/Group.php @@ -85,6 +85,8 @@ class Group extends DataObject private static $table_name = "Group"; + private static bool $must_use_primary_db = true; + private static $indexes = [ 'Title' => true, 'Code' => true, diff --git a/src/Security/LoginAttempt.php b/src/Security/LoginAttempt.php index 9ff1b31ce4e..b5142073c66 100644 --- a/src/Security/LoginAttempt.php +++ b/src/Security/LoginAttempt.php @@ -50,6 +50,8 @@ class LoginAttempt extends DataObject private static $table_name = "LoginAttempt"; + private static bool $must_use_primary_db = true; + /** * @param bool $includerelations Indicate if the labels returned include relation fields * @return array diff --git a/src/Security/Member.php b/src/Security/Member.php index 2003e8f05cd..df2d4035475 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -98,6 +98,8 @@ class Member extends DataObject private static $table_name = "Member"; + private static bool $must_use_primary_db = true; + private static $default_sort = '"Surname", "FirstName"'; private static $indexes = [ diff --git a/src/Security/MemberPassword.php b/src/Security/MemberPassword.php index 8488671f6c6..cebb2cce116 100644 --- a/src/Security/MemberPassword.php +++ b/src/Security/MemberPassword.php @@ -27,6 +27,8 @@ class MemberPassword extends DataObject private static $table_name = "MemberPassword"; + private static bool $must_use_primary_db = true; + /** * Log a password change from the given member. * Call MemberPassword::log($this) from within Member whenever the password is changed. diff --git a/src/Security/Permission.php b/src/Security/Permission.php index ce57f81bbf3..88d1e2950f1 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -46,6 +46,8 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl private static $table_name = "Permission"; + private static bool $must_use_primary_db = true; + /** * This is the value to use for the "Type" field if a permission should be * granted. diff --git a/src/Security/PermissionRole.php b/src/Security/PermissionRole.php index 5f96f877024..1e3d88e103d 100644 --- a/src/Security/PermissionRole.php +++ b/src/Security/PermissionRole.php @@ -40,6 +40,8 @@ class PermissionRole extends DataObject private static $table_name = "PermissionRole"; + private static bool $must_use_primary_db = true; + private static $default_sort = '"Title"'; private static $singular_name = 'Role'; diff --git a/src/Security/PermissionRoleCode.php b/src/Security/PermissionRoleCode.php index 8d7e2979bd0..8893382419e 100644 --- a/src/Security/PermissionRoleCode.php +++ b/src/Security/PermissionRoleCode.php @@ -23,6 +23,8 @@ class PermissionRoleCode extends DataObject ]; private static $table_name = "PermissionRoleCode"; + + private static bool $must_use_primary_db = true; private static $indexes = [ "Code" => true, diff --git a/src/Security/RememberLoginHash.php b/src/Security/RememberLoginHash.php index 8af22057353..1895445b226 100644 --- a/src/Security/RememberLoginHash.php +++ b/src/Security/RememberLoginHash.php @@ -44,6 +44,8 @@ class RememberLoginHash extends DataObject private static $table_name = "RememberLoginHash"; + private static bool $must_use_primary_db = true; + /** * Determines if logging out on one device also clears existing login tokens * on all other devices owned by the member. diff --git a/src/Security/Security.php b/src/Security/Security.php index 214dbcb47d8..84a90c82d41 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -437,6 +437,14 @@ public static function permissionFailure($controller = null, $messageSet = null) */ public static function setCurrentUser($currentUser = null) { + // Always use the primary database and not a replica if a user is logged in + // This is to ensure that when logged in as a CMS user, that when viewing content + // from on the frontend it is always up to date i.e. not from an unsynced replica + // This does mean that non-CMS users who are logged in will always use the primary database + // and won't get any performance benefit from replica reads + if ($currentUser) { + DB::setMustUsePrimary(); + } Security::$currentUser = $currentUser; }