- developers - lists.mariadb.org

Re: [Maria-developers] Freeing options & wrappers.
by Kristian Nielsen 16 Aug '16

16 Aug '16

Oleksandr Byelkin <sanja(a)montyprogram.com> writes: >>> Making something more I think do not worth it because: >>> 1) in 10.2 C/C will replace the library and it has no the problem >> But Connector-C should have exactly the same problem. If it frees the >> options on failed connect, it will break async API. If it does not, code >> that skips mysql_close() will leak. How does C/C solve this problem? >> If we are making any changes in 10.0, we should preferably make things work >> the same as they will work in 10.2. > Don't it has rewritten async operation? (at least I got such impression). My understanding is that Georg implemented all the async API also in connector-c. It should be using much the same code (mysql_async.c and such). But this is not about the implementation. This is about the semantics of how libmysqlclient works. My understanding is that connector-c is supposed to be binary compatible with libmysqlclient.so? So either connector-c removes any mysql_options() set when mysql_real_connect() fails without CLIENT_REMEMBER_OPTIONS. Or it does not. In the former case, there is the problem with MYSQL_OPT_NONBLOCK. In the latter case, the problem with leak when mysql_close() is not called. I really think you need to understand the problem fully, including how this is going to work in 10.2, before you make any changes to how the client library works... - Kristian.

2 3

Re: [Maria-developers] c56c966: MDEV-10563 Crash during shutdown in Master_info_index::any_slave_sql_running
by Kristian Nielsen 16 Aug '16

16 Aug '16

Vicențiu Ciorbaru <vicentiu(a)mariadb.org> writes: > This is undefined behavior: > // In GCC v6 this function _always_ returns 1. > ClassA::member_function() { > if (!this) > return 0; > return 1; > } Yeah, we shouldn't do this, agree. > make use of this construct. I have not changed the logic of the code, > however I am not sure whether "give_error_if_slave_running()" is supposed > to return TRUE if master_info_index is deleted (aka, we are performing a > shutdown). Probably there is no good reason to return TRUE. Unfortunately, there is a lot of code like this in replication, where it is not clear what the purpose is, but also very hard to be sure it's not needed. My guess would be - without knowing for sure - that the return value doesn't matter. This should only happen during shutdown anyway, and it seems to be used only for changing the values of some system-variables. If server is shutting down, then values of replication-related system-variables do not really matter. As to the patch, you're basically moving the check for NULL into all the callers. I'm not sure if the check is even needed for all callers, but then, I also do not think it's worth it to try and determine where it is needed and where not. But wouldn't it be nicer to instead change the give_error_if_slave_running() to be a normal function that takes the Master_info_index pointer as an argument (or a static method, if you want)? This way, you can keep the null-pointer check in the function. Also helps avoid a new bug if another caller is added. - Kristian.

2 1

[Maria-developers] Freeing options & wrappers.
by Oleksandr Byelkin 16 Aug '16

16 Aug '16

Hi! (sorry I forgot mailing list so resending the e-mail) So it is about https://jira.mariadb.org/browse/MDEV-10559 Original problem was that database drivers in Python & Perl (probably other) do not call mysql_close for MYSQL object if there was no connection happing (memory leak). MySQL has solution for this - freeing option on failed connection if user did not asked something else (we even has the same flag, but it did not work). So I took MySQL solution. But it appeared not so compatible with our async wrappers. Frankly speaking the code is difficult to read and debug because everything made over macros nor I have found decent description/overview of how it is done (thank you if somebody point to it) so I can not say that understand what it is doing, and all my suggestion based on how I understood it. So what we have: we allocate memory for some options set between mysql_init and mysql_real_connect, instead of "reall connect" called some wrapper which store some pointers to the options then call mysql_real_connect and if "real_connect" free them it fail on exit. I think that best solution is that wrapper 1) store current state of CLIENT_REMEMBER_OPTIONS 2) set the flag 3) call mysql_real_connect 4) if connect fail and there was not "remember options" then wrapper free options by itself Again, I am not 100% sure that it is possible nor really know the place were this should be added (if idea above ok, I probably find ho to make it).

2 4

Re: [Maria-developers] [Maria-discuss] Known limitation with TokuDB in Read Free Replication & parallel replication ?
by Kristian Nielsen 16 Aug '16

16 Aug '16

[Moving the discussion to maria-developers@, hope that is ok/makes sense...] Ok, so here is a proof-of-concept patch for this, which seems to make TokuDB work with optimistic parallel replication. The core of the patch is this line in lock_request.cc lock_wait_callback(callback_data, m_txnid, conflicts.get(i)); which ends up doing this: thd_report_wait_for (requesting_thd, blocking_thd); All the rest of the patch is just getting the right information around between the different parts of the code. I put this on top of Jocelyn Fournier's tokudb_rpl.rpl_parallel_optimistic patches, and pushed it on my github: https://github.com/knielsen/server/tree/toku_opr2 With this patch, the test case passes! So that's promising. Some things still left to do for this to be a good patch: - I think the callback needs to trigger also for an already waiting transaction, in case another transaction arrives later to contend for the same lock, but happens to get the lock earlier. I can look into this. - This patch needs linear time (in number of active transactions) per callback to find the THD from the TXNID, maybe that could be optimised. - Probably the new callback etc. needs some cleanup to better match TokuDB code organisation and style. - And testing, of course. I'll definitely need some help there, as I'm not familiar with how to run TokuDB efficiently. Any thoughts or comments? - Kristian.

2 8

Re: [Maria-developers] Status of InnoDB 5.7 merge to 10.2
by Kristian Nielsen 13 Aug '16

13 Aug '16

Jan Lindström <jan.lindstrom(a)mariadb.com> writes: > I have reached to situation where I need help fixing remaining test > failures and mentoring is current status good enough for 10.2 Beta. I need > help on test failures on parts where I do not follow why test is failing or > server crashes I do not know how to fix. > > First of all current merge result is on bb-10.2-jan with > commit ca4f194460da469e29624d930e6d0027349d047c > > Higher level of status is on https://jira.mariadb.org/browse/MDEV-10024 > > Note that mtr thinks currently that innodb_plugin compiled using static > linking is xtradb (but not have_xtradb as xtradb is not compiled). > Furthermore galera is not compiled (waiting for codership). > > Here are current issues where I need help and possible persons who could > help: > > (1) Parallel replication lock wait/deadlock handling on InnoDB 5.7 with > refactored lock0lock.cc does not work > https://jira.mariadb.org/browse/MDEV-10550 (Monty, Kristian) >From a quick look at the patch, this seems to be because of code removed from innobase_kill_query(). Specifically: > @@ -4799,65 +5511,11 @@ innobase_kill_query( > DBUG_ENTER("innobase_kill_query"); > DBUG_ASSERT(hton == innodb_hton_ptr); > - if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE) && > - trx->abort_type == TRX_SERVER_ABORT) { > - ut_ad(!lock_mutex_own()); > - lock_mutex_enter(); When parallel replication discovers a case where a transaction T1 goes to wait on a row lock held by a later (in the replication stream) transaction T2, T2 will be killed to avoid a deadlock. In this situation, we are already holding lock mutex. So this code is there in 10.1 to prevent re-taking the mutex, code that is removed here in your patch. > + if (trx != NULL) { > + /* Cancel a pending lock request if there are any */ > + lock_trx_handle_wait(trx); > +lock_trx_handle_wait( > +/*=================*/ > + trx_t* trx) /*!< in/out: trx lock state */ > { > dberr_t err; > - ulint heap_no; > > - ut_ad(!dict_index_is_clust(index)); > - ut_ad(!dict_index_is_online_ddl(index)); > - ut_ad(block->frame == page_align(rec)); > - ut_ad(page_rec_is_user_rec(rec) || page_rec_is_supremum(rec)); > - ut_ad(rec_offs_validate(rec, index, offsets)); > - ut_ad(mode == LOCK_X || mode == LOCK_S); > + lock_mutex_enter(); But in the new code here, the protection on lock_mutex_enter is no longer there, lock_mutex_enter() is done unconditionally. The code which sets the flag TRX_SERVER_ABORT seems to be still there, though: > +void > +lock_report_waiters_to_mysql( > +/*=======================*/ > + struct thd_wait_reports* waitee_buf_ptr, /*!< in: set of trxs */ > + THD* mysql_thd, /*!< in: THD */ > + trx_id_t victim_trx_id) /*!< in: Trx selected > + as deadlock victim, if > + any */ > +{ > + struct thd_wait_reports* p; > + struct thd_wait_reports* q; > + ulint i; > + > + p = waitee_buf_ptr; > + while (p) { > + i = 0; > + while (i < p->used) { > + trx_t *w_trx = p->waitees[i]; > + /* There is no need to report waits to a trx already > + selected as a victim. */ > + if (w_trx->id != victim_trx_id) { > + /* If thd_report_wait_for() decides to kill the > + transaction, then we will get a call back into > + innobase_kill_query. We mark this by setting > + current_lock_mutex_owner, so we can avoid trying > + to recursively take lock_sys->mutex. */ > + w_trx->abort_type = TRX_REPLICATION_ABORT; > + thd_report_wait_for(mysql_thd, w_trx->mysql_thd); > + w_trx->abort_type = TRX_SERVER_ABORT; abort_type is set to TRX_REPLICATION_ABORT around thd_report_wait_for(), during which innobase_kill_query() may be invoked with lock mutex already held. So it would appear that a first attempt should be to restore the protection around lock_mutex_enter()... Note that, if w_trx->abort_type is not functional in your current patch, the parallel replication code was originally working without this, you can see in the snippet above the comment about current_lock_mutex_owner, which was (mistakenly) not removed when this code was changed to use Galera stuff. So you can check git history and go back to the original code (using current_lock_mutex_owner) if necessary, perhaps. Or maybe w_trx->abort_type is fine, I'm not familiar with that change. > (2) mysqld: /home/jan/mysql/10.2/sql/handler.cc:2692: int > handler::ha_index_first(uchar*): Assertion `table_share->tmp_table != > NO_TMP_TABLE || m_lock_type != 2' failed. > https://jira.mariadb.org/browse/MDEV-10549 (Serg) > > (3) Some of the debug sync waits do not work InnoDB 5.7 > https://jira.mariadb.org/browse/MDEV-10548 (Monty, Serg) > > (4) Test multi_update_innodb fails with InnoDB 5.7 > https://jira.mariadb.org/browse/MDEV-10550 (Petrunia) > > (5) Test innodb.defrag_mdl-9155 hangs on InnoDB 5.7 > https://jira.mariadb.org/browse/MDEV-10551 (Jan, Serg) > > (6) IS tables are not found on 10.2 InnoDB 5.7 > https://jira.mariadb.org/browse/MDEV-10200 (Jan) Hope this helps, - Kristian.

2 1

[Maria-developers] MariaDB Developers Meetup
by Ian Gilfillan 10 Aug '16

10 Aug '16

The MariaDB Developers Meetup will be taking place from the 6th to the 8th of October in Amsterdam. https://mariadb.org/2016-mariadb-developers-meetup/ Details of where to sign up, and where to suggest topics for the meetup are on that page. Please feel free to suggest topic you're interested in presenting, as well as topics you'd like to see covered. The meetup is scheduled to be just after Percona Live, which also takes place in Amsterdam from the 3rd to the 5th. The Foundation has arranged a discount so that anyone signing up for the MariaDB Developers Meetup and that also wants to attend Percona Live will receive a discount for that event (the Developers Meetup has no charge). Percona Live are also looking for talks or sessions, so if you have a topic that would be of interest to the broader community, feel free to submit it at https://www.percona.com/live/plam16/program

1 1

[Maria-developers] Please review MDEV-10468 Assertion `nr >= 0.0' failed in Item_sum_std::val_real()
by Alexander Barkov 08 Aug '16

08 Aug '16

Hello Alexey, Can you please review a patch for MDEV-10468? Thanks!

2 3

[Maria-developers] Please review MDEV-10500 CASE/IF Statement returns multiple values and shifts further result values to the next column
by Alexander Barkov 08 Aug '16

08 Aug '16

Hi Serg, Please review a patch for MDEV-10500. Thanks.

2 2

Re: [Maria-developers] 634e130: MDEV-10424 - Assertion `ticket == __null' failed in MDL_request::set_type
by Sergei Golubchik 08 Aug '16

08 Aug '16

Hi, Sergey! On Aug 04, Sergey Vojtovich wrote: > revision-id: 634e130b00bc900d1b7b8a8df6c13b1701f84b30 (mariadb-5.5.50-24-g634e130) > parent(s): 2f55f0b010619a09870ba704b55dd1426a158f1d > committer: Sergey Vojtovich > timestamp: 2016-08-04 15:51:17 +0400 > message: > > MDEV-10424 - Assertion `ticket == __null' failed in MDL_request::set_type > > Reexecution of prepared "ANALYZE TABLE merge_table, table" may miss to > reinitialize "table" for subsequent execution and trigger assertion failure. > > This happens because MERGE engine may adjust table->next_global chain, which > gets cleared by close_thread_tables()/ha_myisammrg::detach_children() later. > Since reinitilization iterates next_global chain, it won't see tables following > merge table. > > Fixed by appending saved next_global chain after merge children. looks ok to me, but let elena do some extra testing of this, please Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] GSoC weekly reports (Unique indexes for blobs)
by Shubham Barai 03 Aug '16

03 Aug '16

GSoC (week 5) Hello everyone, Here is the list of things I have done in the 5th week of GSoC. 1.implemented unique key violation with a hash collision. (actual row comparison is remaining ). 2.modified hash function for data types like varchar and binary data types. 3.fixed a bug which was causing a server to crash for complex unique keys. 4.added support to allow any number of nulls which will not cause any unique key violation. 5.added test cases for above features. On 22 June 2016 at 16:36, Shubham Barai <shubhambaraiss(a)gmail.com> wrote: > Hi, > > can we discuss on IRC first? > > > Regards, > Shubham > > On 22 June 2016 at 13:21, Jan Lindström <jan.lindstrom(a)mariadb.com> wrote: > >> Hi, >> >> Please commit and push these changes to your git branch I have not yet >> seen them, in my opinion as this is only a working branch you can push >> often. I still fail to see any test cases on InnoDB branch, do you have >> more than one branch and if you have why ? Depending on extent of these >> changes my estimate is that you are behind schedule to complete project in >> time. Based on your progress report you are still missing update and delete >> and redo-logging. For alter table you should start from forcing copy-method >> and then if time permits develop on-line method. This naturally only after >> everything else >> has been completed and tested. >> >> R: Jan >> >> On Tue, Jun 21, 2016 at 11:06 PM, Shubham Barai <shubhambaraiss(a)gmail.com >> > wrote: >> >>> GSoC (week 4) >>> >>> Hello everyone, >>> >>> After working on create table operation,next thing I had to work on was >>> insert operations.So I explored some of the functions like >>> row_ins_scan_index_for_duplicates, btr_pcur_get_rec to get clear >>> understanding about how to implement duplicate search on hash index. >>> There was a problem in hash function that I wrote .It would calculate >>> same hash value for two different keys if the prefix length of blob key >>> part was zero. Now it seems to be working after I checked it in debugger.I >>> still have to modify it for data types like varchar etc. >>> I have added test cases for insert operations in myisam. >>> In MyIsam, I found one problem in update operation. When updating a >>> row,if the key is conflicting then server crashes because some pointer goes >>> invalid in compare_record. I haven't fixed this issue yet. >>> >>> I also modified some functions in dict0load.cc to adjust some members >>> of dict_index_t for a new index type.The main problem is that index entry >>> for hash based index cointains only two fields(hash value and row id) while >>> dict_index_t contains hash field and other user defined fields which are >>> used to calculate hash value.Some of the operations like alter table( e.g. >>> rename column) needs to get access to all fields while other functions like >>> rec_get_offsets and row_build_index_entry_low needs to get access to only >>> hash field and row id. I am still working on this to find efficient >>> solution to this problem. >>> >>> On 16 June 2016 at 23:29, Sergei Golubchik <vuvova(a)gmail.com> wrote: >>> >>>> Hi, Shubham! >>>> >>>> What I wanted to say on IRC was: >>>> >>>> here's what the comment of cmp_dtuple_rec_with_match_low() says: >>>> >>>> ............... If rec has an externally stored field we do not >>>> compare it but return with value 0 if such a comparison should be >>>> made. >>>> >>>> Note that blobs are externally stored fields in InnoDB, so, I think, >>>> this means that you cannot use cmp_dtuple_rec() to compare blobs. >>>> >>>> Regards, >>>> Sergei >>>> Chief Architect MariaDB >>>> and security(a)mariadb.org >>>> >>> >>> >>> _______________________________________________ >>> Mailing list: https://launchpad.net/~maria-developers >>> Post to : maria-developers(a)lists.launchpad.net >>> Unsubscribe : https://launchpad.net/~maria-developers >>> More help : https://help.launchpad.net/ListHelp >>> >>> >> >

1 5