[Home]

Summary:ASTERISK-05728: [patch] Allow use of separate MySQL servers for SELECT and UPDATE queries
Reporter:Loic Didelot (voipgate)Labels:
Date Opened:2005-11-28 16:14:42.000-0600Date Closed:2008-10-02 08:58:40
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/Configuration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20060927_bug5881_rev311.res_config_mysql.patch
( 1) 20061104_bug5811_rev315.res_config_mysql.patch
( 2) 20070409__res_config_mysql.diff.txt
( 3) res_config_mysql_new.patch
( 4) res_config_mysql.patch
Description:Hello,
I am using the realtime mysql addon in asterisk-1.2. For testing purposes I tried to simulate network problems so that asterisk can not connect to the mysql server. For this tests I just used simple IPTABLES rules on the asterisk server:
- iptables -j DROP -A OUTPUT -d MYSQL_SERVER
- iptables -j REJECT -A OUTPUT -d MYSQL_SERVER
....

So after installing the iptables rules I did an "realtime mysql status" on the asterisk-cli. After executing this command the asterisk-cli was no longer reacting to any commands. I had to quit using CTRL-C and reconnect with asterisk-rvvvv.

After this I deleted the iptables rules: iptables --flush. I connect again to the asterisk-cli and execute "realtime mysql status" After this asterisk crashes without and hints in any logfile.

****** ADDITIONAL INFORMATION ******

I uploaded a patch wich does not solve the problem but which give you the possibility to have 2 mysql connections, one for SELECT queries and one for UPDATE queries. This can be useful when trying to loadbalance the mysql structure. Well someon need to correct my lousy C code.

Here is my res_mysql.conf:
[general]
dbname = voipgate2

dbhost-read = SERVER_IP
dbuser-read = USERNMAE
dbpass-read = PASSWORD
dbport-read = 3306
dbsock-read = /tmp/mysql.sock

dbhost-write = SERVER_IP
dbuser-write = USERNAME
dbpass-write = PASSWORD
dbport-write = 3306
dbsock-write = /tmp/mysql2.sock
Comments:By: David James (davidj) 2005-11-30 00:33:40.000-0600

if you use iptables to REJECT a packet I think it generates some weird error that won't be handled by any reconnecting algorithm.  what if you just stop/sleep 5/start mysql?

also can you recreate the problem with ODBC, where ODBC is connecting to the same mysql database?  I have found that this works fine for me ( MS SQL)

Also, the way you are doing load balancing is flawed. How would that improve performance? Have you tried connection pooling?


By: twisted (twisted) 2005-11-30 00:47:59.000-0600

initials in comments are nono.

Also, what does this patch have to do with this issue?
By: Loic Didelot (voipgate) 2005-11-30 01:26:43.000-0600

- the patch has noting to do with the issue. but it was while i was developping and testing the patch that i noticed the problems.

- by doing a master-slave or master-slave-slave-slave... replication and putting some loadbalancers (LVS) in front of the slave servers you can really remove all the load coming from the select querries from the important master server which can only be scaled through a master-master replication (i dont like it) or through a mysql cluster (could be expensive).
By: Matt Riddell (zx81) 2005-11-30 04:53:55.000-0600

The splitting of mysql into read and write really allows Asterisk realtime to work in a much better way.

The replication is currently useless as you can't update the slave databases without two way replication, and so can't really have peers registering to boxes that are serviced by MySQL slaves.

This way we could split the updates and send them to the master, and then still read from the slaves.  With newer versions of MySQL you can then also get a slave to become a master in a failover situation and so it increases Asterisk's redundancy.  Not to mention you can redirect people's requests to different boxes (pre asterisk) so that everyone registers to a server closest to them.

:) So, what needs to be changed to get this patch in appart from the inline initial comments?
By: Loic Didelot (voipgate) 2005-11-30 06:14:52.000-0600

Hey, I have one master (dual cpu) and 5 slaves (low cost).  With my patch I send the update-queries to the master and the select-queries to the loadbalancer in front of the 5 slaves. Like this, my mysql select structure is never down and can handle a high traffic of mysql requests.
By: twisted (twisted) 2005-11-30 10:16:22.000-0600

Stick to the bug in here.  if you want to file your patch for possible inclusion, it belongs in it's own issue.
By: Loic Didelot (voipgate) 2005-11-30 11:30:06.000-0600

it has been reported that the way that i did my tests do not really simulate a real life situation, so i guess what i reported is not really a bug. so can anyone  change my patch into a feature request, or whatever....
By: Matt Riddell (zx81) 2005-11-30 23:57:50.000-0600

Could you attach another patch without the start and end initials comments in the code?

Also, have you sent in your disclaimer?  If not, could you?  

The code can't really be looked at until there is one.

:)

Ah, also could the subject be changed to "Allow MySQL to use seperate Read and Write servers for Increased Scalability"


By: Loic Didelot (voipgate) 2005-12-02 03:22:10.000-0600

I just faxed the disclaimer to Digium. And now uploaded the patch without my comments.

PS:I asked to Digium 2 weeks ago to develop this for us. Till now I did not get a quote. I guess, that now I do not need ony anymore.

cjk
By: Matt Riddell (zx81) 2005-12-16 05:14:59.000-0600

Anyone know what this is waiting on?
By: Matt Riddell (zx81) 2005-12-20 04:20:37.000-0600

Could somebody change the status to Disclaimer on File as he has posted. Also, could someone do a new code review as he has changed the code as requested.

:)

Also, any idea why this bug doesn't show up in the list when it is unfiltered?


By: Olle Johansson (oej) 2005-12-20 06:20:34.000-0600

Some comments:
- Please do not log to the DEBUG channel without testing option_debug - if debug level is 0, nothing should be printed to the DEBUG channel.
 
   if (option_debug)

or

  if (option_debug > 1)

- Please do not use a 1 or a 0 as a parameter to mysql_reconnect, use some meaningful enum or #define so the code is more readable

- I do not understand these log messages:
  + ast_log(LOG_DEBUG, "MySQL RealTime READi LIOC debug.\n");
+ ast_log(LOG_DEBUG, "MySQL RealTime READi LIOC debug222 reload.\n");

I would use pointers for the database configuration and embed it in a struct. If someone does not want to declare two database handlers, we could have them point to the same config. We need to be backwards compatible, so the default handler needs to stay there as "dbhost" etc. You are not allowed to change that to "dbhost-read" without handling the old config way for only one database connection.

- There's some bad formatting here (in two places) ...and whats CJK ?

+ if(status==1){ /*READ*/
+ ast_log(LOG_DEBUG, "MySQL RealTime READ CJK: WE RECONNECT\n");
+ if(connected!=1) {
+ ast_log(LOG_DEBUG, "MySQL RealTime READ CJK: WE ARE NOTE CONNECTED\n");


I agree that the functionality is a good thing, but the patch needs some more work. Thanks for contributing to Asterisk. Looking forward to next version of the patch!

/Olle
By: Loic Didelot (voipgate) 2005-12-22 04:45:22.000-0600

Olle,
you are right concerning my bad, unexperienced programming style. And if I would be digium I would not include that patch like this.

But I will not update my patch. I just dont have enough C knowhow and do not have much time at the moment. My patch works and I have done some stress testing.

cjk (<= my nickname)
By: Olle Johansson (oej) 2005-12-22 07:42:58.000-0600

I did not say that you where bad, just gave you some advice on how to make it into something that we could include. You are not that far away. Please try again!
By: Olle Johansson (oej) 2006-01-03 05:04:38.000-0600

If we are not getting an update to this patch, we have to close this report. Can someone please spend some time cleaning it up?
By: Loic Didelot (voipgate) 2006-01-03 08:49:46.000-0600

Hi,
it might take me 2 or 3 hours to update this patch, but I really have not the time at the moment but I will do this as soon as possible if noone else is willing to do it.

Best regards,
Loic Didelot.
By: Clod Patry (junky) 2006-01-06 10:43:39.000-0600

Since db*2 is related to write queries, why not just called it dbwrite, since 2 means nothing crucial.
By: Olle Johansson (oej) 2006-01-30 14:09:54.000-0600

Any updates to this issue?

/Housekeeping
By: Loic Didelot (voipgate) 2006-01-30 14:43:26.000-0600

Hello,
after really extensive testing under heavy load I got to the conclusion that the current implementation with or without my patch is buggy as hell. As soon as you have alot of mysql update and selects and that your mysql server takes only half a second to respond (mysql not on localhost), asterisk crashes. The use of mysql_ping etc is really not the right way to do it. So I think this can be closed as I will not continue on this. A new mysql implementation might be available soon.
By: Olle Johansson (oej) 2006-01-31 00:13:04.000-0600

Yes, there are a lot of problems with the current MySQL drivers. Thank you for looking into this.
By: Serge Vecher (serge-v) 2006-09-27 11:36:36

opening at the request of amdtech on asterisk-dev list (
http://lists.digium.com/pipermail/asterisk-dev/2006-September/023559.html)

Please attach a patch for *trunk* here.
By: Aaron Daniel (adaniel) 2006-09-27 11:47:21

I've modified the patch to work with trunk, as well as making it backwards compatible to an extent.  A run down of what it does:

* The patch still splits out reads and writes (even for the same database) so as to allow for a separation if someone wants to do that later down the road.
* The configuration makes a little more sense.  The "general" section contains global defaults, and then there's a more specific "read" section and "write" section.  If you configure say, dbhost under general, and it's not going to be different across reads and writes, you only have to configure it the one time.  On the same note, you can configure dbname to be general, and configure dbhost separately in the read/write sections.
* The patch modifies the sample config as well to list those options.

Let me know what ya'll think.
By: Aaron Daniel (adaniel) 2006-09-27 11:55:35

side note: my disclaimer is on file
By: JR Richardson (hubguru) 2006-09-27 12:11:25

I'm clustering multiple Asterisk registration servers where the realtime [write] event is sent to a Master MySQL server sitting across the LAN network and [read] events pointed to the localhost replicated database.  The Master is setup to replicate the realtime database to the registration servers.  MySQL replication is far easier to setup than clustering, also you can have 2 Masters with HA failover.  Replication also reduces the amount of servers needed to scale a MySQL cluster.

This patch is working.  I'm not stress testing is though.  Trying to figure out what the best method would be.  As it stands, reading realtime info from local host is a hardware resource issue, how fast can the mysql database respond locally, any limitations here?, none that I can see.  The Asterisk process would crater long before MySQL would have any ill effects.

In an asterisk cluster environment, spreading the load across several asterisk servers reduces the load on individual nodes and allows for grater scale across the cluster.  The write events to the master database from phones registering is so minimal, can't really stress test that, even 1000+ phones registering would not come close to bottlenecking on the master database or the network.

The database replication event is so light; MySQL reported that 1 master could host 15-20 replication slaves without any performance issues.

I'm not seeing any downside here.  This patch is a major contribution to clustering Asterisk.
By: JR Richardson (hubguru) 2006-09-27 12:12:23

Forgot to mention, I'm using asterisk-adddons 1.2.3
By: Serge Vecher (serge-v) 2006-09-27 13:19:13

amdtech: a couple of formatting issues, if you will, as per CODING GUIDELINES:
1) give ample spacing in several places, like:
if(mysql_reconnect(NULL,WRITE_OPT)) {
+ connect_status2=1;
+ if(dbhost_write) {
2) single line if ... else ... constructs don't need curly brackets...

hubguru: the patch is against trunk, not 1.2.3. Are you reporting on a different version of the patch or the patch applied ok to 1.2.3 sources?
By: Aaron Daniel (adaniel) 2006-09-27 13:37:29

on the extra spacing, do you mean convert it to something like:
if (mysql_reconnect(NULL, WRITE_OPT)) {
+ connect_status2 = 1;
+ if (dbhost_write) {

my biggest concern with that, I can go through all the code in there and do it if you like to make the whole module follow the guidelines, or just the stuff in the patch...

also, the if/else constructs, same question... should I just fix those in the patch or the module as a whole (most of those were directly copied and modified from the module itself to separate the functions out).
By: JR Richardson (hubguru) 2006-10-02 17:12:04

serge-v, I asked amdtech to make me a patch for asterisk-addons-1.2.3 and he was able to do it in short order.  So, no i did not apply the attached patch to 1.2.3 but applied the one he made me for 1.2.3
By: jmls (jmls) 2006-11-02 12:18:14.000-0600

ping. housekeeping. where are we with this ?
By: Aaron Daniel (adaniel) 2006-11-02 12:29:33.000-0600

I still need to clean up the patch to match the coding guidelines, haven't had a chance to look at it yet.  I'll look at it again tonight to get it up to speed with latest trunk.
By: Aaron Daniel (adaniel) 2006-11-04 17:14:13.000-0600

The latest patch uploaded has the spacing fixes, I did it across the entire module since it was easier to just run :%s/if(/if (/g in vim than to go through and do it by hand on only a few.  Also got rid of any extraneous braces as per the CODING GUIDELINES.  Let me know if you need anything else out of it.
By: Tilghman Lesher (tilghman) 2007-04-09 13:53:01

Complete revamp of the code, with the coding guidelines in mind.
By: Tilghman Lesher (tilghman) 2007-04-10 18:07:02

Committed, revision 362.