[asterisk-dev] [asterisk-addons-commits] tilghman: trunk r688 - in /trunk: ./ channels/ configs/ res/

[Atis Lezdins]

On Fri, Nov 7, 2008 at 8:49 PM, Tilghman Lesher
<tilghman at mail.jeffandtilghman.com> wrote:
> On Friday 07 November 2008 12:08:43 Atis Lezdins wrote:
>> On Fri, Nov 7, 2008 at 7:51 PM, Tilghman Lesher
>>
>> <tilghman at mail.jeffandtilghman.com> wrote:
>> > On Friday 07 November 2008 11:14:49 Atis Lezdins wrote:
>> >> On Fri, Nov 7, 2008 at 3:35 PM, Atis Lezdins <atis at iq-labs.net> wrote:
>> >> > Author: tilghman
>> >> > Date: Fri Nov  7 01:04:53 2008
>> >> > New Revision: 688
>> >> >
>> >> > URL: http://svn.digium.com/view/asterisk-addons?view=rev&rev=688
>> >> > Log:
>> >> > Revamp res_config_mysql to allow for multiple read/write handle
>> >> > configurations within realtime.
>> >> > (closes issue #13603)
>> >> >  Reported by: atis
>> >> >  Patch by: me
>> >> >
>> >> > Tilghman, i see that you did a great job refactoring db connections in
>> >> > trunk, however this still doesn't fix issue 0013603.
>> >> >
>> >> > Database name within extconfig.conf has been around for long time
>> >> > (even in 1.2), but now it's broken in 1.6.0.
>> >> >
>> >> > So, some sort of minor patch should be applied to addons-1.6.0.
>> >> >
>> >> > Btw, is it ok that there was no "commit comment" in  0013603?
>> >>
>> >> Tilghman, i wonder why do you keep closing this issue. Don't you admit
>> >> that it's a regression?
>> >>
>> >> As 1.6.1. has affected code refactored, there is probably no need for
>> >> fixing this, however it's an issue with 1.6.0 and I have uploaded a
>> >> patch long time ago. If necessary i can rewrite it in other way - so
>> >> that "database" is not shown in query.
>> >
>> > I'm not sure that your fix really is just that.  I'm concerned about it
>> > introducing further regressions.  The fix I created for addons-1.6.1 will
>> > fix the issue permanently, in a way that is scalable and doesn't simply
>> > remove the capability of separating reads from writes, which is what your
>> > patch effectively does.
>> >
>> > Yes, I admit the feature add in 1.6.0 was inadequate and disabled an
>> > important feature, and it is fixed going forward, but disabling another
>> > feature is just as much a regression.
>>
>> Actually i have tested this also on separate "dbread" and "dbwrite"
>> rules, and it worked perfectly. If necessary, this patch could be put
>> under testing to gather some more feedback.
>>
>> If you would take a look at patch itself, it does take into account
>> "database" specified for "dbread" on SELECT as well as "database" from
>> "dbwrite" upon INSERT and DELETE.
>>
>> So i'm confused - what exactly isn't working.
>
> The issue is that you can only specify a single database name in
> extconfig.conf.  So you can have the databases that are configured
> in res_mysql.conf (separate) or the one database configured within
> extconfig.conf (one connection for both).  You can't have it both ways with
> your patch.  The fix I introduced in trunk enables one to have both database
> connections specified in extconfig.conf, so you can have multiple database
> connections, yet still separate your reads from your writes, even within the
> context of having multiple database connections.  While you permit not turning
> off the separation by specifying the same name as the "read" database, that's
> really only a half feature.  You allow full separation for one set of
> databases, and for everything else, reads and writes go to the same database.
>

Hi Tilghman,

Today i tested numerous different scenarios with 1.6.0.

Now I see your point about having two different dbname=> entries in
res_mysql.conf, however i doubt that this was intent of initial
submitter. First sample config did had just one "dbname" entry.

Also i found out several very annoying things regarding realtime/mysql:

1) Completely no documentation regarding usage of [read] and [write]
connections.
2) CLI command "realtime mysql status" shows only one connection even
if two connections are established.
3) res_config_mysql should really mention which connection it's using
wherever possible. Otherwise logs are very confusing.

4) No CLI commands for store/destroy (more a  pbx_realtime problem)

5) CLI command "realtime update" has mixed up parameter order in
documentation (also pbx_realtime)

Usage: realtime update <family> <colupdate> <newvalue> <colmatch> <valuematch>
       Update a single variable using the RealTime driver.
       You must supply a family name, a column to update on, a new
value, column to match, and value to match.
       Ex: realtime update sipfriends name bobsphone port 4343
       will execute SQL as UPDATE sipfriends SET port = 4343 WHERE
name = bobsphone


So, taking a look at first points, i could agree that this read/write
stuff is not ready for 1.6.0. However some users (not me) could be
disappointied by removal of this feature from 1.6.0, so perhaps it
would be good to consider merging your version from trunk.

So, for now i'm reopening the issue until it's either fixed or removed
from 1.6.0.

I will also shortly create issues for rest of the list i found today,
offering patches wherever possible.

Regards,
Atis


-- 
Atis Lezdins,
VoIP Project Manager / Developer,
atis at iq-labs.net
Skype: atis.lezdins
Cell Phone: +371 28806004
Cell Phone: +1 800 7300689
Work phone: +1 800 7502835