From 9a6025f5d6e378816d20df0d83349fb09553f53f Mon Sep 17 00:00:00 2001 From: Steven Ngesera Date: Fri, 29 Nov 2024 05:27:00 +0300 Subject: [PATCH] refactor(db_session): retry mechanism and lock timeout to optimize session locking and concurrency --- lib/session_db.php | 136 ++++++++++++++++++++++++------------- scripts/setup_database.php | 9 ++- 2 files changed, 96 insertions(+), 49 deletions(-) diff --git a/lib/session_db.php b/lib/session_db.php index 81d0398cb..1ac3ff30c 100644 --- a/lib/session_db.php +++ b/lib/session_db.php @@ -28,12 +28,17 @@ class Hm_DB_Session extends Hm_PHP_Session { */ private $lock_timeout = 10; + private $version = 1; + /** * Create a new session * @return boolean|integer|array */ public function insert_session_row() { - return $this->upsert('insert'); + if (!isset($this->data['hm_version'])) { + $this->data['hm_version'] = 1; + } + return $this->save_data(); } /** @@ -77,17 +82,14 @@ public function start_new($request) { */ public function start_existing($key) { $this->session_key = $key; - if (!$this->acquire_lock($key)) { - Hm_Debug::add('DB SESSION: Failed to acquire lock'); - return; - } $data = $this->get_session_data($key); + if (is_array($data)) { Hm_Debug::add('LOGGED IN'); $this->active = true; - $this->data = $data; + $this->data = $data['data']; + $this->version = $data['hm_version']; } - $this->release_lock($key); } /** @@ -96,9 +98,11 @@ public function start_existing($key) { * @return mixed array results or false on failure */ public function get_session_data($key) { - $results = Hm_DB::execute($this->dbh, 'select data from hm_user_session where hm_id=?', [$key]); - if (is_array($results) && array_key_exists('data', $results)) { - return $this->plaintext($results['data']); + $results = Hm_DB::execute($this->dbh, 'select data, hm_version from hm_user_session where hm_id=?', [$key]); + if (is_array($results)) { + if (array_key_exists('data', $results) && array_key_exists('hm_version', $results)) { + return ['data' => $this->plaintext($results['data']), 'hm_version' => $results['hm_version']]; + } } Hm_Debug::add('DB SESSION failed to read session data'); return false; @@ -139,11 +143,28 @@ public function close_early() { */ public function upsert($type) { $res = false; + $params = [':key' => $this->session_key, ':data' => $this->ciphertext($this->data)]; if ($type == 'update') { - $res = Hm_DB::execute($this->dbh, 'update hm_user_session set data=:data where hm_id=:key', $params); + if ($this->version === null) { + Hm_Debug::add('DB SESSION: Missing hm_version for session key ' . $this->session_key); + return false; + } + $params[':hm_version'] = $this->version; + if (!$this->acquire_lock($this->session_key)) { + Hm_Debug::add('Failed to acquire lock on session'); + return false; + } + $res = Hm_DB::execute($this->dbh, 'update hm_user_session set data=:data, hm_version=hm_version+1 where hm_id=:key and hm_version=:hm_version', $params); + if ($res === 0) { + Hm_Debug::add('Optimistic Locking: hm_version mismatch, session data not updated'); + $this->release_lock($this->session_key); + return false; + } + $this->release_lock($this->session_key); } elseif ($type == 'insert') { - $res = Hm_DB::execute($this->dbh, 'insert into hm_user_session values(:key, :data, current_date)', $params); + $res = Hm_DB::execute($this->dbh, 'insert into hm_user_session (hm_id, data, hm_version, date) values(:key, :data, 1, current_date)', $params); + Hm_Debug::add('Session insert params: ' . json_encode($params)); } if (!$res) { Hm_Debug::add('DB SESSION failed to write session data'); @@ -192,44 +213,67 @@ public function db_start($request) { * @return bool true if lock acquired, false otherwise */ private function acquire_lock($key) { - $lock_name = 'session_lock_' . substr(hash('sha256', $key), 0, 51); + $lock_name = 'session_lock_' . substr(hash('sha256', $key), 0, 51); + + // Polling parameters + $max_attempts = 5; + $retry_interval = 500000; + $attempts = 0; + $query = ''; $params = []; - - switch ($this->db_driver) { - case 'mysql': - $query = 'SELECT GET_LOCK(:lock_name, :timeout)'; - $params = [':lock_name' => $lock_name, ':timeout' => $this->lock_timeout]; - break; - - case 'pgsql': - $query = 'SELECT pg_try_advisory_lock(:hash_key)'; - $params = [':hash_key' => crc32($lock_name)]; - break; - - case 'sqlite': - $query = 'UPDATE hm_user_session SET lock=1 WHERE hm_id=? AND lock=0'; - $params = [$key]; - break; - - default: - Hm_Debug::add('DB SESSION: Unsupported db_driver for locking: ' . $this->db_driver); - return false; - } - - $result = Hm_DB::execute($this->dbh, $query, $params); - if ($this->db_driver == 'mysql') { - return isset($result['GET_LOCK(?, ?)']) && $result['GET_LOCK(?, ?)'] == 1; - } - if ($this->db_driver == 'pgsql') { - return isset($result['pg_try_advisory_lock']) && $result['pg_try_advisory_lock'] === true; - } - - if ($this->db_driver == 'sqlite') { - return isset($result[0]) && $result[0] == 1; + + while ($attempts < $max_attempts) { + switch ($this->db_driver) { + case 'mysql': + $query = 'SELECT GET_LOCK(:lock_name, :timeout)'; + $params = [':lock_name' => $lock_name, ':timeout' => $this->lock_timeout]; + break; + + case 'pgsql': + $query = 'SELECT pg_try_advisory_lock(:hash_key)'; + $params = [':hash_key' => crc32($lock_name)]; + break; + + case 'sqlite': + $query = 'UPDATE hm_user_session SET lock=1 WHERE hm_id=? AND lock=0'; + $params = [$key]; + break; + + default: + Hm_Debug::add('DB SESSION: Unsupported db_driver for locking: ' . $this->db_driver); + return false; + } + + $result = Hm_DB::execute($this->dbh, $query, $params); + + if ($this->db_driver == 'mysql') { + if (isset($result['GET_LOCK(?, ?)']) && $result['GET_LOCK(?, ?)'] == 1) { + return true; + } + } + if ($this->db_driver == 'pgsql') { + if (isset($result['pg_try_advisory_lock']) && $result['pg_try_advisory_lock'] === true) { + return true; + } + } + if ($this->db_driver == 'sqlite') { + if (isset($result[0]) && $result[0] == 1) { + return true; + } + } + + // Lock not acquired, so increment attempts and sleep for retry + $attempts++; + + if ($attempts < $max_attempts) { + usleep($retry_interval); + } } + + Hm_Debug::add('DB SESSION: Failed to acquire lock after ' . $max_attempts . ' attempts.'); return false; - } + } /** * Release a lock for the session (unified for all DB types) diff --git a/scripts/setup_database.php b/scripts/setup_database.php index 2ffceb02b..19ac21c7b 100755 --- a/scripts/setup_database.php +++ b/scripts/setup_database.php @@ -132,19 +132,22 @@ function add_missing_columns($conn, $table_name, $required_columns, $db_driver) $tables = [ 'hm_user_session' => [ 'mysql' => [ - 'hm_id' => 'varchar(255) PRIMARY KEY', - 'data' => 'longblob', - 'date' => 'timestamp', + 'hm_id' => 'varchar(255) PRIMARY KEY', + 'data' => 'longblob', + 'hm_version' => 'INT DEFAULT 1', + 'date' => 'timestamp', ], 'sqlite' => [ 'hm_id' => 'varchar(255) PRIMARY KEY', 'data' => 'longblob', 'lock' => 'INTEGER DEFAULT 0', + 'hm_version' => 'INT DEFAULT 1', 'date' => 'timestamp', ], 'pgsql' => [ 'hm_id' => 'varchar(255) PRIMARY KEY', 'data' => 'text', + 'hm_version' => 'INT DEFAULT 1', 'date' => 'timestamp', ], ],