Merge lp:~dingqi-lxb/percona-server/5.5_log_queries_in_memory into lp:percona-server/5.5
Status: | Rejected |
---|---|
Rejected by: | 林晓斌 on 2011-12-21 |
Proposed branch: | lp:~dingqi-lxb/percona-server/5.5_log_queries_in_memory |
Merge into: | lp:percona-server/5.5 |
Diff against target: |
898 lines (+879/-0) (has conflicts) 3 files modified
doc/source/diagnostics/slow_log_in_memory.rst (+74/-0) patches/series (+4/-0) patches/slowlog_in_memory.patch (+801/-0) Text conflict in patches/series |
To merge this branch: | bzr merge lp:~dingqi-lxb/percona-server/5.5_log_queries_in_memory |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Oleg Tsarev (community) | 2011-12-11 | Needs Fixing on 2011-12-11 | |
Review via email:
|
This proposal has been superseded by a proposal from 2011-12-20.
Description of the change
patch for https:/
add a variable 'slowlog_
If the current_memory grows larger than max_memory, the eldest ones will be wash out.
Same action happens when max_mamory are set to a new value that is less than current_memory.
Oleg Tsarev (tsarev) wrote : | # |
8.
+ ulonglong utime_of_query= thd->current_
+ if (utime_of_query <= thd->variables.
+ return;
Please look into the slow_extended.
static inline ulonglong get_query_
...
thd->server_
Please better use this status variable, instead of calculate query execution time directly.
林晓斌 (dingqi-lxb) wrote : | # |
Thank you. Oleg, I have some items to discuss.
2. About innodb_trx_id. Please look to two following cases
In these two cases, the variable thd->innodb_trx_id is set to 0. It seems that I have to show it in the result table as 0, is there other result even use innodb_was_used ?
8.About calculating query execution time directly, it is because that I have to get the query_time, the status is not enough.
* Why plugin doesn't uninstall in test?
You mean that it shows " Plugin is busy and will be uninstalled on shutdown" in result-file? It is the same with the demo for audit plugin "null_audit", the strategy decided by mysqld
Oleg Tsarev (tsarev) wrote : | # |
On Mon, Dec 12, 2011 at 12:05 PM, 林晓斌 <email address hidden> wrote:
> Thank you. Oleg, I have some items to discuss.
>
> 2. About innodb_trx_id. Please look to two following cases
> In these two cases, the variable thd->innodb_trx_id is set to 0. It seems that I have to show it in the result table as 0, is there other result even use innodb_was_used ?
I think right decision is "NULL" value instead of 0.
>
> 8.About calculating query execution time directly, it is because that I have to get the query_time, the status is not enough.
>
> * Why plugin doesn't uninstall in test?
> You mean that it shows " Plugin is busy and will be uninstalled on shutdown" in result-file? It is the same with the demo for audit plugin "null_audit", the strategy decided by mysqld
Ok
> --
> https:/
> You are reviewing the proposed merge of lp:~dingqi-lxb/percona-server/5.5_log_queries_in_memory into lp:percona-server.
--
Oleg Tsarev, Software Engineer, Percona Inc.
林晓斌 (dingqi-lxb) wrote : | # |
2. About innodb_trx_id. Generally, when the thd->innodb_trx_id is 0, the value of "INNODB_TRX_ID" should show NULL, so the thd->innodb_
7. Please add tests to sys_vars suite for the all added variables.
The variable add in this plugin will not be seen in other test-case, it can only be seen the plugin is installed. So, is test-case in sys_vars suits necessary?
1. Please add patch name to the "patches/series" file
2. About innodb_trx_id. Please look to two following cases:
* log_slow_verbosity doesn't set "innodb"
* query doesn't use the InnoDB
In first case slow query log will has no information about InnoDB
In second case slow query will have line "# No InnoDB statistics available for this query\n"
In this two cases innodb_trx_id will be incorrect, and you can't use and setup it.
Please handle variable innodb_was_used correctly (at least for innodb_trx_id)
3. Following is very, very bad: query= "NULL"; query_len= 5; SCHEMA. I_S_SLOW_ LOG WHERE QUERY IS NULL
+ if (general_query)
+ {
....
+ }
+ else
+ {
+ this->general_
+ this->general_
+ }
You are break SQL semantic. For example query:
SELECT * FROM INSFORMATION_
or any join will not work correctly.
You are break SQL language semantic by string "NULL" instead of NULL
Same for Schema.
4. I_S_SLOW_LOG not very good name. Why SLOW_LOG bad?
5. Please add documentation to the doc/
6. Sleep in the test file - bad idea. Please replace this by debug variable query_exec_time.
7. Please add tests to sys_vars suite for the all added variables.
Please fix this, and I will review again.