Fix Mojo::Pg / database handle confusion.
authorVincent Tondellier <tonton+hg@team1664.org>
Sun, 02 Apr 2017 14:36:18 +0200
changeset 127 0bbbadd5d9ea
parent 126 01078f0097b3
child 128 6b56182fd491
Fix Mojo::Pg / database handle confusion. Only allocate a db connection when we're about to use it, don't store it in the class. This fixes transactions and reconnections, and also lower the total number of db connections
lib/CrashTest/Plugin/Storage/Sql/Model/CrashGroup.pm
lib/CrashTest/Plugin/Storage/Sql/Model/CrashReport.pm
lib/CrashTest/Plugin/Storage/Sql/Utils.pm
--- a/lib/CrashTest/Plugin/Storage/Sql/Model/CrashGroup.pm	Sat Dec 31 20:34:01 2016 +0100
+++ b/lib/CrashTest/Plugin/Storage/Sql/Model/CrashGroup.pm	Sun Apr 02 14:36:18 2017 +0200
@@ -23,17 +23,22 @@
 
 has [ qw/instance app config/ ];
 
-has [ qw/db sql_utils/ ];
+has [ qw/dbh sql_utils/ ];
 
 sub new {
     my $self = shift->SUPER::new(@_);
 
-    $self->db($self->instance->dbh->db);
+    $self->dbh($self->instance->dbh);
     $self->sql_utils(CrashTest::Plugin::Storage::Sql::Utils->new(@_));
 
     return $self;
 }
 
+sub db {
+    my ($self) = @_;
+    return $self->dbh->db;
+}
+
 sub _build_query_from_search_string {
     my ($self, $search_str) = @_;
     return $self->sql_utils->build_query_from_search_string(
@@ -51,6 +56,8 @@
 sub index {
     my ($self, $pagen, $nperpage, $search_str) = @_;
 
+    my $db = $self->db;
+
     my $where = "";
     my @values = ();
     my $err;
@@ -63,7 +70,7 @@
         $err = $@;
     }
 
-    my $count = $self->db->query("
+    my $count = $db->query("
         SELECT count(distinct(crash_group_id)) AS total
         FROM crash_reports
         LEFT JOIN crash_users AS crash_user ON crash_reports.crash_user_id = crash_user.id
@@ -83,7 +90,7 @@
     }
     my $extra_columns = join(",", @extra_cols);
 
-    my $results = $self->db->query("
+    my $results = $db->query("
         SELECT crash_groups.uuid, title, group_by_count.*,
             (SELECT json_agg(to_json(bug_links)) FROM bug_links WHERE bug_links.crash_group_id = crash_groups.id) AS bug_links
         FROM crash_groups,
@@ -113,7 +120,9 @@
 sub get {
     my ($self, $uuid) = @_;
 
-    my $result = $self->db->query("
+    my $db = $self->db;
+
+    my $result = $db->query("
         SELECT * FROM crash_groups WHERE id = (SELECT crash_group_id FROM crash_reports WHERE uuid = ?)
         ",
         $uuid
@@ -125,7 +134,9 @@
 sub stats_by_product_and_version {
     my ($self, $group_uuid) = @_;
 
-    my $results = $self->db->query("
+    my $db = $self->db;
+
+    my $results = $db->query("
         SELECT
             name AS product_name,
             version,
@@ -152,9 +163,9 @@
 };
 
 sub _find_by_sig {
-    my ($self, $sig) = @_;
+    my ($self, $db, $sig) = @_;
 
-    return $self->db->query(
+    return $db->query(
         "SELECT id, crash_thread_signature_bt <-> \$1 AS dist
         FROM crash_groups
         WHERE crash_thread_signature_bt % \$1
@@ -165,9 +176,9 @@
 }
 
 sub _create_pg95 {
-    my ($self, $uuid, $sig, $title) = @_;
+    my ($self, $db, $uuid, $sig, $title) = @_;
 
-    my $crash_group = $self->db->query(
+    my $crash_group = $db->query(
         "INSERT INTO crash_groups (uuid, crash_thread_signature_bt, title)
         VALUES (?, ?, ?)
         ON CONFLICT DO NOTHING
@@ -176,7 +187,7 @@
     )->hash;
 
     if(!defined($crash_group)) {
-        $crash_group = $self->_find_by_sig($sig);
+        $crash_group = $self->_find_by_sig($db, $sig);
         warn "crash group is null after insert, reselected " . $crash_group->{id} . "\n";
     }
 
@@ -184,15 +195,15 @@
 }
 
 sub _create_pg94 {
-    my ($self, $uuid, $sig, $title) = @_;
+    my ($self, $db, $uuid, $sig, $title) = @_;
 
-    my $tx = $self->db->begin;
+    my $tx = $db->begin;
 
     # avoid race condition, no other safe way without upsert
-    $self->db->query("LOCK TABLE crash_groups IN EXCLUSIVE MODE")->finish;
+    $db->query("LOCK TABLE crash_groups IN EXCLUSIVE MODE")->finish;
 
     # this query is NOT concurrency safe, there is a possible write race. That's why we lock the table.
-    my $crash_group = $self->db->query(
+    my $crash_group = $db->query(
         "INSERT INTO crash_groups (uuid, crash_thread_signature_bt, title)
         SELECT \$1, \$2, \$3
         WHERE NOT EXISTS (SELECT 1 FROM crash_groups WHERE crash_thread_signature_bt % \$2)
@@ -202,7 +213,7 @@
 
     # may happen if there is an insert between the select (in _find_by_sig) and the lock
     if(!defined($crash_group)) {
-        $crash_group = $self->_find_by_sig($sig);
+        $crash_group = $self->_find_by_sig($db, $sig);
         warn "crash group is null after insert, reselected " . $crash_group->{id} . "\n";
     }
 
@@ -212,31 +223,34 @@
 }
 
 sub _set_similarity_limit {
-    my $self = shift;
+    my ($self, $db) = @_;
+
     my $max_dist = $self->app->config->{Processor}->{CrashSignatureExtractor}->{C_Cpp}->{GroupMaxDistance} || 0.1;
-    $self->db->query("SELECT set_limit(?)", 1.0 - $max_dist)->finish;
+    $db->query("SELECT set_limit(?)", 1.0 - $max_dist)->finish;
 }
 
 sub find_or_create {
-    my ($self, $uuid, $pjson) = @_;
+    my ($self, $uuid, $pjson, $db) = @_;
+
+    #$db //= $self->db;
 
     my $raw_thread = dclone $pjson->{threads}->[$pjson->{crashing_thread}->{threads_index}];
     my $thread = CrashTest::Model::Thread->new($raw_thread);
 
     my $sig = join "\n", @{$self->signature_extractor->extract_signature($thread)};
 
-    $self->_set_similarity_limit();
-    my $crash_group = $self->_find_by_sig($sig);
+    $self->_set_similarity_limit($db);
+    my $crash_group = $self->_find_by_sig($db, $sig);
 
     if(!$crash_group) {
         if($sig ne "") {
             my $title = $self->signature_extractor->extract_signature_title($thread);
 
-            if($self->db->dbh->{pg_server_version} >= 90500) {
+            if($db->dbh->{pg_server_version} >= 90500) {
                 #warn "PostgreSQL 9.5, nice !";
-                $crash_group = $self->_create_pg95($uuid, $sig, $title);
+                $crash_group = $self->_create_pg95($db, $uuid, $sig, $title);
             } else {
-                $crash_group = $self->_create_pg94($uuid, $sig, $title);
+                $crash_group = $self->_create_pg94($db, $uuid, $sig, $title);
             }
         }
     }
--- a/lib/CrashTest/Plugin/Storage/Sql/Model/CrashReport.pm	Sat Dec 31 20:34:01 2016 +0100
+++ b/lib/CrashTest/Plugin/Storage/Sql/Model/CrashReport.pm	Sun Apr 02 14:36:18 2017 +0200
@@ -26,18 +26,23 @@
 
 has [ qw/instance app config/ ];
 
-has [ qw/db sql_utils crash_groups/ ];
+has [ qw/dbh sql_utils crash_groups/ ];
 
 sub new {
     my $self = shift->SUPER::new(@_);
 
-    $self->db($self->instance->dbh->db);
+    $self->dbh($self->instance->dbh);
     $self->sql_utils(CrashTest::Plugin::Storage::Sql::Utils->new(@_));
     $self->crash_groups(CrashTest::Plugin::Storage::Sql::Model::CrashGroup->new(@_));
 
     return $self;
 }
 
+sub db {
+    my ($self) = @_;
+    return $self->dbh->db;
+}
+
 sub _build_query_from_search_string {
     my ($self, $search_str) = @_;
     return $self->sql_utils->build_query_from_search_string(
@@ -56,6 +61,8 @@
 sub index {
     my ($self, $pagen, $nperpage, $search_str) = @_;
 
+    my $db = $self->db;
+
     my $where = "";
     my @values = ();
     my $err;
@@ -68,7 +75,7 @@
         $err = $@;
     }
 
-    my $count = $self->db->query("
+    my $count = $db->query("
         SELECT count(crash_reports.id) AS total FROM crash_reports
         LEFT JOIN crash_users AS crash_user ON crash_reports.crash_user_id = crash_user.id
         LEFT JOIN products AS product ON crash_reports.product_id = product.id
@@ -87,7 +94,7 @@
     }
     my $extra_columns = join(",", @extra_cols);
 
-    my $results = $self->db->query("
+    my $results = $db->query("
         SELECT  crash_reports.*,
                 product.distributor AS p_distributor, product.name AS p_name, product.version AS p_version, product.release_channel AS p_release_channel,
                 crash_user.os AS u_os, crash_user.cpu_arch AS u_cpu_arch, crash_user.cpu_count AS u_cpu_count, crash_user.extra_info AS u_extra_info,
@@ -112,13 +119,15 @@
 sub get {
     my ($self, $uuid) = @_;
 
+    my $db = $self->db;
+
     my @extra_cols;
     foreach my $extra_col(@{$self->app->config->{WebInterface}->{ExtraColumns}->{Index}}) {
         push @extra_cols, $extra_col->{db_column} . " AS " . $extra_col->{id};
     }
     my $extra_columns = join(",", @extra_cols);
 
-    my $result = $self->db->query("
+    my $result = $db->query("
         SELECT  crash_reports.*,
                 product.distributor AS p_distributor, product.name AS p_name, product.version AS p_version, product.release_channel AS p_release_channel,
                 crash_user.os AS u_os, crash_user.cpu_arch AS u_cpu_arch, crash_user.cpu_count AS u_cpu_count, crash_user.extra_info AS u_extra_info,
@@ -139,7 +148,9 @@
 sub get_processed_data {
     my ($self, $uuid) = @_;
 
-    my $dbdata = $self->db->query("SELECT processed FROM crash_report_datas WHERE crash_report_id = (SELECT id FROM crash_reports WHERE uuid = ?)", $uuid)->hash;
+    my $db = $self->db;
+
+    my $dbdata = $db->query("SELECT processed FROM crash_report_datas WHERE crash_report_id = (SELECT id FROM crash_reports WHERE uuid = ?)", $uuid)->hash;
 
     my $processed_data = decode_json($dbdata->{processed});
 
@@ -149,12 +160,14 @@
 sub create {
     my ($self, $uuid, $pjson, $client_info, $dmp_file) = @_;
 
+    my $db = $self->db;
+
     if(!defined($client_info->{UserID})) {
         $self->app->log->info("Invalid crash $uuid: no UserID");
         return;
     }
 
-    my $tx = $self->db->begin;
+    my $tx = $db->begin;
 
     my ($start_time, $crash_time);
     if($client_info->{StartupTime}) {
@@ -176,10 +189,10 @@
         $client_info->{Version},
     );
 
-    my $dbproduct = $self->db->query("SELECT * FROM products WHERE distributor = ? AND name = ? AND version = ?", @product_values)->hash;
+    my $dbproduct = $db->query("SELECT * FROM products WHERE distributor = ? AND name = ? AND version = ?", @product_values)->hash;
     if(!$dbproduct) {
         push @product_values, $client_info->{ReleaseChannel};
-        $dbproduct = $self->db->query(
+        $dbproduct = $db->query(
             "INSERT INTO products (distributor, name, version, release_channel) VALUES (?, ?, ?, ?) RETURNING *",
             @product_values
         )->hash;
@@ -189,20 +202,20 @@
         $client_info->{UserID},
     );
 
-    my $dbuser = $self->db->query("SELECT * FROM crash_users WHERE user_id = ?", @user_values)->hash;
+    my $dbuser = $db->query("SELECT * FROM crash_users WHERE user_id = ?", @user_values)->hash;
     if(!$dbuser) {
         push @user_values, $pjson->{system_info}->{cpu_arch};
         push @user_values, $pjson->{system_info}->{cpu_count};
         push @user_values, $pjson->{system_info}->{os};
         push @user_values, encode_json($client_info);
 
-        $dbuser = $self->db->query(
+        $dbuser = $db->query(
             "INSERT INTO crash_users (user_id, cpu_arch, cpu_count, os, extra_info) VALUES (?, ?, ?, ?, ?) RETURNING *",
             @user_values
         )->hash;
     }
 
-    my $crash_group = $self->crash_groups->find_or_create($uuid, $pjson);
+    my $crash_group = $self->crash_groups->find_or_create($uuid, $pjson, $db);
 
     my $main_module;
     {
@@ -210,7 +223,7 @@
         $main_module = $pjson->{modules}->[$i]->{filename};
     }
 
-    my $dbcrash = $self->db->query(
+    my $dbcrash = $db->query(
         "INSERT INTO crash_reports (start_time, crash_time, uuid, main_module, product_id, crash_user_id, crash_group_id, crash_group_distance)
          VALUES (?, ?, ?, ?, ?, ?, ?, ?) RETURNING id",
         $start_time, $crash_time, $uuid, $main_module, $dbproduct->{id}, $dbuser->{id},
@@ -220,7 +233,7 @@
 
     # Create json for the params
     $pjson->{client_info} = $client_info;
-    $self->db->query(
+    $db->query(
         "INSERT INTO crash_report_datas (crash_report_id, processed) VALUES (?, ?) RETURNING id",
         $dbcrash->{id}, encode_json($pjson)
     );
@@ -233,19 +246,21 @@
 sub update {
     my ($self, $uuid, $pjson, $dmp_file) = @_;
 
-    my $tx = $self->db->begin;
+    my $db = $self->db;
 
-    my $dbcrash = $self->db->query("SELECT id FROM crash_reports WHERE uuid = ?", $uuid)->hash;
+    my $tx = $db->begin;
 
-    my $crash_group = $self->crash_groups->find_or_create($uuid, $pjson);
+    my $dbcrash = $db->query("SELECT id FROM crash_reports WHERE uuid = ?", $uuid)->hash;
 
-    $self->db->query("
+    my $crash_group = $self->crash_groups->find_or_create($uuid, $pjson, $db);
+
+    $db->query("
         UPDATE crash_report_datas SET processed = ?
         WHERE crash_report_id = ?",
         encode_json($pjson),
         $dbcrash->{id},
     );
-    $self->db->query("
+    $db->query("
         UPDATE crash_reports SET crash_group_id = ?, crash_group_distance = ?
         WHERE id = ?",
         $crash_group ? $crash_group->{id} : undef,
--- a/lib/CrashTest/Plugin/Storage/Sql/Utils.pm	Sat Dec 31 20:34:01 2016 +0100
+++ b/lib/CrashTest/Plugin/Storage/Sql/Utils.pm	Sun Apr 02 14:36:18 2017 +0200
@@ -19,13 +19,9 @@
 
 has [ qw/instance app config/ ];
 
-has qw/db/;
-
 sub new {
     my $self = shift->SUPER::new(@_);
 
-    $self->db($self->instance->dbh->db);
-
     return $self;
 }