List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:January 4 2011 6:24am
Subject:Re: bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597
View as plain text  
Hi Luis,

Thank you for the nice work! I do not have any critical comments, here
are some minor issues and suggestions for you to have a look.

- is_any_column_signaled_for_table should return TRUE
  immediately after found one column is signaled.
- move the call to decide_row_lookup_algorithm to
  do_apply_event() and remove the member
- do_hash_scan_and_update() calls do_post_row_operation(),
  which will call unpack_current_row(), this is redundant,
  I'd suggest move the call to unpack_current_row() from
  do_post_row_operation(), and then add the call to
  unpack_current_row() to do_apply_event() directly.
- The following code is rather confusing:
    if (handle_idempotent_errors(rli, &error) || error)
      goto close_table;
  Why not just:
     if (handle_idempotent_errors(rli, &error))
       goto close_table;
- In the following code block, I think it can read the record
  to record[1] directly, so that it can avoid calling
  store_record() to copy the record to record[1].

     /* get the first record from the table */
     error= table->file->ha_rnd_next(table->record[0]);
     switch (error) {
         store_record(table, record[1]);
- There is no need to do error handling
  (handle_idempotent_errors and do_post_row_operations) when
  filling the hash entries, I'd suggest move the row
  processing loop to do_hash_scan_and_update (and
  do_index/table_scan_*). I think this will make the logic
  more clear that it will first loop over all rows to
  calculate the hash list, and then do one scan table to do
- Change signatures to return the value instead of using out
  parameter, for example change the signature of
    bool Hash_slave_rows::make_hash_key(...);
    my_hash_value_type* Hash_slave_rows::make_hash_key(TABLE*,
  Other functions that I think should be changed:
    bool get(TABLE *table, MY_BITMAP *cols, HASH_ROW_POS_ENTRY** entry);
    bool Hash_slave_rows::next(HASH_ROW_POS_ENTRY** entry);
- Suggest to define two functions to calculate the entry or
  preamble correspondingly.
- enum row_lookup_mode {} I'd suggest make
  ROW_LOOKUP_UNDEFINED as the first value.

Luis Soares wrote:
> #At
> file:///home/lsoares/Workspace/bzr/work/features/wl5597/mysql-next-mr-wl5597-commit/ based
> on revid:alexander.nozdrin@stripped
>  3204 Luis Soares	2010-11-23 [merge]
>       WL#5597: Using batch operations when there is no index in RBR
>       When applying large delete_rows or update_rows log events and the
>       destination table does not have any index, the operation can take
>       a very long time. This is due to the fact that several table
>       scans are actually performed instead of just one.
>       We fix this by adding a new search and update procedure to the
>       slave. Consequently, when a delete_rows or update_rows event is
>       about to be processed and there is no PK or INDEX on the
>       associated table, the slave is able to create a temporary
>       in-memory hash table and store the rows to be updated in
>       it. Then, for each row in the storage engine table, it checks if
>       the row exists in the hash table. If there is a match, it does
>       the operation. This is done in a one time table scan instead of
>       several.

bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597Luis Soares23 Nov
  • Re: bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597He Zhenxing4 Jan
    • Re: bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597Luís Soares11 Jan
      • Re: bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597He Zhenxing11 Jan
        • Re: bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597He Zhenxing11 Jan
  • Re: bzr commit into mysql-next-mr branch (luis.soares:3204) WL#5597Alfranio Correia11 Jan