[Maria-developers] MWL#36: feedback for post-review fixes
Hi Alexander, Please find the feedback for post-review fixes below:
=== modified file 'client/mysqlbinlog.cc' --- client/mysqlbinlog.cc 2009-10-24 19:43:39 +0000 +++ client/mysqlbinlog.cc 2009-10-27 13:42:47 +0000 @@ -38,8 +38,8 @@ /* Needed for Rpl_filter */ CHARSET_INFO* system_charset_info= &my_charset_utf8_general_ci;
-#include "sql_string.h" // needed for Rpl_filter -#include "sql_list.h" // needed for Rpl_filter +#include "sql_string.h" // needed for Rpl_filter +#include "sql_list.h" // needed for Rpl_filter #include "rpl_filter.h"
Rpl_filter *binlog_filter; @@ -1190,7 +1190,8 @@ (uchar**) &open_files_limit, (uchar**) &open_files_limit, 0, GET_ULONG, REQUIRED_ARG, MY_NFILE, 8, OS_FILE_LIMIT, 0, 1, 0}, {"rewrite-db", OPT_REWRITE_DB, - "Updates to a database with a different name than the original.", + "Updates to a database with a different name than the original. \ +Example: rewrite-db='from->to'.",
Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase.
0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0} };
=== modified file 'mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result' --- mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result 2009-10-24 21:48:58 +0000 +++ mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result 2009-10-27 13:42:47 +0000 @@ -216,6 +216,201 @@ # End of log file ROLLBACK /* added by mysqlbinlog */; /*!50003 SET COMPLETION_TYPE=@OLD_COMPLETION_TYPE*/; +# +# mysqlbinlog output +# --base64-output = decode-rows +# --rewrite-db = test1->new_test1 +# --rewrite-db = test3->new_test3 +# --read-from-remote-server +# +/*!40019 SET @@session.max_insert_delayed_threads=0*/; +/*!50003 SET @OLD_COMPLETION_TYPE=@@COMPLETION_TYPE,COMPLETION_TYPE=0*/; +DELIMITER /*!*/; +# at # +#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup
Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase.
+ROLLBACK/*!*/; +# at # +use new_test1/*!*/; +#010909 4:46:40 server id # end_log_pos # Query thread_id=# exec_time=# error_code=0 +SET TIMESTAMP=1000000000/*!*/; +SET @@session.pseudo_thread_id=#/*!*/;
After the above is fixed, we'll need to do another push and buildbot run to see if there are any problems. BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog
Hi Sergey,
Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase.
Just few lines above there is a sample which I followed: {"to-last-log", 't', "Requires -R. Will not stop at the end of the \ requested binlog but rather continue printing until the end of the last \ binlog of the MySQL server. If you send the output to the same MySQL server, \ that may lead to an endless loop.", So, either this does not matter or I should do similar changes everywhere in mysqlbinlog.cc (at least). Actually, I don't know whether it is regulated by coding style or some other reasons.
+#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase.
010909 4:46:40 is the result of # Fix timestamp to avoid varying results. SET timestamp=1000000000; in the *.test file. This SETing is used in all mysql_test/t/mysqlbinlog_*.test files (to relieve regex'es of additional replacement) and I followed this pattern. Alex 28.10.09, 00:08, "Sergey Petrunya" <psergey@askmonty.org>:
Hi Alexander, Please find the feedback for post-review fixes below:
=== modified file 'client/mysqlbinlog.cc' --- client/mysqlbinlog.cc 2009-10-24 19:43:39 +0000 +++ client/mysqlbinlog.cc 2009-10-27 13:42:47 +0000 @@ -38,8 +38,8 @@ /* Needed for Rpl_filter */ CHARSET_INFO* system_charset_info= &my_charset_utf8_general_ci;
-#include "sql_string.h" // needed for Rpl_filter -#include "sql_list.h" // needed for Rpl_filter +#include "sql_string.h" // needed for Rpl_filter +#include "sql_list.h" // needed for Rpl_filter #include "rpl_filter.h"
Rpl_filter *binlog_filter; @@ -1190,7 +1190,8 @@ (uchar**) &open_files_limit, (uchar**) &open_files_limit, 0, GET_ULONG, REQUIRED_ARG, MY_NFILE, 8, OS_FILE_LIMIT, 0, 1, 0}, {"rewrite-db", OPT_REWRITE_DB, - "Updates to a database with a different name than the original.", + "Updates to a database with a different name than the original. \ +Example: rewrite-db='from->to'.", Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase. 0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0} };
=== modified file 'mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result' --- mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result 2009-10-24 21:48:58 +0000 +++ mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result 2009-10-27 13:42:47 +0000 @@ -216,6 +216,201 @@ # End of log file ROLLBACK /* added by mysqlbinlog */; /*!50003 SET COMPLETION_TYPE=@OLD_COMPLETION_TYPE*/; +# +# mysqlbinlog output +# --base64-output = decode-rows +# --rewrite-db = test1->new_test1 +# --rewrite-db = test3->new_test3 +# --read-from-remote-server +# +/*!40019 SET @@session.max_insert_delayed_threads=0*/; +/*!50003 SET @OLD_COMPLETION_TYPE=@@COMPLETION_TYPE,COMPLETION_TYPE=0*/; +DELIMITER /*!*/; +# at # +#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase. +ROLLBACK/*!*/; +# at # +use new_test1/*!*/; +#010909 4:46:40 server id # end_log_pos # Query thread_id=# exec_time=# error_code=0 +SET TIMESTAMP=1000000000/*!*/; +SET @@session.pseudo_thread_id=#/*!*/; After the above is fixed, we'll need to do another push and buildbot run to see if there are any problems. BR Sergey
-- Эмоциональная почта находится здесь: http://mail.yandex.ru/promo/new/emotions
Hi Sergey,
Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase. Just few lines above there is a sample which I followed: {"to-last-log", 't', "Requires -R. Will not stop at the end of the \ requested binlog but rather continue printing until the end of the last \ binlog of the MySQL server. If you send the output to the same MySQL server, \ that may lead to an endless loop.", So, either this does not matter or I should do similar changes everywhere in mysqlbinlog.cc (at least). Actually, I don't know whether it is regulated by coding style or some other reasons.
Done.
+#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase. 010909 4:46:40 is the result of # Fix timestamp to avoid varying results. SET timestamp=1000000000; in the *.test file. This SETing is used in all mysql_test/t/mysqlbinlog_*.test files (to relieve regex'es of additional replacement) and I followed this pattern.
Didn't change. Alex 28.10.09, 00:42, "Alexi1952" <Alexi1952@yandex.ru>:
Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase. Just few lines above there is a sample which I followed: {"to-last-log", 't', "Requires -R. Will not stop at the end of the \ requested binlog but rather continue printing until the end of the last \ binlog of the MySQL server. If you send the output to the same MySQL server, \
Hi Sergey, that may lead to an endless loop.", So, either this does not matter or I should do similar changes everywhere in mysqlbinlog.cc (at least). Actually, I don't know whether it is regulated by coding style or some other reasons.
+#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase. 010909 4:46:40 is the result of # Fix timestamp to avoid varying results. SET timestamp=1000000000; in the *.test file. This SETing is used in all mysql_test/t/mysqlbinlog_*.test files (to relieve regex'es of additional replacement) and I followed this pattern. Alex 28.10.09, 00:08, "Sergey Petrunya" : Hi Alexander, Please find the feedback for post-review fixes below: === modified file 'client/mysqlbinlog.cc' --- client/mysqlbinlog.cc 2009-10-24 19:43:39 +0000 +++ client/mysqlbinlog.cc 2009-10-27 13:42:47 +0000 @@ -38,8 +38,8 @@ /* Needed for Rpl_filter */ CHARSET_INFO* system_charset_info= &my_charset_utf8_general_ci;
-#include "sql_string.h" // needed for Rpl_filter -#include "sql_list.h" // needed for Rpl_filter +#include "sql_string.h" // needed for Rpl_filter +#include "sql_list.h" // needed for Rpl_filter #include "rpl_filter.h"
Rpl_filter *binlog_filter; @@ -1190,7 +1190,8 @@ (uchar**) &open_files_limit, (uchar**) &open_files_limit, 0, GET_ULONG, REQUIRED_ARG, MY_NFILE, 8, OS_FILE_LIMIT, 0, 1, 0}, {"rewrite-db", OPT_REWRITE_DB, - "Updates to a database with a different name than the original.", + "Updates to a database with a different name than the original. \ +Example: rewrite-db='from->to'.", Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase. 0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0} };
=== modified file 'mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result' --- mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result 2009-10-24 21:48:58 +0000 +++ mysql-test/suite/binlog/r/binlog_row_mysqlbinlog_options.result 2009-10-27 13:42:47 +0000 @@ -216,6 +216,201 @@ # End of log file ROLLBACK /* added by mysqlbinlog */; /*!50003 SET COMPLETION_TYPE=@OLD_COMPLETION_TYPE*/; +# +# mysqlbinlog output +# --base64-output = decode-rows +# --rewrite-db = test1->new_test1 +# --rewrite-db = test3->new_test3 +# --read-from-remote-server +# +/*!40019 SET @@session.max_insert_delayed_threads=0*/; +/*!50003 SET @OLD_COMPLETION_TYPE=@@COMPLETION_TYPE,COMPLETION_TYPE=0*/; +DELIMITER /*!*/; +# at # +#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase. +ROLLBACK/*!*/; +# at # +use new_test1/*!*/; +#010909 4:46:40 server id # end_log_pos # Query thread_id=# exec_time=# error_code=0 +SET TIMESTAMP=1000000000/*!*/; +SET @@session.pseudo_thread_id=#/*!*/; After the above is fixed, we'll need to do another push and buildbot run to see if there are any problems. BR Sergey
-- Предупредительная почта находится здесь: http://mail.yandex.ru/promo/new/notifications
On Wed, Oct 28, 2009 at 12:42:20AM +0300, Alexi1952 wrote:
Hi Sergey,
Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase.
Just few lines above there is a sample which I followed:
{"to-last-log", 't', "Requires -R. Will not stop at the end of the \ requested binlog but rather continue printing until the end of the last \ binlog of the MySQL server. If you send the output to the same MySQL server, \ that may lead to an endless loop.",
So, either this does not matter or I should do similar changes everywhere in mysqlbinlog.cc (at least). Actually, I don't know whether it is regulated by coding style or some other reasons.
I see, it seems both ways are in use (I was looking mostly at the server code), so either way is ok.
+#010909 4:46:40 server id # end_log_pos # Start: binlog v 4, server v #.##.## created 010909 4:46:40 at startup Apparently this testcase will succeed only at 4:40 on 01-Sept-2009. Please make a stable testcase.
010909 4:46:40 is the result of
# Fix timestamp to avoid varying results. SET timestamp=1000000000;
in the *.test file. This SETing is used in all mysql_test/t/mysqlbinlog_*.test files (to relieve regex'es of additional replacement) and I followed this pattern.
So it's repeatable. Thanks for the explanation, the test is ok then. It seems that buildbot runs have not revealed any failures specific to this WL, so we can mark the review as complete. BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog
Hi!
"Sergey" == Sergey Petrunya <psergey@askmonty.org> writes:
Sergey> On Wed, Oct 28, 2009 at 12:42:20AM +0300, Alexi1952 wrote:
Hi Sergey,
Please change to use multiple-strings; "Updates to a database with a different name than the original." "Example: rewrite-db='from->to'.", as that is what is done around the codebase.
Just few lines above there is a sample which I followed:
{"to-last-log", 't', "Requires -R. Will not stop at the end of the \ requested binlog but rather continue printing until the end of the last \ binlog of the MySQL server. If you send the output to the same MySQL server, \ that may lead to an endless loop.",
So, either this does not matter or I should do similar changes everywhere in mysqlbinlog.cc (at least). Actually, I don't know whether it is regulated by coding style or some other reasons.
Sergey> I see, it seems both ways are in use (I was looking mostly at the server Sergey> code), so either way is ok. For new stuff, we should use multiple-strings, as this is easier to read. We shouldn't change all old stuff, as this will make merges with the MySQL branch harder to do. <cut> Regards, Monty
participants (3)
-
Alexi1952 -
Michael Widenius -
Sergey Petrunya