Changeset 32582 for main/trunk/greenstone2/perllib/gssql.pm

Timestamp:

2018-11-07T20:44:34+13:00 (5 years ago)

Author:

ak19

Message:

Now that previous commit(s) put sig handlers in place in gs_sql, have been able to add in Undo on build/import Cancel for the GS SQL Plugs. This utilizes AutoCommit vs Transaction (rollback/commit) behaviour. On cancel, a sig handler is triggered (SIGINT) and, if AutoCommit is off, does a rollback before die() which calls object destructor and disconnects from db. On regular program execution running to normal termination, the last finish() call on gs_sql that will trigger the disconnect, will now first do a commit(), if AutoCommit is off, before disconnecting. For now, the default for both GreenstoneSQLPlugs is to support Undo (i.e. transactions), which turns AutoCommit off. Not sure whether this will be robust: what if transactions take place in memory, we could be dealing with millions of docs of large full-txt. Another issue is that the SQL DB may be out of sync with archives and index folder on Cancel: archives and index just terminate and are in an intermediate state depending on when cancel was pressed. Whereas the GS SQL DB is in a rolled back state as if the import or build never took place. A third issue is that during buildcol (perhaps specifically during buildcol's doc processing phase), pressing Cancel still continues buildcol: the current perl process is cancelled but the next one continues, rather than terminating buildcol in entirety. What happens with the GS SQL DB is that any 'transaction' until then is rolled back, perhaps a transaction regarding one doc if the Cancel affects on a doc basis, and the next process (next doc processing?) continues and allows for further transactions that are all committed at the end on natural termination of buildcol. Need to whether Undo behavious is really what we want. But it's available now and we can simply change the default to not support Undo if we want the old behaviour again.

File:

• main/trunk/greenstone2/perllib/gssql.pm (modified) (8 diffs)

main/trunk/greenstone2/perllib/gssql.pm

@@ -33 +33 @@
 use DBI; # the central package for this module used by GreenstoneSQL Plugout and Plugin
 
-#$SIG{INT}  = sub { die "Caught a sigint $!" };
-#$SIG{TERM}  = sub { die "Caught a sigterm $!" };
-#$SIG{KILL}  = sub { die "Caught a sigkill $!" };
-
-
-$SIG{INT}  = \&finish_signal_handler;
-$SIG{TERM}  = \&finish_signal_handler;
-$SIG{KILL}  = \&finish_signal_handler;
-
-sub finish_signal_handler {
-    my ($sig) = @_; # one of INT|KILL|TERM
-    die "Caught a $sig signal $!"; # will call destructor
-}
+
 ##############################
 
@@ -75 +63 @@
 # - db_name (which is the GS3 sitename)
 
+
+
+$SIG{INT}  = \&finish_signal_handler;
+$SIG{TERM}  = \&finish_signal_handler;
+$SIG{KILL}  = \&finish_signal_handler;
+
+sub finish_signal_handler {
+    my ($sig) = @_; # one of INT|KILL|TERM
+
+    if ($_dbh_instance) { # database handle (note, using singleton) still active.
+    
+    # TODO: If autocommit wasn't set, then this is a cancel operation.
+    # If we've not disconnected from the sql db yet and if we've not committed
+    # transactions yet, then cancel means we do a rollback here
+    
+    if($_dbh_instance->{AutoCommit} == 0) {
+        print STDERR "   User cancelled: rolling back SQL database transaction.\n";
+        $_dbh_instance->rollback(); # will warn on failure, nothing more we can/want to do,
+    }
+    }
+
+    
+    die "Caught a $sig signal $!"; # die() will always call destructor (sub DESTROY)
+}
+
 sub new
 {  
@@ -111 +124 @@
 # We want to ensure we've closed the db connection in such cases.
 # "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."
+#
 # https://perldoc.perl.org/perlobj.html#Destructors
+#
+# https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#disconnect
+# "Disconnects the database from the database handle. disconnect is typically only used before exitin# g the program. The handle is of little use after disconnecting.
+#
+# The transaction behaviour of the disconnect method is, sadly, undefined. Some database systems (such as Oracle and Ingres) will automatically commit any outstanding changes, but others (such as Informix) will rollback any outstanding changes. Applications not using AutoCommit should explicitly call commit or rollback before calling disconnect.
+#
+# The database is automatically disconnected by the DESTROY method if still connected when there are no longer any references to the handle. The DESTROY method for each driver should implicitly call rollback to undo any uncommitted changes. This is vital behaviour to ensure that incomplete transactions don't get committed simply because Perl calls DESTROY on every object before exiting. Also, do not rely on the order of object destruction during "global destruction", as it is undefined.
+#
+# Generally, if you want your changes to be committed or rolled back when you disconnect, then you should explicitly call "commit" or "rollback" before disconnecting.
+#
+# If you disconnect from a database while you still have active statement handles (e.g., SELECT statement handles that may have more data to fetch), you will get a warning. The warning may indicate that a fetch loop terminated early, perhaps due to an uncaught error. To avoid the warning call the finish method on the active handles."
+#
 sub DESTROY {
     my $self = shift;
 
     if (${^GLOBAL_PHASE} eq 'DESTRUCT') {
-    if ($_dbh_instance) {
+    
+    if ($_dbh_instance) { # database handle still active. Use singleton handle!
+
+        # THIS CODE HAS MOVED TO finish_signal_handler() WHERE IT BELONGS
+        # If autocommit wasn't set, then this is a cancel operation.
+        # If we've not disconnected from the sql db yet and if we've not committed
+        # transactions yet, then cancel means we do a rollback here
+
+        # if($_dbh_instance->{AutoCommit} == 0) {
+        
+        #   $_dbh_instance->rollback(); # will warn on failure, nothing more we can/want to do,
+            #     # don't do a die() here: possibility of infinite loop and we still want to disconnect
+        # }
+
+        # Either way, we're now finally ready to disconnect as is required for premature
+        # termination too
         print STDERR "XXXXXXXX Global Destruct: Disconnecting from database\n";
         $_dbh_instance->disconnect or warn $_dbh_instance->errstr;
@@ -182 +223 @@
     #my $self= shift (@_); # singleton method doesn't use self, but callers don't need to know that
     my ($params_map) = @_;
-
+    
+    if($params_map->{'verbosity'}) {
+    if(!defined $params_map->{'autocommit'}) {
+        print STDERR "  Autocommit parameter not defined\n";
+    }
+    if($params_map->{'autocommit'}) {
+        print STDERR "   SQL DB UNDO SUPPORT OFF.\n";
+    } else {
+        print STDERR "   SQL DB UNDO SUPPORT ON.\n";
+    }
+    }
+    
     return $_dbh_instance if($_dbh_instance);
 
@@ -217 +269 @@
     print STDERR "\nAssuming the mysql server has been started with: --character_set_server=utf8mb4\n" if $db_driver eq "mysql";
     }
+
+    # DBI AutoCommit connection param is on/1 by default, so if a value for this is not defined
+    # as a method parameter to _get_connection_instance, then fallback to the default of on/1
+    my $autocommit = (defined $params_map->{'autocommit'}) ? $params_map->{'autocommit'} : 1;
     
     my $dbh = DBI->connect("$connect_str", $db_user, $db_pwd,
@@ -223 +279 @@
                    PrintError => 1, # on by default, but being explicit
                    RaiseError => 0, # off by default, but being explicit
-                   AutoCommit => 1, # on by default, but being explicit
+                   AutoCommit => $autocommit,
                    mysql_enable_utf8mb4 => 1 # tells MySQL to use UTF-8 for communication and tells DBD::mysql to decode the data, see https://stackoverflow.com/questions/46727362/perl-mysql-utf8mb4-issue-possible-bug 
                });
@@ -273 +329 @@
     my $self= shift (@_);
 
+    # TODO: if AutoCommit was off, meaning transactions were on/enabled,
+    # then here is where we commit our one long transaction.
+    # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
+    my $rc = 1;    
+    
     $ref_count--;
     if($ref_count == 0) {
+    # Only commit transaction when we're about to disconnect, not before
+    # If autocommit was on, then we'd have committed after every db operation, so nothing to do
+    $rc = $self->do_commit_if_on();
+    
     $self->force_disconnect_from_db();
-    }    
+    }
+
+    return $rc;
+}
+
+sub do_commit_if_on {
+    my $self= shift (@_);
+    my $dbh = $self->{'db_handle'};
+    
+    my $rc = 1; # return code: everything went fine, regardless of whether we needed to commit
+                # (AutoCommit on or off)
+
+    # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
+    if($dbh->{AutoCommit} == 0) {
+    print STDERR "   Committing transaction to SQL database now.\n" if $self->{'verbosity'};
+    $rc = $dbh->commit() or warn("SQL DB COMMIT FAILED: " . $dbh->errstr); # important problem
+                                                            # worth embellishing error message
+    }
+    # If autocommit was on, then we'd have committed after every db operation, so nothing to do
+
+    return $rc;
 }
 
@@ -429 +514 @@
     $dbh->do("drop table $table");# || warn("@@@ Couldn't delete $table");
     }
+
+    # TODO Q: commit here, so that future select statements work?
+    # See https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#Transactions
 }