[Maria-developers] The Monty Program AB vendor check (rpm)
Hello, I got assigned to look at the rpm packaging and fixed some stuff there, but got stuck at the Monty Program AB vendor check in support-files/rpm/server- prein.sh when I wanted to replace the fedora packages with in this case mariadb galera packages. Is this check absolutely nessesary? This prevents you from an easy switch from the fedora mariadb packages to the ones provided by mariadb using yum and a yum repository as provided at http://yum.mariadb.org/. Basically this check prevents me from doing any serious work with the rpm packages because you manually have to, by force deinstall any conflicting packages and then install the packages provided by us. Cheers. ___ 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] Build fail of MariaDB10 with TokuDB enabled
Hi, Leif! On Dec 02, Leif Walsh wrote: Yep, CMAKE_SKIP_BUILD_RPATH=0 would cause this exact problem. We should add something to our cmake to warn if that is set to 0. I'd suggest this change: --- storage/tokudb/ft-index/ft/CMakeLists.txt 2013-11-19 14:35:31 + +++ storage/tokudb/ft-index/ft/CMakeLists.txt 2013-12-03 09:04:29 + @@ -11 +11 @@ -target_link_libraries(logformat ${LIBTOKUPORTABILITY}) +target_link_libraries(logformat ${LIBTOKUPORTABILITY}_static) logformat is used internally during the build, it's not going to be installed. Thus it can use static libraries and avoid the whole rpath issue altogether. This should probably be done for all binaries that are only used during the build. Regards, Sergei ___ 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
[Maria-developers] review of mdev-5095 (executing triggers on slave)
Hi! Here is the review of mdev-5095 I got the diff from: bzr diff -r3942.. === modified file 'mysql-test/r/mysqld--help.result' --- mysql-test/r/mysqld--help.result 2012-10-18 21:33:06 + +++ mysql-test/r/mysqld--help.result 2013-12-03 10:53:01 + @@ -747,6 +747,14 @@ --slave-net-timeout=# Number of seconds to wait for more data from a master/slave connection before aborting the read + --slave-run-triggers-for-rbr=name + Modes for how triggers in row-base replication on slave + side will be executed. Legal values are NO (default), YES + and LOGGING. NO means that trigger for RBR will not be + running on slave YES and LOGGING means that triggers will + be running on slave (if there was not triggers running on + the naster), LOGGING also mens that flag about executed + triggers will be written to binlog. Suggested new text: --slave-run-triggers-for-rbr=name Modes for how triggers in row-base replication on slave side will be executed. Legal values are NO (default), YES and LOGGING. NO means that trigger for RBR will not be running on slave. YES and LOGGING means that triggers will be running on slave, if there was not triggers running on the master for the statement. LOGGING also means that the executed triggers will be written to binlog. cut === modified file 'sql/log_event.cc' cut -#if !defined(MYSQL_CLIENT) defined(HAVE_REPLICATION) +#if defined(MYSQL_SERVER) defined(HAVE_REPLICATION) Was the above needed or mostly an optimization? + +static void restore_empty_query_table_list(LEX *lex) +{ + if (lex-first_not_own_table()) + (*lex-first_not_own_table()-prev_global)= NULL; + lex-query_tables= NULL; + lex-query_tables_last= lex-query_tables; +} Add a comment of what is the purpose of the above function. @@ -8340,6 +8351,22 @@ int Rows_log_event::do_apply_event(Relay /* A small test to verify that objects have consistent types */ DBUG_ASSERT(sizeof(thd-variables.option_bits) == sizeof(OPTION_RELAXED_UNIQUE_CHECKS)); +if (slave_run_triggers_for_rbr) +{ + LEX *lex= thd-lex; + uint8 new_trg_event_map= get_trg_event_map(); + + DBUG_ASSERT(lex-query_tables == NULL); + if ((lex-query_tables= rli-tables_to_lock)) +rli-tables_to_lock-prev_global= lex-query_tables; Add a comment why we have to set prev_global (not obvious). It's also not obvious why restore_empty_tables() is not resetting rli-tables_to_lock-prev_global as it's reseting the the rest of the changed things. @@ -8429,8 +8462,11 @@ int Rows_log_event::do_apply_event(Relay */ TABLE_LIST *ptr= rli-tables_to_lock; for (uint i=0 ; ptr (i rli-tables_to_lock_count); ptr= ptr-next_global, i++) +{ const_castRelay_log_info*(rli)-m_table_map.set_table(ptr-table_id, ptr-table); - What does this test really do ? (I was quickly checking the code to understand what m_table_id stands for, but it was not obvious). + if (m_table_id == ptr-table_id) +master_had_triggers= ((RPL_TABLE_LIST*)ptr)-master_had_triggers; cut if (table) { bool transactional_table= table-file-has_transactions(); @@ -8618,6 +8656,9 @@ int Rows_log_event::do_apply_event(Relay */ thd-reset_current_stmt_binlog_format_row(); thd-is_slave_error= 1; +/* remove trigger's tables */ +if (slave_run_triggers_for_rbr) + restore_empty_query_table_list(thd-lex); DBUG_RETURN(error); } You do the above 'restore_empty_query_table_list(thd-lex);' before every return in this function, except the last return. - Isn't it better to do a 'error:' tag at the end of the function to ensure we don't miss any cases ? - Add a comment why we don't need to do this at the very end of Rows_log_event::do_apply_event(). Something like: /* We don't have to call restore_empty_query_table_list() here as it's called in rows_event_stmt_cleanup(). */ When is lex-query_tables used? Is it only used by 'open' ? If yes, isn't it enough to always reset it after the 'open' call? @@ -9773,6 +9831,29 @@ Write_rows_log_event::do_before_row_oper from the start. */ } + if (slave_run_triggers_for_rbr !master_had_triggers m_table-triggers ) + { +if (m_table-triggers-has_triggers(TRG_EVENT_DELETE, +TRG_ACTION_AFTER)) +{ + /* +The table has AFTER DELETE triggers that might access to +subject table and therefore might need delete to be done +immediately. So we turn-off the batching. + */ + (void) m_table-file-extra(HA_EXTRA_DELETE_CANNOT_BATCH); +} +if (m_table-triggers-has_triggers(TRG_EVENT_UPDATE, +TRG_ACTION_AFTER)) +{ + /* +The table has AFTER UPDATE triggers that might access to subject +table and therefore might need update to be done immediately. +So we turn-off
Re: [Maria-developers] Build fail of MariaDB10 with TokuDB enabled
That's a good solution, but I'd still be wary of messing around too much with cmake's RPATH handling. I'll do this in ft-index soon though. Sent from my iPhone On Dec 3, 2013, at 4:09, Sergei Golubchik s...@mariadb.org wrote: Hi, Leif! On Dec 02, Leif Walsh wrote: Yep, CMAKE_SKIP_BUILD_RPATH=0 would cause this exact problem. We should add something to our cmake to warn if that is set to 0. I'd suggest this change: --- storage/tokudb/ft-index/ft/CMakeLists.txt 2013-11-19 14:35:31 + +++ storage/tokudb/ft-index/ft/CMakeLists.txt 2013-12-03 09:04:29 + @@ -11 +11 @@ -target_link_libraries(logformat ${LIBTOKUPORTABILITY}) +target_link_libraries(logformat ${LIBTOKUPORTABILITY}_static) logformat is used internally during the build, it's not going to be installed. Thus it can use static libraries and avoid the whole rpath issue altogether. This should probably be done for all binaries that are only used during the build. Regards, Sergei ___ 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] MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES and INSTALL PLUGIN
Hi Sergei, thanks for your feedback. Comments/questions inline. On Fri, Nov 29, 2013 at 01:57:13PM +0100, Sergei Golubchik wrote: Hi, Sergey! On Nov 27, Sergey Vojtovich wrote: Hi Sergei, I'm afraid I couldn't find easy fix for this deadlock. The question is if it is worth to spend time for more complex (and possibly less stable) solution. Details below. Locking profile --- INSTALL PLUGIN: LOCK_plugin- LOCK_system_variables_hash (w) SHOW VARIABLES: LOCK_system_variables_hash (r) - LOCK_global_system_variables change_user(): LOCK_global_system_variables - LOCK_plugin INSTALL PLUGIN -- Needs LOCK_plugin to register new plugin. Needs LOCK_system_variables_hash to register plugin variables. Doesn't seem to need both locks at the same time. Similar lock order is used in a few places. SHOW VARIABLES -- Needs LOCK_system_variables_hash to iterate system_variable_hash. Needs LOCK_global_system_variables to read variable value. Does seem to need both locks at the same time. This lock order is not used in other places. change_user() - Needs LOCK_global_system_variables to read global_system_variable.table_plugin. Needs LOCK_plugin to my_plugin_lock(table_plugin). Does seem to need both locks at the same time. Similar lock order is used in a few places. Observations: THD::init() starts from locking LOCK_global_system_variables, then invokes plugin_thdvar_init(), that invokes cleanup_variables(), that read-locks LOCK_system_variables_hash. This is directly against SHOW VARIABLES order. Suggestion: swap the order of cleanup_variables() and LOCK_global_system_variables. Right and I plan to do something about it later, when MDEV-5345 is fixed. Anyway currently there should be no deadlock here, since it doesn't conflict with write-lock order. SHOW VARIABLES looks correct. So I have only two ideas: - Fix INSTALL PLUGIN so it releases LOCK_plugin before registering variables. Sounds like the best solution, but there are a few more things to fix, e.g. UNINTALL PLUGIN. Is it possible? On a quick look I wasn't able to answer that :( It should be quite easy: for INSTALL PLUGIN we just move test_plugin_options() to plugin_initialize() while plugin state is PLUGIN_IS_UNINITIALIZED. Same for UNINSTALL PLUGIN. Did I miss something important? - Store default storage engine as a string, not Sys_var_plugin. That'd mean, that neither global nor local @@default_storage_engine setting locks the plugin. We'll need to do a hash lookup and lock the plugin for every statement that uses the engine. Which may be or may not be a problem. Right, I can't say if it may or may not be a problem yet. Here's yet another idea: don't protect plugin-ref_cnt and plugin-state with LOCK_plugin. Then intern_plugin_lock and intern_plugin_unlock could be completely lock-free. One would need to increment ref_cnt before or at the same time as checking the state. It *seems* that the rest of the code could support that with very few changes. But it'd need more analysys. Well, lock-free ref_count is not a problem. Lock-free state is challenging, but looks doable. What about reap_plugins() which is called by plugin_unlock()? Thanks, Sergey ___ 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] review of mdev-5095 (executing triggers on slave)
Hi! 03.12.2013 14:09, Michael Widenius пишет: Hi! Here is the review of mdev-5095 I got the diff from: bzr diff -r3942.. [skip] === modified file 'sql/log_event.cc' cut -#if !defined(MYSQL_CLIENT) defined(HAVE_REPLICATION) +#if defined(MYSQL_SERVER) defined(HAVE_REPLICATION) Was the above needed or mostly an optimization? Actually above made to make headers and bodies of methods be controlled by the same condition, because it is strange that header has one condition and body different (now it means the same bt who know what it will be in the future). [skip] @@ -8429,8 +8462,11 @@ int Rows_log_event::do_apply_event(Relay */ TABLE_LIST *ptr= rli-tables_to_lock; for (uint i=0 ; ptr (i rli-tables_to_lock_count); ptr= ptr-next_global, i++) +{ const_castRelay_log_info*(rli)-m_table_map.set_table(ptr-table_id, ptr-table); - What does this test really do ? (I was quickly checking the code to understand what m_table_id stands for, but it was not obvious). Following is passing flag about triggers on the server. The problem was to pass it between table map event and row event. I do it via extended TABLE_LIST (RPL_TABLE_LIST). but row event uses only TABLE so I need to find somehow the corresponding TABLE_LIST. + if (m_table_id == ptr-table_id) +master_had_triggers= ((RPL_TABLE_LIST*)ptr)-master_had_triggers; cut if (table) { bool transactional_table= table-file-has_transactions(); @@ -8618,6 +8656,9 @@ int Rows_log_event::do_apply_event(Relay */ thd-reset_current_stmt_binlog_format_row(); thd-is_slave_error= 1; +/* remove trigger's tables */ +if (slave_run_triggers_for_rbr) + restore_empty_query_table_list(thd-lex); DBUG_RETURN(error); } You do the above 'restore_empty_query_table_list(thd-lex);' before every return in this function, except the last return. it is done before very last return but not so close to it. - Isn't it better to do a 'error:' tag at the end of the function to ensure we don't miss any cases ? - Add a comment why we don't need to do this at the very end of Rows_log_event::do_apply_event(). Something like: /* We don't have to call restore_empty_query_table_list() here as it's called in rows_event_stmt_cleanup(). */ When is lex-query_tables used? the problem was that trigger procedures works only with lex-query_tables (and it was one of the cause of skipping triggers in row log events. Is it only used by 'open' ? It is where triggers processed (openng and locking tables). If yes, isn't it enough to always reset it after the 'open' call? We have a lot of ASSERTS that check the lists when we close tables. they have the same problem with MERGE MYISAM tables and went this way I only trying to do more or less the same. @@ -9773,6 +9831,29 @@ Write_rows_log_event::do_before_row_oper from the start. */ } + if (slave_run_triggers_for_rbr !master_had_triggers m_table-triggers ) + { +if (m_table-triggers-has_triggers(TRG_EVENT_DELETE, +TRG_ACTION_AFTER)) +{ + /* +The table has AFTER DELETE triggers that might access to +subject table and therefore might need delete to be done +immediately. So we turn-off the batching. + */ + (void) m_table-file-extra(HA_EXTRA_DELETE_CANNOT_BATCH); +} +if (m_table-triggers-has_triggers(TRG_EVENT_UPDATE, +TRG_ACTION_AFTER)) +{ + /* +The table has AFTER UPDATE triggers that might access to subject +table and therefore might need update to be done immediately. +So we turn-off the batching. + */ + (void) m_table-file-extra(HA_EXTRA_UPDATE_CANNOT_BATCH); +} + } Good that you noticed and fixed the above. However, why not just call the function prepare_triggers_for_insert_stmt() that does exactly the same thing? it also has table-mark_columns_needed_for_insert(); Raw log event mark all columns in any case also checks some correspondence between real columns on master slave so I prefer do not interfere here. cut @@ -10749,6 +10894,17 @@ Delete_rows_log_event::do_before_row_ope */ return 0; } + if (slave_run_triggers_for_rbr !master_had_triggers m_table-triggers + m_table-triggers-has_triggers(TRG_EVENT_DELETE, +TRG_ACTION_AFTER)) + { +/* + The table has AFTER DELETE triggers that might access to subject table + and therefore might need delete to be done immediately. So we turn-off + the batching. +*/ +(void) m_table-file-extra(HA_EXTRA_DELETE_CANNOT_BATCH); + } Would be better if we could do a function 'prepare_triggers_for_delete_stmt()' and use the same code here and in sql_delete.cc maybe, but as I it will be temptation to put something more to the function without noticing that it is called for events