Fixed redundancy in sql caching system and compatibility with Galera Cluster (#5439)

- Removed redundant 'created' column from cache and cache_shared tables
- Removed use of redundant data records
- Added missing primary keys (dictionary, cache, cache_shared tables)

CHANGELOG

@@ -63,6 +63,10 @@ CHANGELOG Roundcube Webmail
 - Managesieve: Unhide advanced rule controls if there are inputs with errors
 - Managesieve: Display warning message when filter form contains errors
 - Control search engine crawlers via X-Robots-Tag header instead of <meta> and robots.txt (#5098)
+- Fixed redundancy in sql caching system and compatibility with Galera Cluster (#5439)
+    - Removed redundant 'created' column from cache and cache_shared tables
+    - Removed use of redundant data records
+    - Added missing primary keys (dictionary, cache, cache_shared tables)
 - Fix so templating system does not mess with external (e.g. email) content (#5499)
 - Fix redundant keep-alive/refresh after session error on compose page (#5500)
 - Fix flickering of header topline in min-mode (#5426)

SQL/mssql.initial.sql

@@ -8,7 +8,6 @@ GO
 
 CREATE TABLE [dbo].[cache_shared] (
 	[cache_key] [varchar] (255) COLLATE Latin1_General_CI_AI NOT NULL ,
-	[created] [datetime] NOT NULL ,
 	[expires] [datetime] NULL ,
 	[data] [text] COLLATE Latin1_General_CI_AI NOT NULL 
 ) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
@@ -131,6 +130,20 @@ CREATE TABLE [dbo].[system] (
 ) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
 GO
 
+ALTER TABLE [dbo].[cache] WITH NOCHECK ADD 
+	 PRIMARY KEY CLUSTERED 
+	(
+		[user_id],[cache_key]
+	) ON [PRIMARY] 
+GO
+
+ALTER TABLE [dbo].[cache_shared] WITH NOCHECK ADD 
+	 PRIMARY KEY CLUSTERED 
+	(
+		[cache_key]
+	) ON [PRIMARY] 
+GO
+
 ALTER TABLE [dbo].[cache_index] WITH NOCHECK ADD 
 	 PRIMARY KEY CLUSTERED 
 	(
@@ -211,11 +224,6 @@ GO
 ALTER TABLE [dbo].[cache] ADD 
 	CONSTRAINT [DF_cache_user_id] DEFAULT ('0') FOR [user_id],
 	CONSTRAINT [DF_cache_cache_key] DEFAULT ('') FOR [cache_key],
-	CONSTRAINT [DF_cache_created] DEFAULT (getdate()) FOR [created]
-GO
-
-ALTER TABLE [dbo].[cache_shared] ADD 
-	CONSTRAINT [DF_cache_shared_created] DEFAULT (getdate()) FOR [created]
 GO
 
 ALTER TABLE [dbo].[cache_index] ADD 
@@ -226,12 +234,6 @@ ALTER TABLE [dbo].[cache_messages] ADD
 	CONSTRAINT [DF_cache_messages_flags] DEFAULT (0) FOR [flags]
 GO
 
-CREATE INDEX [IX_cache_user_id] ON [dbo].[cache]([user_id]) ON [PRIMARY]
-GO
-
-CREATE INDEX [IX_cache_cache_key] ON [dbo].[cache]([cache_key]) ON [PRIMARY]
-GO
-
 CREATE INDEX [IX_cache_shared_cache_key] ON [dbo].[cache_shared]([cache_key]) ON [PRIMARY]
 GO
 
@@ -393,6 +395,6 @@ CREATE TRIGGER [contact_delete_member] ON [dbo].[contacts]
     WHERE [contact_id] IN (SELECT [contact_id] FROM deleted)
 GO
 
-INSERT INTO [dbo].[system] ([name], [value]) VALUES ('roundcube-version', '2016100900')
+INSERT INTO [dbo].[system] ([name], [value]) VALUES ('roundcube-version', '2016112200')
 GO
 

SQL/mssql/2016112200.sql

@@ -0,0 +1,36 @@
+DROP TABLE [dbo].[cache]
+GO
+DROP TABLE [dbo].[cache_shared]
+GO
+
+CREATE TABLE [dbo].[cache] (
+  [user_id] [int] NOT NULL ,
+  [cache_key] [varchar] (128) COLLATE Latin1_General_CI_AI NOT NULL ,
+  [expires] [datetime] NULL ,
+  [data] [text] COLLATE Latin1_General_CI_AI NOT NULL 
+) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
+GO
+CREATE TABLE [dbo].[cache_shared] (
+  [cache_key] [varchar] (255) COLLATE Latin1_General_CI_AI NOT NULL ,
+  [expires] [datetime] NULL ,
+  [data] [text] COLLATE Latin1_General_CI_AI NOT NULL
+) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
+GO
+ALTER TABLE [dbo].[cache] ADD
+  CONSTRAINT [DF_cache_user_id] DEFAULT ('0') FOR [user_id],
+  CONSTRAINT [DF_cache_cache_key] DEFAULT ('') FOR [cache_key],
+GO
+CREATE INDEX [IX_cache_expires] ON [dbo].[cache]([expires]) ON [PRIMARY]
+GO
+CREATE INDEX [IX_cache_shared_expires] ON [dbo].[cache_shared]([expires]) ON [PRIMARY]
+GO
+ALTER TABLE [dbo].[cache] WITH NOCHECK ADD
+  PRIMARY KEY CLUSTERED (
+    [user_id],[cache_key]
+  ) ON [PRIMARY]
+GO
+ALTER TABLE [dbo].[cache_shared] WITH NOCHECK ADD
+  PRIMARY KEY CLUSTERED (
+    [cache_key]
+  ) ON [PRIMARY]
+GO

SQL/mysql.initial.sql

@@ -37,13 +37,12 @@ CREATE TABLE `users` (
 CREATE TABLE `cache` (
  `user_id` int(10) UNSIGNED NOT NULL,
  `cache_key` varchar(128) /*!40101 CHARACTER SET ascii COLLATE ascii_general_ci */ NOT NULL,
- `created` datetime NOT NULL DEFAULT '1000-01-01 00:00:00',
  `expires` datetime DEFAULT NULL,
  `data` longtext NOT NULL,
+ PRIMARY KEY (`user_id`, `cache_key`),
  CONSTRAINT `user_id_fk_cache` FOREIGN KEY (`user_id`)
    REFERENCES `users`(`user_id`) ON DELETE CASCADE ON UPDATE CASCADE,
- INDEX `expires_index` (`expires`),
- INDEX `user_cache_index` (`user_id`,`cache_key`)
+ INDEX `expires_index` (`expires`)
 ) /*!40000 ENGINE=INNODB */ /*!40101 CHARACTER SET utf8 COLLATE utf8_general_ci */;
 
 
@@ -51,11 +50,10 @@ CREATE TABLE `cache` (
 
 CREATE TABLE `cache_shared` (
  `cache_key` varchar(255) /*!40101 CHARACTER SET ascii COLLATE ascii_general_ci */ NOT NULL,
- `created` datetime NOT NULL DEFAULT '1000-01-01 00:00:00',
  `expires` datetime DEFAULT NULL,
  `data` longtext NOT NULL,
- INDEX `expires_index` (`expires`),
- INDEX `cache_key_index` (`cache_key`)
+ PRIMARY KEY (`cache_key`),
+ INDEX `expires_index` (`expires`)
 ) /*!40000 ENGINE=INNODB */ /*!40101 CHARACTER SET utf8 COLLATE utf8_general_ci */;
 
 
@@ -176,7 +174,8 @@ CREATE TABLE `identities` (
 -- Table structure for table `dictionary`
 
 CREATE TABLE `dictionary` (
-  `user_id` int(10) UNSIGNED DEFAULT NULL,
+  `id` int(10) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, -- redundant, for compat. with Galera Cluster
+  `user_id` int(10) UNSIGNED DEFAULT NULL, -- NULL here is for "shared dictionaries"
   `language` varchar(5) NOT NULL,
   `data` longtext NOT NULL,
   CONSTRAINT `user_id_fk_dictionary` FOREIGN KEY (`user_id`)
@@ -210,4 +209,4 @@ CREATE TABLE `system` (
 
 /*!40014 SET FOREIGN_KEY_CHECKS=1 */;
 
-INSERT INTO system (name, value) VALUES ('roundcube-version', '2016100900');
+INSERT INTO system (name, value) VALUES ('roundcube-version', '2016112200');

SQL/mysql/2016112200.sql

@@ -0,0 +1,24 @@
+ALTER TABLE `dictionary` ADD COLUMN `id` int(10) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY FIRST; -- redundant, for compat. with Galera Cluster
+
+DROP TABLE `cache`;
+DROP TABLE `cache_shared`;
+
+CREATE TABLE `cache` (
+ `user_id` int(10) UNSIGNED NOT NULL,
+ `cache_key` varchar(128) /*!40101 CHARACTER SET ascii COLLATE ascii_general_ci */ NOT NULL,
+ `expires` datetime DEFAULT NULL,
+ `data` longtext NOT NULL,
+ PRIMARY KEY (`user_id`, `cache_key`),
+ CONSTRAINT `user_id_fk_cache` FOREIGN KEY (`user_id`)
+   REFERENCES `users`(`user_id`) ON DELETE CASCADE ON UPDATE CASCADE,
+ INDEX `expires_index` (`expires`)
+) /*!40000 ENGINE=INNODB */ /*!40101 CHARACTER SET utf8 COLLATE utf8_general_ci */;
+
+
+CREATE TABLE `cache_shared` (
+ `cache_key` varchar(255) /*!40101 CHARACTER SET ascii COLLATE ascii_general_ci */ NOT NULL,
+ `expires` datetime DEFAULT NULL,
+ `data` longtext NOT NULL,
+ PRIMARY KEY (`cache_key`),
+ INDEX `expires_index` (`expires`)
+) /*!40000 ENGINE=INNODB */ /*!40101 CHARACTER SET utf8 COLLATE utf8_general_ci */;

SQL/oracle.initial.sql

@@ -126,23 +126,21 @@ CREATE TABLE "cache" (
     "user_id" integer NOT NULL
         REFERENCES "users" ("user_id") ON DELETE CASCADE,
     "cache_key" varchar(128) NOT NULL,
-    "created" timestamp with time zone DEFAULT current_timestamp NOT NULL,
     "expires" timestamp with time zone DEFAULT NULL,
-    "data" long NOT NULL
+    "data" long NOT NULL,
+    PRIMARY KEY ("user_id", "cache_key")
 );
 
-CREATE INDEX "cache_user_id_idx" ON "cache" ("user_id", "cache_key");
 CREATE INDEX "cache_expires_idx" ON "cache" ("expires");
 
 
 CREATE TABLE "cache_shared" (
     "cache_key" varchar(255) NOT NULL,
-    "created" timestamp with time zone DEFAULT current_timestamp NOT NULL,
     "expires" timestamp with time zone DEFAULT NULL,
-    "data" long NOT NULL
+    "data" long NOT NULL,
+    PRIMARY KEY ("cache_key")
 );
 
-CREATE INDEX "cache_shared_cache_key_idx" ON "cache_shared" ("cache_key");
 CREATE INDEX "cache_shared_expires_idx" ON "cache_shared" ("expires");
 
 
@@ -219,4 +217,4 @@ CREATE TABLE "system" (
     "value" long
 );
 
-INSERT INTO "system" ("name", "value") VALUES ('roundcube-version', '2016100900');
+INSERT INTO "system" ("name", "value") VALUES ('roundcube-version', '2016112200');

SQL/oracle/2016100900.sql

@@ -1 +1 @@
-ALTER TABLE session MODIFY ip varchar(41) NOT NULL;
+ALTER TABLE "session" MODIFY "ip" varchar(41) NOT NULL;

SQL/oracle/2016112200.sql

@@ -0,0 +1,23 @@
+DROP TABLE "cache";
+DROP TABLE "cache_shared";
+
+CREATE TABLE "cache" (
+    "user_id" integer NOT NULL
+        REFERENCES "users" ("user_id") ON DELETE CASCADE,
+    "cache_key" varchar(128) NOT NULL,
+    "expires" timestamp with time zone DEFAULT NULL,
+    "data" long NOT NULL,
+    PRIMARY KEY ("user_id", "cache_key")
+);
+
+CREATE INDEX "cache_expires_idx" ON "cache" ("expires");
+
+
+CREATE TABLE "cache_shared" (
+    "cache_key" varchar(255) NOT NULL,
+    "expires" timestamp with time zone DEFAULT NULL,
+    "data" long NOT NULL,
+    PRIMARY KEY ("cache_key")
+);
+
+CREATE INDEX "cache_shared_expires_idx" ON "cache_shared" ("expires");

SQL/postgres.initial.sql

@@ -167,12 +167,11 @@ CREATE TABLE "cache" (
     user_id integer NOT NULL
         REFERENCES users (user_id) ON DELETE CASCADE ON UPDATE CASCADE,
     cache_key varchar(128) DEFAULT '' NOT NULL,
-    created timestamp with time zone DEFAULT now() NOT NULL,
     expires timestamp with time zone DEFAULT NULL,
-    data text NOT NULL
+    data text NOT NULL,
+    PRIMARY KEY (user_id, cache_key)
 );
 
-CREATE INDEX cache_user_id_idx ON "cache" (user_id, cache_key);
 CREATE INDEX cache_expires_idx ON "cache" (expires);
 
 --
@@ -181,13 +180,11 @@ CREATE INDEX cache_expires_idx ON "cache" (expires);
 --
 
 CREATE TABLE "cache_shared" (
-    cache_key varchar(255) NOT NULL,
-    created timestamp with time zone DEFAULT now() NOT NULL,
+    cache_key varchar(255) NOT NULL PRIMARY KEY,
     expires timestamp with time zone DEFAULT NULL,
     data text NOT NULL
 );
 
-CREATE INDEX cache_shared_cache_key_idx ON "cache_shared" (cache_key);
 CREATE INDEX cache_shared_expires_idx ON "cache_shared" (expires);
 
 --
@@ -291,4 +288,4 @@ CREATE TABLE "system" (
     value text
 );
 
-INSERT INTO system (name, value) VALUES ('roundcube-version', '2016100900');
+INSERT INTO system (name, value) VALUES ('roundcube-version', '2016112200');

SQL/postgres/2016112200.sql

@@ -0,0 +1,21 @@
+DROP TABLE "cache";
+DROP TABLE "cache_shared";
+
+CREATE TABLE "cache" (
+    user_id integer NOT NULL
+        REFERENCES users (user_id) ON DELETE CASCADE ON UPDATE CASCADE,
+    cache_key varchar(128) DEFAULT '' NOT NULL,
+    expires timestamp with time zone DEFAULT NULL,
+    data text NOT NULL,
+    PRIMARY KEY (user_id, cache_key)
+);
+
+CREATE INDEX cache_expires_idx ON "cache" (expires);
+
+CREATE TABLE "cache_shared" (
+    cache_key varchar(255) NOT NULL PRIMARY KEY,
+    expires timestamp with time zone DEFAULT NULL,
+    data text NOT NULL
+);
+
+CREATE INDEX cache_shared_expires_idx ON "cache_shared" (expires);

SQL/sqlite.initial.sql

@@ -126,12 +126,11 @@ CREATE UNIQUE INDEX ix_searches_user_type_name ON searches (user_id, type, name)
 CREATE TABLE cache (
   user_id integer NOT NULL default 0,
   cache_key varchar(128) NOT NULL default '',
-  created datetime NOT NULL default '0000-00-00 00:00:00',
   expires datetime DEFAULT NULL,
-  data text NOT NULL
+  data text NOT NULL,
+  PRIMARY KEY (user_id, cache_key)
 );
 
-CREATE INDEX ix_cache_user_cache_key ON cache(user_id, cache_key);
 CREATE INDEX ix_cache_expires ON cache(expires);
 
 -- 
@@ -140,12 +139,11 @@ CREATE INDEX ix_cache_expires ON cache(expires);
 
 CREATE TABLE cache_shared (
   cache_key varchar(255) NOT NULL,
-  created datetime NOT NULL default '0000-00-00 00:00:00',
   expires datetime DEFAULT NULL,
-  data text NOT NULL
+  data text NOT NULL,
+  PRIMARY KEY (cache_key)
 );
 
-CREATE INDEX ix_cache_shared_cache_key ON cache_shared(cache_key);
 CREATE INDEX ix_cache_shared_expires ON cache_shared(expires);
 
 --
@@ -202,4 +200,4 @@ CREATE TABLE system (
   value text NOT NULL
 );
 
-INSERT INTO system (name, value) VALUES ('roundcube-version', '2016100900');
+INSERT INTO system (name, value) VALUES ('roundcube-version', '2016112200');

SQL/sqlite/2016112200.sql

@@ -0,0 +1,21 @@
+DROP TABLE cache;
+DROP TABLE cache_shared;
+
+CREATE TABLE cache (
+  user_id integer NOT NULL default 0,
+  cache_key varchar(128) NOT NULL default '',
+  expires datetime DEFAULT NULL,
+  data text NOT NULL,
+  PRIMARY KEY (user_id, cache_key)
+);
+
+CREATE INDEX ix_cache_expires ON cache(expires);
+
+CREATE TABLE cache_shared (
+  cache_key varchar(255) NOT NULL,
+  expires datetime DEFAULT NULL,
+  data text NOT NULL,
+  PRIMARY KEY (cache_key)
+);
+
+CREATE INDEX ix_cache_shared_expires ON cache_shared(expires);

program/lib/Roundcube/rcube_cache.php

@@ -296,14 +296,10 @@ class rcube_cache
             }
         }
         else {
-            $sql_result = $this->db->limitquery(
-                "SELECT `data`, `cache_key`".
-                " FROM {$this->table}".
-                " WHERE `user_id` = ? AND `cache_key` = ?".
-                // for better performance we allow more records for one key
-                // get the newer one
-                " ORDER BY `created` DESC",
-                0, 1, $this->userid, $this->prefix.'.'.$key);
+            $sql_result = $this->db->query(
+                "SELECT `data`, `cache_key` FROM {$this->table}"
+                . " WHERE `user_id` = ? AND `cache_key` = ?",
+                $this->userid, $this->prefix.'.'.$key);
 
             if ($sql_arr = $this->db->fetch_assoc($sql_result)) {
                 if (strlen($sql_arr['data']) > 0) {
@@ -367,42 +363,44 @@ class rcube_cache
             return $result;
         }
 
-        $key_exists = array_key_exists($key, $this->cache_sums);
-        $key        = $this->prefix . '.' . $key;
+        $db_key = $this->prefix . '.' . $key;
 
         // Remove NULL rows (here we don't need to check if the record exist)
         if ($data == 'N;') {
-            $this->db->query(
+            $result = $this->db->query(
                 "DELETE FROM {$this->table}".
                 " WHERE `user_id` = ? AND `cache_key` = ?",
-                $this->userid, $key);
+                $this->userid, $db_key);
 
-            return true;
+            return !$this->db->is_error($result);
         }
 
-        // update existing cache record
-        if ($key_exists) {
+        $key_exists = array_key_exists($key, $this->cache_sums);
+        $expires    = $this->ttl ? $this->db->now($this->ttl) : 'NULL';
+
+        if (!$key_exists) {
+            // Try INSERT temporarily ignoring "duplicate key" errors
+            $this->db->set_option('ignore_key_errors', true);
+
             $result = $this->db->query(
-                "UPDATE {$this->table}".
-                " SET `created` = " . $this->db->now().
-                    ", `expires` = " . ($this->ttl ? $this->db->now($this->ttl) : 'NULL').
-                    ", `data` = ?".
-                " WHERE `user_id` = ?".
-                " AND `cache_key` = ?",
-                $data, $this->userid, $key);
+                "INSERT INTO {$this->table} (`expires`, `user_id`, `cache_key`, `data`)"
+                . " VALUES ($expires, ?, ?, ?)",
+                $this->userid, $db_key, $data);
+
+            $this->db->set_option('ignore_key_errors', false);
         }
-        // add new cache record
-        else {
-            // for better performance we allow more records for one key
-            // so, no need to check if record exist (see rcube_cache::read_record())
+
+        // otherwise try UPDATE
+        if (!isset($result) || !($count = $this->db->affected_rows($result))) {
             $result = $this->db->query(
-                "INSERT INTO {$this->table}".
-                " (`created`, `expires`, `user_id`, `cache_key`, `data`)".
-                " VALUES (" . $this->db->now() . ", " . ($this->ttl ? $this->db->now($this->ttl) : 'NULL') . ", ?, ?, ?)",
-                $this->userid, $key, $data);
+                "UPDATE {$this->table} SET `expires` = $expires, `data` = ?"
+                . " WHERE `user_id` = ? AND `cache_key` = ?",
+                $data, $this->userid, $db_key);
+
+            $count = $this->db->affected_rows($result);
         }
 
-        return $this->db->affected_rows($result) > 0;
+        return $count > 0;
     }
 
     /**

program/lib/Roundcube/rcube_cache_shared.php

@@ -291,14 +291,10 @@ class rcube_cache_shared
             }
         }
         else {
-            $sql_result = $this->db->limitquery(
-                "SELECT `data`, `cache_key`".
-                " FROM {$this->table}" .
-                " WHERE `cache_key` = ?".
-                // for better performance we allow more records for one key
-                // get the newer one
-                " ORDER BY `created` DESC",
-                0, 1, $this->prefix . '.' . $key);
+            $sql_result = $this->db->query(
+                "SELECT `data`, `cache_key` FROM {$this->table}"
+                . " WHERE `cache_key` = ?",
+                $this->prefix . '.' . $key);
 
             if ($sql_arr = $this->db->fetch_assoc($sql_result)) {
                 if (strlen($sql_arr['data']) > 0) {
@@ -362,37 +358,41 @@ class rcube_cache_shared
             return $result;
         }
 
-        $key_exists = array_key_exists($key, $this->cache_sums);
-        $key        = $this->prefix . '.' . $key;
+        $db_key = $this->prefix . '.' . $key;
 
         // Remove NULL rows (here we don't need to check if the record exist)
         if ($data == 'N;') {
-            $this->db->query("DELETE FROM {$this->table} WHERE `cache_key` = ?", $key);
-            return true;
+            $result = $this->db->query("DELETE FROM {$this->table} WHERE `cache_key` = ?", $db_key);
+
+            return !$this->db->is_error($result);
         }
 
-        // update existing cache record
-        if ($key_exists) {
+        $key_exists = array_key_exists($key, $this->cache_sums);
+        $expires    = $this->ttl ? $this->db->now($this->ttl) : 'NULL';
+
+        if (!$key_exists) {
+            // Try INSERT temporarily ignoring "duplicate key" errors
+            $this->db->set_option('ignore_key_errors', true);
+
             $result = $this->db->query(
-                "UPDATE {$this->table}" .
-                " SET `created` = " . $this->db->now() .
-                    ", `expires` = " . ($this->ttl ? $this->db->now($this->ttl) : 'NULL') .
-                    ", `data` = ?".
-                " WHERE `cache_key` = ?",
-                $data, $key);
-        }
-        // add new cache record
-        else {
-            // for better performance we allow more records for one key
-            // so, no need to check if record exist (see rcube_cache::read_record())
+                "INSERT INTO {$this->table} (`expires`, `cache_key`, `data`)"
+                . " VALUES ($expires, ?, ?)",
+                $db_key, $data);
+
+            $this->db->set_option('ignore_key_errors', false);
+        }
+
+        // otherwise try UPDATE
+        if (!isset($result) || !($count = $this->db->affected_rows($result))) {
             $result = $this->db->query(
-                "INSERT INTO {$this->table}".
-                " (`created`, `expires`, `cache_key`, `data`)".
-                " VALUES (".$this->db->now().", " . ($this->ttl ? $this->db->now($this->ttl) : 'NULL') . ", ?, ?)",
-                $key, $data);
+                "UPDATE {$this->table} SET `expires` = $expires, `data` = ?"
+                . " WHERE `cache_key` = ?",
+                $data, $db_key);
+
+            $count = $this->db->affected_rows($result);
         }
 
-        return $this->db->affected_rows($result) > 0;
+        return $count > 0;
     }
 
     /**