Re: [Maria-developers] Updated (by Guest): Add a mysqlbinlog option to change the used database (36)
worklog-nore...@askmonty.org writes: > -=-=(Guest - Sat, 03 Oct 2009, 02:27)=-=- > High-Level Specification modified. > --- /tmp/wklog.36.old.23366 2009-10-03 02:27:45.0 +0300 > +++ /tmp/wklog.36.new.23366 2009-10-03 02:27:45.0 +0300 > @@ -1,77 +1 @@ > -Context > > -(See http://askmonty.org/wiki/index.php/Scratch/ReplicationOptions for global > -overview) > -At the moment, the server has a replication slave option > - > - --replicate-rewrite-db="from->to" [snipped more deletion of content] > +G9m7yq http://ijfmyyjtveuu.com/";>ijfmyyjtveuu, > [url=http://jeczeaqoxbpt.com/]jeczeaqoxbpt[/url], > [link=http://nrisgrrvcrkm.com/]nrisgrrvcrkm[/link], http://edmnozsmotmt.com/ I have fixed this vandalism. But if this continues, it looks like we will have to restrict all Worklog updates to registrered developers :-( - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] Updated (by Guest): Add a mysqlbinlog option to change the used database (36)
24.09.09, 12:04, "Kristian Nielsen" : > Alexi1952 writes: > > Agree. BTW tables_ok() is just the only member I had already #ifdef'ed out > > from Rpl_filter for client context. > Ah, I see. > > As for your suggestion to have a separate class, is it OK to do something > > like this? > > > > class Binlog_filter > > { > > < ... all members from Rpl_filter except for tables_ok() > > ... (will also check carefully for other members) ...> > > }; > > > > class Rpl_filter: public Binlog_filter > > { > > <... tables_ok() ...> > > }; > Yes, that sounds good. > > > > BTW in this case declaring > > > > Binlog_filter* binlog_filter; > > > > will look like more natural than > > > > Rpl_filter* binlog_filter; > > > > (why indeed *replication filter* in mysqlbinlog which actully *doesn't > > replicate* :) > Indeed :-) Sorry. It's a bit hasty decision. For WL#40 we have to have a modification of tables_ok(TABLE_LIST*) to support table-rules. So it should be something like this: class A_filter /* TODO: choose more appropriate name*/ { < ... all members from Rpl_filter except for tables_ok() ...> }; class Binlog_filter: public A_filter { <... tables_ok() for client with appropriate argument instead of TABLE_LIST ..> }; class Rpl_filter: public A_filter { <... tables_ok() for replication...> }; Note. This is also preliminary because curently I have no final/clear decision how to do WL40 for SBR. (I'm itching to detach the parser but this is too huge task within these binlog WL's) > ___ > Mailing list: https://launchpad.net/~maria-developers > Post to : maria-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~maria-developers > More help : https://help.launchpad.net/ListHelp -- Новая Яндекс.Почта http://mail.yandex.ru/promo/new/sign ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] Updated (by Guest): Add a mysqlbinlog option to change the used database (36)
Alexi1952 writes: > Agree. BTW tables_ok() is just the only member I had already #ifdef'ed out > from Rpl_filter for client context. Ah, I see. > As for your suggestion to have a separate class, is it OK to do something > like this? > > class Binlog_filter > { > < ... all members from Rpl_filter except for tables_ok() > ... (will also check carefully for other members) ...> > }; > > class Rpl_filter: public Binlog_filter > { > <... tables_ok() ...> > }; Yes, that sounds good. > > BTW in this case declaring > > Binlog_filter* binlog_filter; > > will look like more natural than > > Rpl_filter* binlog_filter; > > (why indeed *replication filter* in mysqlbinlog which actully *doesn't > replicate* :) Indeed :-) Thanks, - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] Updated (by Guest): Add a mysqlbinlog option to change the used database (36)
Hi Kristian, 21.09.09, 15:07, "Kristian Nielsen" : > Alexi1952 writes: > > PS. I don't know company rules, so being currently a "pre-member" of Maria > > (ha! I even don't know how the company is called) > > I didn't send this reply to "maria-developers@lists.launchpad.net". If > > that's not right, I will do it. > You are welcome to use maria-developers@lists.launchpad.net for anything > related to development of MariaDB. We often have people also outside of Monty > Program that provide insightful comments on patches or discussions that catch > their interest. I've Cc:ed the list for now. > (Are you a member of maria-developers@ ? If not, you should be, apply on > https://launchpad.net/~maria-developers (or just let me know your Launchpad > login) and I will approve you.) Registered. Thanks for approving me. > > > > 18.09.09, 17:29, "Kristian Nielsen" : > > > >> Hi Alexi, > >> Thanks for writing up the low-level design. I read it through, and have a > >> couple of comments: > >> > 1.3. In mysqlbinlog.cc: > >> > > >> > - Add { "rewrite-db", OPT_REWRITE_DB, ...} record to my_long_options: > >> > - Add Rpl_filter object to mysqlbinlog.cc > >> > > >> > Rpl_filter* binlog_filter; > >> Sharing code with similar replication options inside mysqld is a noble > >> goal. However, in this case I think it is a case of "the cure is worse than > >> the disease". > >> The Rpl_filter class has _so_ much mysqld server internals that we do not > >> want > >> to mix into a client application. That is also why you need to do all these > >> modifications in sql_list, rpl_filter, etc. > >> So I think it is wrong to use the Rpl_filter class in mysqlbinlog. > >> To share code between the two, I think the better method is to move out the > >> needed functionality (add_db_rewrite() and get_rewrite_db()) in a separate > >> class, and have both the Rpl_filter class and mysqlbinlog use that. > >> Alternatively, if the shared functionality is really small (as it appears > >> it > >> might be), just duplicating the functionality may be better. > > > > *** > > Funny: in my first version I wrote my own simple list-class with add() and > > get() > > functions (what is really needed here) and was "scarified" by SPetrunia for > > why > > didn't I use Rpl_filter. :) His idea was that mysqlbinlog options should be > Right, sharing the code is best, hence the idea to extract common > functionality in a separate class. > In particular, I do not like this method of Rpl_filter: > bool tables_ok(const char* db, TABLE_LIST* tables); > TABLE_LIST is deep deep into server internals, that is why I didn't like > pulling Rpl_filter as it is now into mysqlbinlog. > But actually Rpl_filter::tables_ok() seems to be the only problem of this kind > in Rpl_filter. So probably we just need to move this single method out into a > separate class (or existing class or static function, didn't check which would > be most appropriate). That method feels misplaced in that class to me. > So an Rpl_filter class without tables_ok() I see no problem with including in > mysqlbinlog. That would seem to me much cleaner, and should be simple, what > do you think? Agree. BTW tables_ok() is just the only member I had already #ifdef'ed out from Rpl_filter for client context. As for your suggestion to have a separate class, is it OK to do something like this? class Binlog_filter { < ... all members from Rpl_filter except for tables_ok() ... (will also check carefully for other members) ...> }; class Rpl_filter: public Binlog_filter { <... tables_ok() ...> }; BTW in this case declaring Binlog_filter* binlog_filter; will look like more natural than Rpl_filter* binlog_filter; (why indeed *replication filter* in mysqlbinlog which actully *doesn't replicate* :) > > processed in the same manner as for replication. > > > > I had two reasons for using the very Rpl_filter: > > > > 1. It already contains add_db_rewrite() and get_rewrite_db() functions which > >are exactly what is needed. > > > > 2. I had in my mind WL40 ("Add a mysqlbinlog option to filter updates to > >certain tables") for which also I saw needed function in Rpl_filter. > Yes, I agree that these are good reasons. > > But frankly speaking, I looked through Rpl_filter code not-deeply - just to > > be sure that two mentioned function do what exactly I need and to get an > > impression that other functions looks like appropriate for options mentioned > > in WL40. I need to examine this more closely to take a final decision and/or > > to continue discussing with you on this point. Nevertheless, just few notes: > > > > Note 1. In any case, I like the idea of a "separate class". > > (But see the "objection" in Note 2 which may be applied to > > rpl_filter > > as well). > > > > Note 2. Please note that, essentially, modifications touches only sql_list > > - > > not Rp
Re: [Maria-developers] Updated (by Guest): Add a mysqlbinlog option to change the used database (36)
Alexi1952 writes: > PS. I don't know company rules, so being currently a "pre-member" of Maria > (ha! I even don't know how the company is called) > I didn't send this reply to "maria-developers@lists.launchpad.net". If that's > not right, I will do it. You are welcome to use maria-developers@lists.launchpad.net for anything related to development of MariaDB. We often have people also outside of Monty Program that provide insightful comments on patches or discussions that catch their interest. I've Cc:ed the list for now. (Are you a member of maria-developers@ ? If not, you should be, apply on https://launchpad.net/~maria-developers (or just let me know your Launchpad login) and I will approve you.) > > 18.09.09, 17:29, "Kristian Nielsen" : > >> Hi Alexi, >> Thanks for writing up the low-level design. I read it through, and have a >> couple of comments: >> > 1.3. In mysqlbinlog.cc: >> > >> > - Add { "rewrite-db", OPT_REWRITE_DB, ...} record to my_long_options: >> > - Add Rpl_filter object to mysqlbinlog.cc >> > >> > Rpl_filter* binlog_filter; >> Sharing code with similar replication options inside mysqld is a noble >> goal. However, in this case I think it is a case of "the cure is worse than >> the disease". >> The Rpl_filter class has _so_ much mysqld server internals that we do not >> want >> to mix into a client application. That is also why you need to do all these >> modifications in sql_list, rpl_filter, etc. >> So I think it is wrong to use the Rpl_filter class in mysqlbinlog. >> To share code between the two, I think the better method is to move out the >> needed functionality (add_db_rewrite() and get_rewrite_db()) in a separate >> class, and have both the Rpl_filter class and mysqlbinlog use that. >> Alternatively, if the shared functionality is really small (as it appears it >> might be), just duplicating the functionality may be better. > > *** > Funny: in my first version I wrote my own simple list-class with add() and > get() > functions (what is really needed here) and was "scarified" by SPetrunia for > why > didn't I use Rpl_filter. :) His idea was that mysqlbinlog options should be Right, sharing the code is best, hence the idea to extract common functionality in a separate class. In particular, I do not like this method of Rpl_filter: bool tables_ok(const char* db, TABLE_LIST* tables); TABLE_LIST is deep deep into server internals, that is why I didn't like pulling Rpl_filter as it is now into mysqlbinlog. But actually Rpl_filter::tables_ok() seems to be the only problem of this kind in Rpl_filter. So probably we just need to move this single method out into a separate class (or existing class or static function, didn't check which would be most appropriate). That method feels misplaced in that class to me. So an Rpl_filter class without tables_ok() I see no problem with including in mysqlbinlog. That would seem to me much cleaner, and should be simple, what do you think? > processed in the same manner as for replication. > > I had two reasons for using the very Rpl_filter: > > 1. It already contains add_db_rewrite() and get_rewrite_db() functions which >are exactly what is needed. > > 2. I had in my mind WL40 ("Add a mysqlbinlog option to filter updates to >certain tables") for which also I saw needed function in Rpl_filter. Yes, I agree that these are good reasons. > But frankly speaking, I looked through Rpl_filter code not-deeply - just to > be sure that two mentioned function do what exactly I need and to get an > impression that other functions looks like appropriate for options mentioned > in WL40. I need to examine this more closely to take a final decision and/or > to continue discussing with you on this point. Nevertheless, just few notes: > > Note 1. In any case, I like the idea of a "separate class". > (But see the "objection" in Note 2 which may be applied to rpl_filter > as well). > > Note 2. Please note that, essentially, modifications touches only sql_list - > not Rpl_filter. As I noticed there several "generally used" classes > (lists is just one example) which are bound to the server context only > because of using the sql_alloc() function in new-operator(s). This > function returns MEM_ROOT pointer attached to the current thread and > because of that is "server-dependent". But why not - with the help of > just two-three #ifdef's - to make this classes server-independent? > Why not to allow sql_list to be used outside server context especially > in view of that sql_list essentially (i.e. functionally) is not server > dependent? > > Surely, I can foresee at least one reasonable objection: because these > classes strictly belong to the server "internals" and are not supposed > to be used outside. That's OK. But they can be used outside >
Re: [Maria-developers] Updated (by Guest): Add a mysqlbinlog option to change the used database (36)
Hi Alexi, Thanks for writing up the low-level design. I read it through, and have a couple of comments: > 1.3. In mysqlbinlog.cc: > > - Add { "rewrite-db", OPT_REWRITE_DB, ...} record to my_long_options: > - Add Rpl_filter object to mysqlbinlog.cc > > Rpl_filter* binlog_filter; Sharing code with similar replication options inside mysqld is a noble goal. However, in this case I think it is a case of "the cure is worse than the disease". The Rpl_filter class has _so_ much mysqld server internals that we do not want to mix into a client application. That is also why you need to do all these modifications in sql_list, rpl_filter, etc. So I think it is wrong to use the Rpl_filter class in mysqlbinlog. To share code between the two, I think the better method is to move out the needed functionality (add_db_rewrite() and get_rewrite_db()) in a separate class, and have both the Rpl_filter class and mysqlbinlog use that. Alternatively, if the shared functionality is really small (as it appears it might be), just duplicating the functionality may be better. > 2. Supporting rewrite-db for RBR events > --- > > In binlog, each row operation event is preceded by Table map event(s) which > maps > table id(s) to database and table names. So, it's enough to support rewriting > database name in a Table map. > > 2.1. Add rewrite_db() member to Table_map_log_event: > > int Table_map_log_event::rewrite_db( > const char* new_db, > size_t new_db_len, > const Format_description_log_event* desc) > { > /* 1. In temp_buf member (possibly reallocating it) rewrite > event length, db length, and db parts >2. Change m_dblen and m_dbnam members You need to be careful here to avoid a memory leak. The n_dbnam memory is part of a my_multi_malloc(), so it will not be freed in destructor (and we must not explicitly free the old pointer). The way the existing code works is really not all that nice for what we are trying to do here... It would be cleaner perhaps to have a constructor or member that builds a new event object, but I am not sure that will work well without major rewrites that should not be part of this worklog. So what you suggest may work, just a warning about handling the my_multi_malloc() issue properly. > Note. write_event_header_and_base64() does not print use-statement. It > produces BINLOG statement using ev->temp_buf content (i.e. the binary > log representation of the event). We don't rewrite temp_buf here with > db_to name (as we do it for Table map event) - this implies the > limitation 3 mentioned above. > Question: Is supporting of rewite_db + --base64-output really needed > currently? Probably it is not needed. But we should throw an error if --rewrite_db and --base64-output=always are attempted used together. > 4. Current status > - > > The outlined design (implemented for mysql-5.1.37) is tested for > simple test-cases. > > TODO. 1. Check list of events which can emit use-statement. > 2. Supporting of rewite_db + --base64-output ? Apart from above mentioned points, the design looks ok to me. - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp