[asterisk-dev] func_odbc auto commit at answer time

[Jaco Kroon]

Hi All,

So I've been updating some code to use func_odbc vs previous solutions
and have bumped into a few concerns (1 I'd appreciate feedback on the
review, 2 is prelude to 3, which is the main issue currently):

1.  ARGn from Gosub() getting leaked into func_odbc if adequate ARG
parameters are not given:

[FOO]
...
writesql = INSERT INTO foo(${ARG1}) VALUES('${SQL_ESC(${VALUE})}')

[called]
exten => s,1,Set(ODBC_FOO()=bar)

[caller]
exten => s,1,Gosub(called,s,1(moo))

Would end up executing INSERT INTO foo(moo) VALUES('bar') where I'd
naturally expect INSERT INTO foo() VALUES('bar'); and a resulting error.

I've addressed this with:  https://gerrit.asterisk.org/c/asterisk/+/15448

2.  Too many queries resulting in the database server (which is set for
maximum persistence) being unable to keep up, so enters
ODBC(transaction,mydb)=meh + ODBC(forcecommit)=1.  This sorts out the
COMMIT rate since now things are getting grouped into transactions.

3.  Transactions remains open for full duration of the call, in my case
it would be great if they can forcecommit when the channel gets answered
(currently the above result in the transaction only getting commit'ed
once the channel goes down, which is wasteful).  I can "fix" this by
adding manual commits just prior to Dial() in various places, but in
many of these cases I would prefer the transaction to persist, so there
are a few options:

3.1  Dial's G option.  As with the commit before Dial() there are a few
places this would need to be added and I might miss here.  If I do miss
one or two, assuming it's not on long calls (anything over a minute or
two) it's probably OK as the ODBC(forcecommit)=1 will eventually clean
up.  Config change only on my side, a fair amount of work and testing,
no code impact.

3.2  Create a hook similar to
https://wiki.asterisk.org/wiki/display/AST/Hangup+Handlers for pre_dial,
pre_bridge_aside and pre_bridge_bside kind of thing.  This may well be
useful for other things too.  Unsure of code impact, no behavioural
change except by config change.

3.3  Adjust ODBC(forcecommit) to take a set of values, such that the
value is a set of comma-separated values, what I can think of "atanswer"
and "athangup" where the current implementation is athangup, so you
could do Set(ODBC(forcecommit)=atanswer,athangup) and if the channel
goes into Up state, commit executes, as well as at hangup (in both cases
if a transaction is active).  In order to remain backwards identical,
any value that would normally evaluate to true should mean "athangup"
(even though I personally believe atanswer makes more sense).  Code
impact and potential (unwanted) behavioural impact.

3.4  Add an additional option to ODBC to determine when to commit, eg
ODBC(commitat)= ^^ above symantics, if ODBC(forcecommit) gets set we
simply set commitat=hangup or false we set commitat="", this seems
easier to keep 100% backwards compatible, but could be more confusing to
document.  Code impact.  Lower risk of behaviour change except by config
change, but more confusing to users and documentation will be tricky.

3.5  Outright change the behaviour of ODBC(forcecommit) to commit at
answer time instead of channel destruction time (I'm not in favour of
this as there are conceivably valid reasons to only commit once the
channel hangs up, mentioning for completeness sake).  Unkown code impact.

Whereas my above mentioned change/review only changes behaviour in case
of actual code/config change, the changes for 3 potentially has impact
that's a little more widespread.  Would appreciate input from others. 
Happy to cook the code.

Kind Regards,
Jaco