From f2593e2addced3659704b12b1c6539ec8486534b Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Tue, 26 Apr 2022 15:46:27 +0200 Subject: [PATCH 1/4] Compute user existence by count instead of by id --- lib/Zonemaster/Backend/DB.pm | 6 +++--- lib/Zonemaster/Backend/DB/MySQL.pm | 2 +- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 2 +- lib/Zonemaster/Backend/DB/SQLite.pm | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index c88cce8a7..a83a63996 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -582,14 +582,14 @@ sub user_authorized { my ( $self, $user, $api_key ) = @_; my $dbh = $self->dbh; - my ( $id ) = $dbh->selectrow_array( - "SELECT id FROM users WHERE username = ? AND api_key = ?", + my ( $count ) = $dbh->selectrow_array( + "SELECT count(*) FROM users WHERE username = ? AND api_key = ?", undef, $user, $api_key ); - return $id; + return $count; } sub batch_exists_in_db { diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index 86ec1d7df..412e47829 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -249,7 +249,7 @@ sub add_batch_job { my $dbh = $self->dbh; - if ( $self->user_authorized( $params->{username}, $params->{api_key} ) ) { + if ( 1 == $self->user_authorized( $params->{username}, $params->{api_key} ) ) { $batch_id = $self->create_new_batch_job( $params->{username} ); my $test_params = $params->{test_params}; diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 617d12abc..75c765b5d 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -222,7 +222,7 @@ sub add_batch_job { my $dbh = $self->dbh; - if ( $self->user_authorized( $params->{username}, $params->{api_key} ) ) { + if ( 1 == $self->user_authorized( $params->{username}, $params->{api_key} ) ) { $batch_id = $self->create_new_batch_job( $params->{username} ); my $test_params = $params->{test_params}; diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index e04108f89..0c61efc4d 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -210,7 +210,7 @@ sub add_batch_job { my $dbh = $self->dbh; - if ( $self->user_authorized( $params->{username}, $params->{api_key} ) ) { + if ( 1 == $self->user_authorized( $params->{username}, $params->{api_key} ) ) { $batch_id = $self->create_new_batch_job( $params->{username} ); my $test_params = $params->{test_params}; From 45fd9a2c6de483356566ff433ddfc140c7b5eeac Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Thu, 16 Mar 2023 15:59:44 +0100 Subject: [PATCH 2/4] Remove unneeded field --- lib/Zonemaster/Backend/DB.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index a83a63996..3013cf84e 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -514,7 +514,6 @@ sub get_test_history { (SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_critical, (SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_error, (SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_warning, - id, hash_id, created_at, undelegated From f972231ad474f9da2c1a196da5f460cce9e3fc97 Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Tue, 3 May 2022 15:22:59 +0200 Subject: [PATCH 3/4] Use JOIN clause --- lib/Zonemaster/Backend/DB.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index 3013cf84e..97caa9ba6 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -511,9 +511,9 @@ sub get_test_history { my @results; my $query = q[ SELECT - (SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_critical, - (SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_error, - (SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_warning, + (SELECT count(*) FROM result_entries JOIN test_results ON result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_critical, + (SELECT count(*) FROM result_entries JOIN test_results ON result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_error, + (SELECT count(*) FROM result_entries JOIN test_results ON result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_warning, hash_id, created_at, undelegated From 0ec07decca854ec6d0ee27049ba653213029fd9b Mon Sep 17 00:00:00 2001 From: Alexandre Pion Date: Tue, 3 May 2022 15:52:03 +0200 Subject: [PATCH 4/4] Order using common field Rely on the "created_at" field to perform the search and ordering in database. This requires the indexes to be updated (which was already the case when runnin batches). --- lib/Zonemaster/Backend/DB.pm | 6 +++--- lib/Zonemaster/Backend/DB/MySQL.pm | 8 ++++---- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 6 +++--- lib/Zonemaster/Backend/DB/SQLite.pm | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index 97caa9ba6..409fcb6e9 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -519,7 +519,7 @@ sub get_test_history { undelegated FROM test_results WHERE progress = 100 AND domain = ? AND ( ? IS NULL OR undelegated = ? ) - ORDER BY id DESC + ORDER BY created_at DESC LIMIT ? OFFSET ?]; @@ -642,7 +642,7 @@ sub get_test_request { WHERE progress = 0 AND queue = ? ORDER BY priority DESC, - id ASC + created_at ASC LIMIT 1 ], undef, @@ -657,7 +657,7 @@ sub get_test_request { FROM test_results WHERE progress = 0 ORDER BY priority DESC, - id ASC + created_at ASC LIMIT 1 ], ); diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index 412e47829..eb06a0109 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -89,9 +89,9 @@ sub create_schema { # retrieve all indexes by key name my $indexes = $dbh->selectall_hashref( 'SHOW INDEXES FROM test_results', 'Key_name' ); - if ( not exists($indexes->{test_results__hash_id}) ) { + if ( not exists($indexes->{test_results__hash_id_created_at}) ) { $dbh->do( - 'CREATE INDEX test_results__hash_id ON test_results (hash_id)' + 'CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)' ); } if ( not exists($indexes->{test_results__fingerprint}) ) { @@ -257,7 +257,7 @@ sub add_batch_job { my $queue_label = $test_params->{queue}; $dbh->{AutoCommit} = 0; - eval {$dbh->do( "DROP INDEX test_results__hash_id ON test_results" );}; + eval {$dbh->do( "DROP INDEX test_results__hash_id_created_at ON test_results" );}; eval {$dbh->do( "DROP INDEX test_results__fingerprint ON test_results" );}; eval {$dbh->do( "DROP INDEX test_results__batch_id_progress ON test_results" );}; eval {$dbh->do( "DROP INDEX test_results__progress ON test_results" );}; @@ -298,7 +298,7 @@ sub add_batch_job { $undelegated, ); } - $dbh->do( "CREATE INDEX test_results__hash_id ON test_results (hash_id, created_at)" ); + $dbh->do( "CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)" ); $dbh->do( "CREATE INDEX test_results__fingerprint ON test_results (fingerprint)" ); $dbh->do( "CREATE INDEX test_results__batch_id_progress ON test_results (batch_id, progress)" ); $dbh->do( "CREATE INDEX test_results__progress ON test_results (progress)" ); diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 75c765b5d..02c57f689 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -81,7 +81,7 @@ sub create_schema { ) or die Zonemaster::Backend::Error::Internal->new( reason => "PostgreSQL error, could not create 'test_results' table", data => $dbh->errstr() ); $dbh->do( - 'CREATE INDEX IF NOT EXISTS test_results__hash_id ON test_results (hash_id)' + 'CREATE INDEX IF NOT EXISTS test_results__hash_id_created_at ON test_results (hash_id, created_at)' ); $dbh->do( 'CREATE INDEX IF NOT EXISTS test_results__fingerprint ON test_results (fingerprint)' @@ -234,7 +234,7 @@ sub add_batch_job { $dbh->begin_work(); $dbh->do( "ALTER TABLE test_results DROP CONSTRAINT IF EXISTS test_results_pkey" ); - $dbh->do( "DROP INDEX IF EXISTS test_results__hash_id" ); + $dbh->do( "DROP INDEX IF EXISTS test_results__hash_id_created_at" ); $dbh->do( "DROP INDEX IF EXISTS test_results__fingerprint" ); $dbh->do( "DROP INDEX IF EXISTS test_results__batch_id_progress" ); $dbh->do( "DROP INDEX IF EXISTS test_results__progress" ); @@ -271,7 +271,7 @@ sub add_batch_job { } $dbh->pg_putcopyend(); $dbh->do( "ALTER TABLE test_results ADD PRIMARY KEY (id)" ); - $dbh->do( "CREATE INDEX test_results__hash_id ON test_results (hash_id, created_at)" ); + $dbh->do( "CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)" ); $dbh->do( "CREATE INDEX test_results__fingerprint ON test_results (fingerprint)" ); $dbh->do( "CREATE INDEX test_results__batch_id_progress ON test_results (batch_id, progress)" ); $dbh->do( "CREATE INDEX test_results__progress ON test_results (progress)" ); diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 0c61efc4d..5d0f5c9ce 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -84,7 +84,7 @@ sub create_schema { ) or die Zonemaster::Backend::Error::Internal->new( reason => "SQLite error, could not create 'test_results' table", data => $dbh->errstr() ); $dbh->do( - 'CREATE INDEX IF NOT EXISTS test_results__hash_id ON test_results (hash_id)' + 'CREATE INDEX IF NOT EXISTS test_results__hash_id_created_at ON test_results (hash_id, created_at)' ); $self->dbh->do( 'CREATE INDEX IF NOT EXISTS test_results__fingerprint ON test_results (fingerprint)' @@ -218,7 +218,7 @@ sub add_batch_job { my $queue_label = $test_params->{queue}; $dbh->{AutoCommit} = 0; - eval {$dbh->do( "DROP INDEX IF EXISTS test_results__hash_id " );}; + eval {$dbh->do( "DROP INDEX IF EXISTS test_results__hash_id_created_at " );}; eval {$dbh->do( "DROP INDEX IF EXISTS test_results__fingerprint " );}; eval {$dbh->do( "DROP INDEX IF EXISTS test_results__batch_id_progress " );}; eval {$dbh->do( "DROP INDEX IF EXISTS test_results__progress " );}; @@ -257,7 +257,7 @@ sub add_batch_job { $undelegated, ); } - $dbh->do( "CREATE INDEX test_results__hash_id ON test_results (hash_id, created_at)" ); + $dbh->do( "CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)" ); $dbh->do( "CREATE INDEX test_results__fingerprint ON test_results (fingerprint)" ); $dbh->do( "CREATE INDEX test_results__batch_id_progress ON test_results (batch_id, progress)" ); $dbh->do( "CREATE INDEX test_results__progress ON test_results (progress)" );