[GUSDEV] GUS plugins: getQueryHandle & commit

[Steve F. <sfi...@pc...>]

Folks-

We are making a change to ga.  Upon plugin completion, ga will rollback 
any outstanding transactions on the query handle (acquired from 
getQueryHandle()).  Previously they were being committed.  Plugins must 
explicitly handle commits to this query handle.  Also, previously, the 
documentation for getQueryHandle() stated that it used autocommit by 
default.  That was not correct.  If you want autocommit, call it like 
this: getQueryHandle(1).


As the documentation states, this query handle does not obey the 
--nocommit option.  It should never be used to write to data tables.  
Temp tables are ok. 


The following plugins seem to be mis-using the query handle, writing 
actual data with it, and should be reviewed:
  PlasmoDBData/Load/plugin/perl/UpdateRodentPlasmodiumChromosomes.pm
  GUS/Community/plugin/perl/InsertFromCsv.pm
  DoTS/Gene/plugin/perl/MoveGenomeDotsGeneToAllgenes.pm
  GUS/Community/plugin/perl/LoadBLATAlignments
  GUS/Community/plugin/perl/UpdateNASequences


I have updated the following plugins, so that they now explicitly call 
commit() upon completion.
      DoTS/Gene/plugin/perl/MoveGenomeDotsGeneToAllgenes.pm
      DoTS/Gene/plugin/perl/EstClonePairs.pm
      GUS/Community/plugin/perl/LoadBLATAlignments.pm
      GUS/Community/plugin/perl/InsertFromCsv.pm
      GUS/PluginMgr/lib/perl/GusApplication.pm
      GUS/Supported/plugin/perl/LoadNRDB.pm
      GUS/Supported/plugin/perl/InsertGeneOntology.pm
      GUS/Supported/plugin/perl/LoadArrayResults.pm
      GUS/ObjRelP/lib/perl/DbiDatabase.pm
      PlasmoDBData/Load/plugin/perl/UpdateRodentPlasmodiumChromosomes.pm

Steve
steve

Brian Brunk wrote:
> OK
>
> On Feb 11, 2009, at 11:58 AM, Steve Fischer wrote:
>
>> brian-
>>
>> normal plugin completion does call logout.
>>
>> but... if the plugin doesn't return a message describing the results, 
>> ga dies (and doesn't close the alg invocation, or disconnect).
>>
>> i am thinking that we should only die like that if we are in nocommit 
>> mode.  that will warn the author to fix the bug.  but, in a real run, 
>> i think we should always close the invocation, with or without a 
>> message.
>>
>> furthermore, since run is an an eval{} block, we can put the rollback 
>> after it, if there was an error condition, so we don't have to worry 
>> about destroy.
>>
>> ?
>>
>> steve
>>
>> Brian Brunk wrote:
>>> I prefer (b) as (a) seems sloppy and prone to hidden problems.  
>>> Essentially we are relying on a side effect of the behavior of DBI 
>>> and Oracle to do the work for us which I don't like.
>>>
>>> The DBI connect string must allow one to specify the behavior of a 
>>> handle on disconnect (or failure) otherwise it seems like we would 
>>> be having problems with transactions in the dbhandle on killing a 
>>> process.  Would be better to get the right kind of handle that takes 
>>> care of the rollback automatically rather than explicitly doing a 
>>> rollback I think.
>>>
>>> -Brian
>>>
>>> On Feb 11, 2009, at 8:41 AM, Steve Fischer wrote:
>>>
>>>> ok, so we have two choices:  (a) stay with current behavior but 
>>>> correct for dbms's that do rollback on destroy.  to do this, i 
>>>> would add a commit() after the run method in ga
>>>> (b) *change* the current behavior to *require* explicit commits in 
>>>> the plugins.  to do this, i would add a rollback() after the run 
>>>> method.  then we'd need to review all the plugins and add an 
>>>> explicit commit().
>>>>
>>>> in either case i would fix the documentation to reflect the reality.
>>>>
>>>> i think i favor (a) because it is less work, and less risk.  i 
>>>> think that plugins that actually need to do careful transactional 
>>>> logic are doing it already, and for the rest, a commit on 
>>>> termination is probably fine.  unless we think we are having 
>>>> invisible data errors as a result of this subtle problem, i think 
>>>> we should leave well enough alone.
>>>>
>>>> ?
>>>>
>>>> steve
>>>>
>>>> Brian Brunk wrote:
>>>>> Sounds good .. would strongly prefer that we add an explicit 
>>>>> commit to all plugins updating the db with this handle in addition 
>>>>> to just changing the documentation.
>>>>>
>>>>> -Brian
>>>>>
>>>>> On Feb 11, 2009, at 8:14 AM, Steve Fischer wrote:
>>>>>
>>>>>> brian-
>>>>>>
>>>>>> i have looked a bit into this.
>>>>>>
>>>>>> there are 16 plugins that contain "getQueryHandle" and also 
>>>>>> contain "insert into" and 6 that contain "delete from" (it is 
>>>>>> harder to comb through to find UPDATE because it is a common word).
>>>>>>
>>>>>> some subset of these explicitly call commit(), and some subset 
>>>>>> are using the words "insert into" in a comment.
>>>>>>
>>>>>> the DBI documentation says that transaction behavior on 
>>>>>> disconnect is undefined.  in oracle it commits, on informix it 
>>>>>> rolls back.
>>>>>>
>>>>>> so, on our rig we are probably ok.  don't know about gus users 
>>>>>> running postgres.
>>>>>>
>>>>>> i suppose the right thing to do would be to review these plugins 
>>>>>> and add an explicit commit().
>>>>>>
>>>>>> i will definitely change the documentation to stress that an 
>>>>>> explicit commit is required.
>>>>>>
>>>>>> what do you think?
>>>>>>
>>>>>> steve
>>>>>
>>>>> Brian P. Brunk, Ph.D.
>>>>> EuPathDB Manager
>>>>> 1403 Blockley Hall
>>>>> Penn Center For Bioinformatics
>>>>> University of Pennsylvania
>>>>> Philadelphia PA 19104-6021
>>>>> Tel: 215-573-3118
>>>>> Fax: 215-573-3111
>>>>>
>>>>>
>>>
>>> Brian P. Brunk, Ph.D.
>>> EuPathDB Manager
>>> 1403 Blockley Hall
>>> Penn Center For Bioinformatics
>>> University of Pennsylvania
>>> Philadelphia PA 19104-6021
>>> Tel: 215-573-3118
>>> Fax: 215-573-3111
>>>
>>>
>
> Brian P. Brunk, Ph.D.
> EuPathDB Manager
> 1403 Blockley Hall
> Penn Center For Bioinformatics
> University of Pennsylvania
> Philadelphia PA 19104-6021
> Tel: 215-573-3118
> Fax: 215-573-3111
>
>