Skip to content

Commit

Permalink
Properly avoid advisory Yugabyte lock errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
theory committed Dec 30, 2024
1 parent 91f1999 commit 86bf940
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 11 deletions.
7 changes: 7 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 11 additions & 9 deletions lib/App/Sqitch/Engine/pg.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
},
},
Expand Down Expand Up @@ -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 {
Expand All @@ -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]
}
Expand Down
4 changes: 2 additions & 2 deletions lib/sqitchtutorial-oracle.pod
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,10 @@ using the Docker or VM environments described above, you might need to
L<create the database|https://docs.oracle.com/cd/B28359_01/server.111/b28310/create001.htm#ADMIN11068>
and configure the SID. Assuming you have an Oracle SID named C<FLIPR_TEST> set
up in your C<F<TNSNAMES.ORA>|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<database URI|https://github.com/libwww-perl/uri-db/>, 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:

Expand Down

0 comments on commit 86bf940

Please sign in to comment.