From 686394a6c3b5c43dd9a5f7dcfaed93414a31b2a4 Mon Sep 17 00:00:00 2001 From: Stephan Duehr Date: Mon, 15 Jun 2020 12:54:55 +0200 Subject: [PATCH] bvfs: Prevent from unique key violations on .bvfs_update When multiple .bvfs_update operations were run simultaneously and both jobs have many common paths, it was possible that this caused unique constraint violation errors (PostgreSQL) or duplicate entry errors (MySQL/MariaDB). While this did not cause any functional problem, it could bloat logs with lots of error messages in some cases. This change adds an explicit exclusive write lock to prevent from such error messages. --- core/src/cats/bdb_query_enum_class.h | 4 +++- core/src/cats/bdb_query_names.inc | 2 ++ core/src/cats/bvfs.cc | 11 +++++++++++ core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0 | 1 + .../cats/dml/0078_bvfs_lock_pathhierarchy_0.mysql | 1 + .../dml/0078_bvfs_lock_pathhierarchy_0.postgresql | 1 + core/src/cats/dml/0079_bvfs_unlock_tables_0 | 1 + core/src/cats/dml/0079_bvfs_unlock_tables_0.mysql | 1 + core/src/cats/mysql_queries.inc | 8 ++++++++ core/src/cats/postgresql_queries.inc | 14 +++++++++++--- core/src/cats/sqlite_queries.inc | 14 +++++++++++--- 11 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0 create mode 100644 core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.mysql create mode 100644 core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.postgresql create mode 100644 core/src/cats/dml/0079_bvfs_unlock_tables_0 create mode 100644 core/src/cats/dml/0079_bvfs_unlock_tables_0.mysql diff --git a/core/src/cats/bdb_query_enum_class.h b/core/src/cats/bdb_query_enum_class.h index 02f4ec758f0..20384be3bf7 100644 --- a/core/src/cats/bdb_query_enum_class.h +++ b/core/src/cats/bdb_query_enum_class.h @@ -81,6 +81,8 @@ class BareosDbQueryEnum { bvfs_ls_sub_dirs_5 = 74, list_volumes_select_0 = 75, list_volumes_select_long_0 = 76, - SQL_QUERY_NUMBER = 77 + bvfs_lock_pathhierarchy_0 = 77, + bvfs_unlock_tables_0 = 78, + SQL_QUERY_NUMBER = 79 }; }; diff --git a/core/src/cats/bdb_query_names.inc b/core/src/cats/bdb_query_names.inc index 114b5992774..0c4ae7ebdd6 100644 --- a/core/src/cats/bdb_query_names.inc +++ b/core/src/cats/bdb_query_names.inc @@ -78,5 +78,7 @@ const char *BareosDb::query_names[] = { "bvfs_ls_sub_dirs_5", "list_volumes_select_0", "list_volumes_select_long_0", +"bvfs_lock_pathhierarchy_0", +"bvfs_unlock_tables_0", NULL }; diff --git a/core/src/cats/bvfs.cc b/core/src/cats/bvfs.cc index 277ffed22df..f62a546a48d 100644 --- a/core/src/cats/bvfs.cc +++ b/core/src/cats/bvfs.cc @@ -283,6 +283,14 @@ bool BareosDb::UpdatePathHierarchyCache(JobControlRecord* jcr, result[i++] = strdup(row[1]); } + /* The PathHierarchy table needs exclusive write lock here to + * prevent from unique key constraint violations (PostgreSQL) + * or duplicate entry errors (MySQL/MariaDB) when multiple + * bvfs update operations are run simultaneously. + */ + FillQuery(cmd, SQL_QUERY::bvfs_lock_pathhierarchy_0); + if (!QUERY_DB(jcr, cmd)) { goto bail_out; } + i = 0; while (num > 0) { BuildPathHierarchy(jcr, ppathid_cache, result[i], result[i + 1]); @@ -291,6 +299,9 @@ bool BareosDb::UpdatePathHierarchyCache(JobControlRecord* jcr, num--; } free(result); + + FillQuery(cmd, SQL_QUERY::bvfs_unlock_tables_0); + if (!QUERY_DB(jcr, cmd)) { goto bail_out; } } StartTransaction(jcr); diff --git a/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0 b/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0 new file mode 100644 index 00000000000..1bf4c3f4e04 --- /dev/null +++ b/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0 @@ -0,0 +1 @@ +BEGIN diff --git a/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.mysql b/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.mysql new file mode 100644 index 00000000000..21c49263544 --- /dev/null +++ b/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.mysql @@ -0,0 +1 @@ +LOCK TABLES PathHierarchy write, Path write diff --git a/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.postgresql b/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.postgresql new file mode 100644 index 00000000000..df235994841 --- /dev/null +++ b/core/src/cats/dml/0078_bvfs_lock_pathhierarchy_0.postgresql @@ -0,0 +1 @@ +BEGIN; LOCK TABLE PathHierarchy IN SHARE ROW EXCLUSIVE MODE diff --git a/core/src/cats/dml/0079_bvfs_unlock_tables_0 b/core/src/cats/dml/0079_bvfs_unlock_tables_0 new file mode 100644 index 00000000000..2a4398d82dc --- /dev/null +++ b/core/src/cats/dml/0079_bvfs_unlock_tables_0 @@ -0,0 +1 @@ +COMMIT diff --git a/core/src/cats/dml/0079_bvfs_unlock_tables_0.mysql b/core/src/cats/dml/0079_bvfs_unlock_tables_0.mysql new file mode 100644 index 00000000000..f882403b9ca --- /dev/null +++ b/core/src/cats/dml/0079_bvfs_unlock_tables_0.mysql @@ -0,0 +1 @@ +UNLOCK TABLES diff --git a/core/src/cats/mysql_queries.inc b/core/src/cats/mysql_queries.inc index 3e229203efb..b052c5c12dd 100644 --- a/core/src/cats/mysql_queries.inc +++ b/core/src/cats/mysql_queries.inc @@ -1008,5 +1008,13 @@ const char *BareosDbMysql::query_definitions[] = { "LEFT JOIN Storage USING(StorageId) " , +/* 0078_bvfs_lock_pathhierarchy_0.mysql */ +"LOCK TABLES PathHierarchy write, Path write " +, + +/* 0079_bvfs_unlock_tables_0.mysql */ +"UNLOCK TABLES " +, + NULL }; diff --git a/core/src/cats/postgresql_queries.inc b/core/src/cats/postgresql_queries.inc index 523b8bc6655..3b22e04564b 100644 --- a/core/src/cats/postgresql_queries.inc +++ b/core/src/cats/postgresql_queries.inc @@ -545,9 +545,9 @@ const char *BareosDbPostgresql::query_definitions[] = { "Path.Path||File.Name AS Name, " "StartTime, " "TYPE AS JobType, " - "JobStatus, " - "JobFiles, " - "JobBytes " + "JobStatus, " + "JobFiles, " + "JobBytes " "FROM Client, " "Job, " "File, " @@ -987,5 +987,13 @@ const char *BareosDbPostgresql::query_definitions[] = { "LEFT JOIN Storage USING(StorageId) " , +/* 0078_bvfs_lock_pathhierarchy_0.postgresql */ +"BEGIN; LOCK TABLE PathHierarchy IN SHARE ROW EXCLUSIVE MODE " +, + +/* 0079_bvfs_unlock_tables_0 */ +"COMMIT " +, + NULL }; diff --git a/core/src/cats/sqlite_queries.inc b/core/src/cats/sqlite_queries.inc index 31d4d5e03ea..f01b78474c1 100644 --- a/core/src/cats/sqlite_queries.inc +++ b/core/src/cats/sqlite_queries.inc @@ -565,9 +565,9 @@ const char *BareosDbSqlite::query_definitions[] = { "Path.Path||File.Name AS Name, " "StartTime, " "TYPE AS JobType, " - "JobStatus, " - "JobFiles, " - "JobBytes " + "JobStatus, " + "JobFiles, " + "JobBytes " "FROM Client, " "Job, " "File, " @@ -993,5 +993,13 @@ const char *BareosDbSqlite::query_definitions[] = { "LEFT JOIN Storage USING(StorageId) " , +/* 0078_bvfs_lock_pathhierarchy_0 */ +"BEGIN " +, + +/* 0079_bvfs_unlock_tables_0 */ +"COMMIT " +, + NULL };