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
m_rows_lookup_algorithm.
- 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
updates/deletes.
- Change signatures to return the value instead of using out
parameter, for example change the signature of
bool Hash_slave_rows::make_hash_key(...);
to
my_hash_value_type* Hash_slave_rows::make_hash_key(TABLE*,
MY_BITMAP*);
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.
HASH_ROW_POS_PREAMBLE* get_preamble(HASH_ROW_POS_ENTRY*);
HASH_ROW_POS_ENTRY* get_entry(HASH_ROW_POS_PREAMBLE*);
- 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.
