From 86bf9408712019e2784afbb6f758ec508515c571 Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" Date: Mon, 30 Dec 2024 18:43:30 -0500 Subject: [PATCH] Properly avoid advisory Yugabyte lock errors The `try_lock` method was not properly checking for Yugabyte before attempting to acquire an advisory lock. Likely it used to be a warning, but now is an error that cannot be avoided. So have `try_lock` check for Yugabyte before attempting to take an advisory lock. This requires fetching the database handle, first, as the database is not identified until it connects. So abstract that need by replacing the `_provider` attribute with a method that checks a DBI private module attribute. Fixes #841. While at it, replace a wayward mention of the long-unsupported (or maybe incorrectly documented?) `SQITCH_URI` environment variable in the Oracle tutorial with the correct name, `SQITCH_TARGET`. Resolves #845. --- Changes | 7 +++++++ lib/App/Sqitch/Engine/pg.pm | 20 +++++++++++--------- lib/sqitchtutorial-oracle.pod | 4 ++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Changes b/Changes index 94449a84..a0b21874 100644 --- a/Changes +++ b/Changes @@ -10,6 +10,13 @@ Revision history for Perl extension App::Sqitch exception objects rather than strings, for more consistent error handling throughout. - Removed duplicate DBI error handling code from engines and tests. + - Fixed an order of operation issue that prevented Sqitch from detecting + Yugabyte before attempting to create an advisory lock, which results in + an error for more recent Yugabyte releases. Thanks to Stefano Ricciardi + for the report (#841). + - Removed a wayward mention of the long-deprecated `SQITCH_URI` + environment variable from the Oracle tutorial. Thanks to Austin Hanson + for the report (#845). 1.4.1 2024-02-04T16:35:32Z - Removed the quoting of the role and warehouse identifiers that was diff --git a/lib/App/Sqitch/Engine/pg.pm b/lib/App/Sqitch/Engine/pg.pm index 312ac1bc..480b4448 100644 --- a/lib/App/Sqitch/Engine/pg.pm +++ b/lib/App/Sqitch/Engine/pg.pm @@ -104,13 +104,6 @@ sub name { 'PostgreSQL' } sub driver { 'DBD::Pg 2.0' } sub default_client { 'psql' } -has _provider => ( - is => 'rw', - isa => enum([qw( postgres yugabyte )]), - default => 'postgres', - lazy => 1, -); - has dbh => ( is => 'rw', isa => DBH, @@ -144,7 +137,9 @@ has dbh => ( my $v = $dbh->selectcol_arrayref( q{SELECT split_part(version(), ' ', 2)} )->[0] // ''; - $self->_provider('yugabyte') if $v =~ /-YB-/; + $dbh->{private_sqitch_info} = { + provider => $v =~ /-YB-/ ? 'yugabyte' : 'postgres', + }; return; }, }, @@ -270,6 +265,11 @@ sub _run_registry_file { $self->dbh->do('SET search_path = ?', undef, $schema); } +# Returns the name of the provider. +sub _provider { + shift->dbh->{private_sqitch_info}{provider} +} + # Override to lock the changes table. This ensures that only one instance of # Sqitch runs at one time. sub begin_work { @@ -287,7 +287,9 @@ sub begin_work { # Override to try to acquire a lock on a constant number without waiting. sub try_lock { - shift->dbh->selectcol_arrayref( + my $self = shift; + return 1 if $self->_provider ne 'postgres'; + $self->dbh->selectcol_arrayref( 'SELECT pg_try_advisory_lock(75474063)' )->[0] } diff --git a/lib/sqitchtutorial-oracle.pod b/lib/sqitchtutorial-oracle.pod index 7b73d2bf..22efb866 100644 --- a/lib/sqitchtutorial-oracle.pod +++ b/lib/sqitchtutorial-oracle.pod @@ -306,10 +306,10 @@ using the Docker or VM environments described above, you might need to L and configure the SID. Assuming you have an Oracle SID named C set up in your C|https://www.orafaq.com/wiki/Tnsnames.ora> file, -tell Sqitch where to send the change via a +tell Sqitch where to send the change to a target specified as a L, such as - export SQITCH_URI=db:oracle://$username:$password@/flipr_test + export SQITCH_TARGET=db:oracle://$username:$password@/flipr_test With that URI set up, we can deploy: