Ignore:
Timestamp:
2018-11-08T17:22:04+13:00 (5 years ago)
Author:
ak19
Message:
  1. Some tidying up of the code. 2. Removing unnecessary calls to force_disconnect_from_db (now made 'private') after die() since on die() perl calls object destructor which will ensure disconnection anyway.
File:
1 edited

Legend:

Unmodified
Added
Removed

  • main/trunk/greenstone2/perllib/gssql.pm

    r32582 r32583  
    22#
    33# gssql.pm -- DBI for SQL related utility functions used by
    4 # GreenstoneSQLPlugout and hereafter by GreenstoneSQLPlugin too.
     4# GreenstoneSQLPlugout and GreenstoneSQLPlugin too.
    55# A component of the Greenstone digital library software
    66# from the New Zealand Digital Library Project at the
    … …  
    6161
    6262# Parameterise (one or more methods may use them):
    63 # - db_name (which is the GS3 sitename)
     63# - db_name (which is the GS3 sitename, or "greenstone2" for GS2)
    6464
    6565
    … …  
    8383    }
    8484    }
    85 
    8685   
    8786    die "Caught a $sig signal $!"; # die() will always call destructor (sub DESTROY)
    … …  
    125124# "It’s common to call die when handling SIGINT and SIGTERM. die is useful because it will ensure that Perl stops correctly: for example Perl will execute a destructor method if present when die is called, but the destructor method will not be called if a SIGINT or SIGTERM is received and no signal handler calls die."
    126125#
    127 # https://perldoc.perl.org/perlobj.html#Destructors
     126# Useful: https://perldoc.perl.org/perlobj.html#Destructors
    128127#
    129128# https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#disconnect
    … …  
    145144    if ($_dbh_instance) { # database handle still active. Use singleton handle!
    146145
    147         # THIS CODE HAS MOVED TO finish_signal_handler() WHERE IT BELONGS
    148         # If autocommit wasn't set, then this is a cancel operation.
    149         # If we've not disconnected from the sql db yet and if we've not committed
    150         # transactions yet, then cancel means we do a rollback here
    151 
    152         # if($_dbh_instance->{AutoCommit} == 0) {
    153        
    154         #   $_dbh_instance->rollback(); # will warn on failure, nothing more we can/want to do,
    155             #     # don't do a die() here: possibility of infinite loop and we still want to disconnect
    156         # }
    157 
    158         # Either way, we're now finally ready to disconnect as is required for premature
    159         # termination too
     146        # rollback code has moved to finish_signal_handler() where it belongs?
     147       
     148        # NOTE: if RaiseError is set on dbi connection, then on any error, perl process will die()
     149        # which will end up calling this DESTROY. If it was a die() that called DESTROY
     150        # then need to rollback the db here. However, if it was not a die() but natural termination
     151        # of the perl process, destroy() will also get called. In that case we don't want to rollback
     152        # but do a commit() to the DB instead.
     153        # Perhaps detecting the difference may be accomplished by checking ref_count:
     154        # - If ref_count not 0 it may require a rollback?
     155        # - If ref_count 0 it may be a natural termination and require a commit? Except that ref_count
     156        # is set back to 0 in finished(), which will do the commit when ref_count becomes 0. So shouldn't
     157        # (have to) do that here.
     158       
     159        # We're now finally ready to disconnect, as is required for both natural and premature termination
    160160        print STDERR "XXXXXXXX Global Destruct: Disconnecting from database\n";
    161161        $_dbh_instance->disconnect or warn $_dbh_instance->errstr;
    … …  
    165165    return;
    166166    }
     167
    167168}
    168169
    … …  
    229230    }
    230231    if($params_map->{'autocommit'}) {
    231         print STDERR "   SQL DB UNDO SUPPORT OFF.\n";
     232        print STDERR "   SQL DB CANCEL SUPPORT OFF.\n";
    232233    } else {
    233         print STDERR "   SQL DB UNDO SUPPORT ON.\n";
     234        print STDERR "   SQL DB CANCEL SUPPORT ON.\n";
    234235    }
    235236    }
    … …  
    326327
    327328# Will disconnect if this instance of gssql holds the last reference to the db connection
     329# If disconnecting and autocommit is off, then this will commit before disconnecting
    328330sub finished {
    329     my $self= shift (@_);
    330 
    331     # TODO: if AutoCommit was off, meaning transactions were on/enabled,
    332     # then here is where we commit our one long transaction.
    333     # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
    334     my $rc = 1;   
    335    
    336     $ref_count--;
    337     if($ref_count == 0) {
    338     # Only commit transaction when we're about to disconnect, not before
    339     # If autocommit was on, then we'd have committed after every db operation, so nothing to do
    340     $rc = $self->do_commit_if_on();
    341    
    342     $self->force_disconnect_from_db();
    343     }
    344 
    345     return $rc;
    346 }
    347 
    348 sub do_commit_if_on {
    349331    my $self= shift (@_);
    350332    my $dbh = $self->{'db_handle'};
    … …  
    352334    my $rc = 1; # return code: everything went fine, regardless of whether we needed to commit
    353335                # (AutoCommit on or off)
    354 
    355     # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
    356     if($dbh->{AutoCommit} == 0) {
    357     print STDERR "   Committing transaction to SQL database now.\n" if $self->{'verbosity'};
    358     $rc = $dbh->commit() or warn("SQL DB COMMIT FAILED: " . $dbh->errstr); # important problem
    359                                                             # worth embellishing error message
    360     }
    361     # If autocommit was on, then we'd have committed after every db operation, so nothing to do
     336   
     337    $ref_count--;
     338    if($ref_count == 0) { # Only commit transaction when we're about to actually disconnect, not before
     339   
     340    # TODO: If AutoCommit was off, meaning transactions were on/enabled,
     341    # then here is where we commit our one long transaction.
     342    # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
     343    if($dbh->{AutoCommit} == 0) {
     344        print STDERR "   Committing transaction to SQL database now.\n" if $self->{'verbosity'};
     345        $rc = $dbh->commit() or warn("SQL DB COMMIT FAILED: " . $dbh->errstr); # important problem
     346        # worth embellishing error message
     347    }
     348    # else if autocommit was on, then we'd have committed after every db operation, so nothing to do
     349   
     350    $self->_force_disconnect_from_db();
     351    }
    362352
    363353    return $rc;
    364354}
     355
    365356
    366357# Call this method on die(), so that you're sure the perl process has disconnected from SQL db
    … …  
    368359# TODO: make sure to have committed or rolled back before disconnect
    369360# and that you've call finish() on statement handles if any fetch remnants remain
    370 sub force_disconnect_from_db {
     361sub _force_disconnect_from_db {
    371362    my $self= shift (@_);
    372363
Note: See TracChangeset for help on using the changeset viewer.