Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asterisk_log indexes (Performance Improvement Suggestions) #47

Closed
miksir opened this issue Jul 26, 2012 · 16 comments
Closed

asterisk_log indexes (Performance Improvement Suggestions) #47

miksir opened this issue Jul 26, 2012 · 16 comments

Comments

@miksir
Copy link

miksir commented Jul 26, 2012

It's strange, but this table almost without indexes. My suggestion (based on fast review of source code):

  KEY `timestampCall` (`timestampCall`),
  KEY `uistate` (`uistate`),
  KEY `callstate` (`callstate`),
  KEY `channel` (`channel`),
  KEY `asterisk_id` (`asterisk_id`),
  KEY `asterisk_dest_id` (`asterisk_dest_id`),
  KEY `call_record_id` (`call_record_id`)
@blak3r
Copy link
Owner

blak3r commented Jul 30, 2012

Performance is definitely something that could use some improvements. If you'd like to help, please take a look here:
https://github.com/blak3r/yaai/wiki/Project-TODO-List#wiki-Performance_Improvements

@miksir
Copy link
Author

miksir commented Aug 1, 2012

Index on timestampCall will work with < and index on channel will work with LIKE in this query (indexes work with LIKE if we search from beginning of the string)
Sure indexes will be useful if we are not cleaning up table. If we do - very important is call traffic. If number of calls low - MySQL can skip indexes and use full scan. But anyway indexes will not interfere.
It's like quick fix.

About long fix. First, do not use NULL records, put default value. So (uistate IS NULL OR uistate != "Closed") can be easy used as uistate != "Closed".

Second, my suggestion to move extension extraction to asteriskLogger. Can extract from Channel with simple regexp or even use other field of asterisk packet. I finished to write my own version of asteriskLogger and I can say, that extension number can be found in CallerIDNum or ConnectedLineNum (depend on direction of call).
As result, you can replace (channel LIKE 'SIP/{$current_user->asterisk_ext_c}%' OR channel LIKE 'Local%{$current_user->asterisk_ext_c}%') with simple extension = "{$current_user->asterisk_ext_c}"

And this request will be very fast.

@blak3r
Copy link
Owner

blak3r commented Aug 3, 2012

@miksir I like your suggestions for indexing and default values. Any chance you could figure out how to put all that into the pre_install.php script so it alters people's database that already have it and will create it properly in the first place for people who don't.

While you're at it go ahead and add a new column if need be for the extension storage.

The extension extraction is a great idea... Moving the extension is a great idea... that alone will prevent table scans.

@miksir
Copy link
Author

miksir commented Aug 3, 2012

I'll try to look pre_install.php on monday.

@blak3r
Copy link
Owner

blak3r commented Aug 8, 2012

@miksir Any luck? Also, can you email me at blake.robertson [at gmail.com] please include your github name in the body.

@miksir
Copy link
Author

miksir commented Aug 8, 2012

08.08.2012 9:02, Blake Robertson пишет:

@miksir https://github.com/miksir Any luck? Also, can you email me
at blake.robertson [at gmail.com] please include your github name in
the body.

Yeh, sorry, was little busy. I decided to rewrite all yaai :)
I wrote fixes to pre_install.php (attachment) .
Changed almost all NULL columns to NOT NULL, added indexes and changed
some columns length for match asterisk length of this data.
I did one test - all look good, but test it too pls :)

@blak3r
Copy link
Owner

blak3r commented Aug 8, 2012

Sweet. I'd be the happiest guy in the world if you solved the performance bottlenecks! The changes to pre_install look really clean. Haven't tested but will soon.

@miksir
Copy link
Author

miksir commented Aug 8, 2012

On 9 Август 2012 г. 0:54:48, Blake Robertson wrote:

Sweet. I'd be the happiest guy in the world if you solved the
performance bottlenecks! The changes to pre_install look really clean.
Haven't tested but will soon.

Do you really have performance troubles in your work? I have only 2-3
operators, so even 700ms fine for me. After installed APC accelerator
ajax request response time about 60-80ms, and my server PC not strong.

In rewritten version this requests to asterisk_log isolated to separate
class, and I'm already thought about Memcache implementation.


Reply to this email directly or view it on GitHub
#47 (comment).

@blak3r
Copy link
Owner

blak3r commented Aug 8, 2012

No I don't. I only have about 10 and my sugar server is a beast.
I suspect it'll be an issue for larger deployments. Also, as it stands now
the performance will get worse and worse as asterisk_log gets more and more
rows.

~blake

On Wed, Aug 8, 2012 at 5:19 PM, miksir notifications@github.com wrote:

On 9 Август 2012 г. 0:54:48, Blake Robertson wrote:

Sweet. I'd be the happiest guy in the world if you solved the
performance bottlenecks! The changes to pre_install look really clean.
Haven't tested but will soon.

Do you really have performance troubles in your work? I have only 2-3
operators, so even 700ms fine for me. After installed APC accelerator
ajax request response time about 60-80ms, and my server PC not strong.

In rewritten version this requests to asterisk_log isolated to separate
class, and I'm already thought about Memcache implementation.


Reply to this email directly or view it on GitHub
#47 (comment).


Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-7597862.

@miksir
Copy link
Author

miksir commented Aug 8, 2012

On 09.08.2012 2:46, Blake Robertson wrote:

No I don't. I only have about 10 and my sugar server is a beast.
I suspect it'll be an issue for larger deployments. Also, as it stands
now
the performance will get worse and worse as asterisk_log gets more and
more
rows.
Need to find how to add new job to Sugar's job manager (during Yaai
installation) and cleanup this table time to time.

@blak3r
Copy link
Owner

blak3r commented Aug 9, 2012

That shouldn't be too hard.

I noticed that the plugin jjw design maps adds a job to the scheduler so could look at his code for that.

What were you thinking of doing a part of me doesn't like the idea of deleting anything.

Oh and I thought of something that impacts performance. If you have people that open like 6 browser tabs... Each tab is effectively another user since they all independently poll.

~blake

On Aug 8, 2012, at 7:01 PM, miksir notifications@github.com wrote:

On 09.08.2012 2:46, Blake Robertson wrote:

No I don't. I only have about 10 and my sugar server is a beast.
I suspect it'll be an issue for larger deployments. Also, as it stands
now
the performance will get worse and worse as asterisk_log gets more and
more
rows.
Need to find how to add new job to Sugar's job manager (during Yaai
installation) and cleanup this table time to time.

Reply to this email directly or view it on GitHub.

@blak3r
Copy link
Owner

blak3r commented Oct 2, 2012

@miksir had emailed me some changes he made to pre_install to add indexes.

I'm posting them here so I remember to merge them in to our code. Thanks for your contribution.

        // Columns Added in v2.0
        add_column_if_not_exist($db,"asterisk_log","uistate", "VARCHAR(10) DEFAULT NULL");
        add_column_if_not_exist($db,"asterisk_log","remote_channel", "VARCHAR(30) DEFAULT NULL");
        add_column_if_not_exist($db,"asterisk_log","contact_id", "VARCHAR(36) DEFAULT NULL");
        // Columns Added in v2.3
        add_column_if_not_exist($db,"asterisk_log","opencnam", "VARCHAR(16) DEFAULT NULL");

        //
        add_index_if_not_exist($db,"asterisk_log","timestampCall");
        add_index_if_not_exist($db,"asterisk_log","uistate");
        add_index_if_not_exist($db,"asterisk_log","callstate");
        add_index_if_not_exist($db,"asterisk_log","channel");
        add_index_if_not_exist($db,"asterisk_log","asterisk_id");
        add_index_if_not_exist($db,"asterisk_log","asterisk_dest_id");
        add_index_if_not_exist($db,"asterisk_log","call_record_id");

        modify_column($db, "asterisk_log", "call_record_id", "char(36) NOT NULL default ''");
        modify_column($db, "asterisk_log", "asterisk_id", "varchar(150) NOT NULL default ''");
        modify_column($db, "asterisk_log", "callstate", "varchar(10) NOT NULL default ''");
        modify_column($db, "asterisk_log", "uistate", "varchar(10) NOT NULL default ''");
        modify_column($db, "asterisk_log", "callerID", "varchar(45) NOT NULL default ''");
        modify_column($db, "asterisk_log", "callerName", "varchar(45) NOT NULL default ''");
        modify_column($db, "asterisk_log", "channel", "varchar(80) NOT NULL default ''");
        modify_column($db, "asterisk_log", "remote_channel", "varchar(80) NOT NULL default ''");
        modify_column($db, "asterisk_log", "timestampCall", "datetime NOT NULL");
        modify_column($db, "asterisk_log", "direction", "varchar(1) NOT NULL default ''");
        modify_column($db, "asterisk_log", "asterisk_dest_id", "varchar(150) NOT NULL default ''");
        modify_column($db, "asterisk_log", "contact_id", "VARCHAR(36) NOT NULL default ''");
        modify_column($db, "asterisk_log", "opencnam", "VARCHAR(16) NOT NULL default ''");
    }
}

// http://www.edmondscommerce.co.uk/mysql/mysql-add-column-if-not-exists-php-function/
function add_column_if_not_exist($db, $table, $column, $column_attr = "VARCHAR( 255 ) NULL" ){
    $exists = false;
    $cols = $db->get_columns($table);
    if( !array_key_exists($column, $cols) ) {
        $db->query("ALTER TABLE `$table` ADD `$column`  $column_attr");
    }
}

function modify_column($db, $table, $column, $column_attr) {
    $db->query("ALTER TABLE `$table` MODIFY `$column` $column_attr");
}

function add_index_if_not_exist($db, $table, $index) {
    $res = $db->query("SHOW INDEX FROM `$table` WHERE Key_name = '$index'");
    if (empty($res)) {
        $db->query("ALTER TABLE `$table` ADD INDEX $index($index)");
    }
}

@blak3r
Copy link
Owner

blak3r commented Nov 6, 2012

@miksir I was looking at this code again and was wondering what the reasoning behind changing all the columns from being NULL to NOT NULL by default.

@miksir
Copy link
Author

miksir commented Nov 15, 2012

Sorry for long delay :) Forgot to answer :)
Well, as i known, NULL in index need to use additional decisions. So index of NOT NULL fields processed more simply and probably faster. Also found it now http://stackoverflow.com/questions/471367/when-to-use-null-in-mysql-tables
But I think we need to review it and set null or not null based on semantic. Field which can not be undefined set to not null. But fields like opencnam can be in 3 states: unknown yet (null), not found (empty string), found (string).

@blak3r
Copy link
Owner

blak3r commented Nov 15, 2012

Ok. That makes sense. Not sure it's worth the effort to go through and make the app not null compatible.

From the little investigation, we decided best approach is to just purge asterisk log table of all expired events.

This would be done when ever asterisk log ami timeout occurs.

That way all queries can be cached completely since there would be no need to query based on time parameters.

I've added a user extension column which will be queried in the v3 branch which will further eliminate the need for any regex's in the query. (But haven't yet implemented it)

It's slated for the v3 release. There is a branch for this.

One thing to note is I don't think the index adding in preinstall is working.

~blake

On Nov 15, 2012, at 1:22 PM, miksir notifications@github.com wrote:

Sorry for long delay :) Forgot to answer :)
Well, as i known, NULL in index need to use additional decisions. So index of NOT NULL fields processed more simply and probably faster. Also found it now http://stackoverflow.com/questions/471367/when-to-use-null-in-mysql-tables
But I think we need to review it and set null or not null based on semantic. Field which can not be undefined set to not null. But fields like opencnam can be in 3 states: unknown yet (null), not found (empty string), found (string).


Reply to this email directly or view it on GitHub.

@blak3r
Copy link
Owner

blak3r commented Dec 18, 2012

I've implemented most of these things in the V3 branch. Closing this thread now. Thanks @miksir

@blak3r blak3r closed this as completed Dec 18, 2012
blak3r added a commit that referenced this issue Jun 17, 2013
…s much faster

AJAX calls to get call info for call popups are much faster.  They no longer include timestamps or Like expressions which prevented queries from being cached.  Now asteriskLogger periodically purges event from asterisk_log table which are no longer relevant.  This is a fairly signifigant change as you can no longer use asterisk log for reporting... but I don't think many users did this anyway.  Instead design reports around Calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants