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 -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev