- developers - lists.mariadb.org

[Maria-developers] Questions on recent window function code changes
by Sergey Petrunia 14 Sep '16

14 Sep '16

Hi Vicențiu, > item_windowfunc.cc|168| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list); > item_windowfunc.cc|176| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list); > item_windowfunc.cc|218| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list); Coding style violation: Use peer_tracker=. > sql_window.cc: > // TODO check if this can be placed outside the loop. > err= tbl->file->ha_update_row(tbl->record[1], tbl->record[0]); Yes, it should be. Can you try it? (and if it doesn't work that's surprising and we could discuss why..) > grep fech_prev_row *.h *.cc > sql_window.cc|633| bool fetch_prev_row() This function is not used anymore? > static bool is_computed_with_remove(Item_sum::Sumfunctype sum_func) What is the difference between this function and Item_sum::supports_removal ? Does one imply/require the other? > sql_window.h > /* > This handles computation of one window function. > > Currently, we make a spearate filesort() call for each window function. > */ > > class Window_func_runner : public Sql_alloc Both statements in the comment are not true anymore, right? Can you update the comment? > sql_window.cc > /* > Regular frame cursors add or remove values from the sum functions they > manage. By calling this method, they will only perform the required > movement within the table, but no adding/removing will happen. > */ > void set_no_action() > { > perform_no_action= true; > } Is there any difference between perform_no_action=true and Frame_cursor::sum_functions being an empty list? Please clarify. > sql_window.cc > /* > A class that owns cursor objects associated with a specific window function. > */ > class Cursor_manager > But as I understand, your recent changes make window functions share the frame cursors. That is, window frame has its cursors, and all window functions that are sharing the frame, share the cursors. Is Cursor_manager still needed? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] MariaDB update deficiency
by Sachin Setia 14 Sep '16

14 Sep '16

Hello Everyone! This thread is about solving this issue https://jira.mariadb.org/browse/MDEV-10220. I made a google doc https://docs.google.com/document/d/1sor3nVGz2v9Jm4-MDDFfMSn7d7xOip7Qio_T0Aq… In which I am explaining my idea , which i think should solve this problem. Please have a look and tell me what you think ? Regards sachin

1 0

Re: [Maria-developers] [Commits] 49b2502: Fix assertion/hang in read_init_file()
by Kristian Nielsen 12 Sep '16

12 Sep '16

Monty, My latest push uncovered a problem in the 10.2 tree; however the problem was there before, just not triggered by the testsuite. I pushed the below patch to fix the test failures in Buildbot, but I am not sure it is the correct solution, so please check it. It appears the problem was introduced with this patch: commit 3d4a7390c1a94ef6e07b04b52ea94a95878cda1b Author: Monty <monty(a)mariadb.org> Date: Mon Feb 1 12:45:39 2016 +0200 MDEV-6150 Speed up connection speed by moving creation of THD to new thread diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 84f0c63..355f62d 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -844,12 +844,13 @@ void do_handle_bootstrap(THD *thd) delete thd; #ifndef EMBEDDED_LIBRARY - thread_safe_decrement32(&thread_count); + DBUG_ASSERT(thread_count == 1); in_bootstrap= FALSE; - - mysql_mutex_lock(&LOCK_thread_count); - mysql_cond_broadcast(&COND_thread_count); - mysql_mutex_unlock(&LOCK_thread_count); + /* + dec_thread_count will signal bootstrap() function that we have ended as + thread_count will become 0. + */ + dec_thread_count(); my_thread_end(); pthread_exit(0); #endif The problem is that do_handle_bootstrap is not only used for bootstrap - it is also used for read_init_file(). In the latter case, it is _not_ guaranteed that thread_count will drop to zero. For example, the binlog background thread might be running. You can see this in 10.2 (before my just-pushed fix) by running ./mtr main.init_file --mysqld=--log-bin=mysql-bin It will assert, or hang if assert is disabled. To get rid of the failures in Buildbot, I pushed the below patch. I think the patch is perfectly safe. However, maybe there is a better way - it seems a bit messy, mixing bootstrap and read_init_file() and having these if (opt_bootstrap) conditions. And maybe there are other problems caused by this as well that I did not notice? As I am not much familiar with how this thread count checking / bootstrap / init file handling works. Thanks, - Kristian. Kristian Nielsen <knielsen(a)knielsen-hq.org> writes: > revision-id: 49b25020ef512866751f192b91d8439670d0430b (mariadb-10.1.8-257-g49b2502) > parent(s): be2b833c426b420073c50564125049e2b4a95e8b > committer: Kristian Nielsen > timestamp: 2016-09-09 18:09:59 +0200 > message: > > Fix assertion/hang in read_init_file() > > If there are other threads running (for example binlog background > thread), then the thread count may not drop to zero at the end of > do_handle_bootstrap(). This caused an assertion and missing wakeup of > the main thread. The missing wakeup is because THD::~THD() only > signals the COND_thread_count mutex when the number of threads drops > to zero. > > Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> > > --- > sql/sql_parse.cc | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 7263082..6baff31 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -1082,9 +1082,20 @@ void do_handle_bootstrap(THD *thd) > end: > in_bootstrap= FALSE; > delete thd; > + if (!opt_bootstrap) > + { > + /* > + We need to wake up main thread in case of read_init_file(). > + This is not done by THD::~THD() when there are other threads running > + (binlog background thread, for example). So do it here again. > + */ > + mysql_mutex_lock(&LOCK_thread_count); > + mysql_cond_broadcast(&COND_thread_count); > + mysql_mutex_unlock(&LOCK_thread_count); > + } > > #ifndef EMBEDDED_LIBRARY > - DBUG_ASSERT(thread_count == 0); > + DBUG_ASSERT(!opt_bootstrap || thread_count == 0); > my_thread_end(); > pthread_exit(0); > #endif > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

2 3

Re: [Maria-developers] Sachin weekly report
by Sachin Setia 10 Sep '16

10 Sep '16

Hi Sergei, Weekly Report for 4th week of gsoc 1. Field property is_row_hash , field_visibility successfully saved and retrived from frm , using extra2 space 2. Some tests added. 3. Solved the error when there is another primary key(it used to accept duplicate in this case ). 4. Added hidden in parser. 5. Identified the memory leak 1 is because of malloc db_row_hash str.I did not freed it. second memory leak i am searching for it. Work for this week. 1 First solve the memory leak problem. 2 Work on FULL_HIDDEN_FIELDS. 3 in mysql_prepare_create_table I am using an iterator it would be better if i can add custom field when it says an error. So will not have to use iterator as suggested by you sir. 4 rename the hash field automatically in the case clash. This week On Thu, Jun 16, 2016 at 11:46 PM, Sergei Golubchik <serg(a)mariadb.org> wrote: > Hi, Sachin! > > On Jun 15, Sachin Setia wrote: > > > > But the major problem is:- > > Consider this case > > > > create table tbl(abc int primary key,xyz blob unique); > > > > In this case , second key_info will have one user_defined_key_parts but > two > > ext_key_parts > > second key_part refers to primary key. > > because of this ha_index_read_idx_map always return HA_ERR_KEY_NOT_FOUND > > I am trying to solve this problem. > > I've seen you solved this, but I do not understand the problem (and so I > cannot understand the fix either). Problem was consider this create table tbl(abc int primary key , xyz blob unique); insert into tbl value(1,12); insert into tbl value(2,12); # no error , details in commit comment https://github.com/MariaDB/server/commit/baecc73380084c61b9323a30f3e2597717… > Please, try to add a test case for > the problem you're fixing. In the same commit, preferrably. > Now you can still commit a test case for this problem and your fix, > then, I hope, I'll be able to understand better what the problem was. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org >

4 29

Re: [Maria-developers] [Commits] d6f760d: MDEV-10296 - Multi-instance table cache
by Sergei Golubchik 09 Sep '16

09 Sep '16

Hi, Sergey! On Sep 08, Sergey Vojtovich wrote: > revision-id: d6f760d344f45177936c72cb65ba1ffff596710e (mariadb-10.1.8-230-gd6f760d) > parent(s): 9f7e77b5414b5b2ef9b5f50f131966821fefb9c7 > committer: Sergey Vojtovich > timestamp: 2016-09-08 16:02:49 +0400 > message: > > MDEV-10296 - Multi-instance table cache > > Table cache instances autosizing. Looks fairly simple, good :) Code-wise it's ok, I've added a comment in Jira. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 9f7e77b: MDEV-10296 - Multi-instance table cache
by Sergei Golubchik 09 Sep '16

09 Sep '16

Hi, Sergey! On Sep 07, Sergey Vojtovich wrote: > revision-id: 9f7e77b5414b5b2ef9b5f50f131966821fefb9c7 (mariadb-10.1.8-229-g9f7e77b) > parent(s): 58634f6e50b40b28533a03a1afcb68f139937351 > committer: Sergey Vojtovich > timestamp: 2016-09-07 12:47:43 +0400 > message: > > MDEV-10296 - Multi-instance table cache > > - simplified access to per-share free tables list > - explain paddings > > @@ -43,7 +52,9 @@ struct TDC_element > for this share. > */ > All_share_tables_list all_tables; > - char pad[CPU_LEVEL1_DCACHE_LINESIZE]; // free_tables follows this immediately > + /** Avoid false sharing between TDC_element and free_tables */ > + char pad[CPU_LEVEL1_DCACHE_LINESIZE]; > + Share_free_tables free_tables[0]; > }; I vaguely remember that zero-length arrays are not supported by some compilers (they're gcc extension, after all). So I generally use [1] in cases like this, not [0]. Otherwise ok. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

Re: [Maria-developers] [Commits] 66b4459: MDEV-9143 JSON_xxx functions.
by Sergei Golubchik 09 Sep '16

09 Sep '16

Hi, Alexey! On Sep 05, Alexey Botchkov wrote: > revision-id: 66b4459e38f5df4913ecce1c9e3d71c7afa7860d (mariadb-10.2.1-21-g66b4459) > parent(s): 31a8cf54c8a7913338480a0571feaf32143b5f64 > committer: Alexey Botchkov > timestamp: 2016-09-05 14:03:33 +0400 > message: > > MDEV-9143 JSON_xxx functions. > > Library with JSON-related functions added. I've just briefly looked through the code, assuming it's mostly the same as the in previous commit. Looks much better comment- and naming-wise, thanks. Could please you add some unit tests for it into unittest/strings ? In a separate commit, if you'd like, that doesn't matter. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 0a86a91: MDEV-10765: Wrong result - query does not retrieve values from default partition on a table partitioned by list columns
by Sergey Petrunia 08 Sep '16

08 Sep '16

Hi Sanja, > revision-id: 0a86a915842d268477c5febd8481263f00d6c792 (mariadb-10.1.8-242-g0a86a91) > parent(s): effb65bc863da0f1115e16ef5f11d11a13cdc7a0 > committer: Oleksandr Byelkin > timestamp: 2016-09-08 19:43:09 +0200 > message: > > MDEV-10765: Wrong result - query does not retrieve values from default partition on a table partitioned by list columns > > Partial matches should be treat as not exact one. > So I'm running this example: create table t14n ( a int not null, b int not null, c int ) partition by list columns(a,b) ( partition p1 values in ((10,10)), partition p2 values in ((10,20)), partition p3 values in ((10,30)), partition p4 values in ((10,40)), partition p5 values in ((10,50)) ); insert into t14n values (10,10,1234), (10,20,1234), (10,30,1234), (10,40,1234), (10,50,1234); explain partitions select * from t14n where a>=10 and (a <=10 and b <=30); and then I get: #1 0x0000555555eb88ef in get_part_iter_for_interval_cols_via_map (part_info=0x7fff90046208, is_subpart=false, store_length_array=0x7ffff436a0a0, min_value=0x7fff90069518 "\n", max_value=0x7fff90069528 "\n", min_len=8, max_len=4, flags=0, part_iter=0x7ffff436a858) at /home/psergey/dev-git/10.2/sql/sql_partition.cc:7751 This means I get all the way to the memcmp(min_value, max_value, min_len) call, where min_len=8, max_len=4, which means we're comparing garbage. Please add a check that min_len==max_len. Ok to push after this is addressed. On Thu, Sep 08, 2016 at 07:43:10PM +0200, Oleksandr Byelkin wrote: > diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc > index 54396b9..24dff23 100644 > --- a/sql/sql_partition.cc > +++ b/sql/sql_partition.cc > @@ -7723,6 +7723,7 @@ int get_part_iter_for_interval_cols_via_map(partition_info *part_info, > bool can_match_multiple_values; > uint32 nparts; > get_col_endpoint_func UNINIT_VAR(get_col_endpoint); > + uint full_length= 0; > DBUG_ENTER("get_part_iter_for_interval_cols_via_map"); > > if (part_info->part_type == RANGE_PARTITION) > @@ -7740,9 +7741,13 @@ int get_part_iter_for_interval_cols_via_map(partition_info *part_info, > else > assert(0); > > + for (uint32 i= 0; i < part_info->num_columns; i++) > + full_length+= store_length_array[i]; > + > can_match_multiple_values= ((flags & > (NO_MIN_RANGE | NO_MAX_RANGE | NEAR_MIN | > NEAR_MAX)) || > + (min_len != full_length) || > memcmp(min_value, max_value, min_len)); > DBUG_ASSERT(can_match_multiple_values || (flags & EQ_RANGE) || flags == 0); > if (can_match_multiple_values && part_info->has_default_partititon()) > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] Contents of information_schema.partitions.PARTITION_DESCRIPTION for DEFAULT partitions?
by Sergey Petrunia 08 Sep '16

08 Sep '16

Hi Sanja, I'm looking at I_S description for the DEFAULT partition and I see this: PARTITION_DESCRIPTION: (MAXVALUE,MAXVALUE) Is this normal? The full example below: show create table t10; +-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | Table | Create Table | +-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | t10 | CREATE TABLE `t10` ( `i` varchar(10) DEFAULT NULL, `j` varchar(10) DEFAULT NULL ) ENGINE=InnoDB DEFAULT CHARSET=latin1 /*!50500 PARTITION BY LIST COLUMNS(i,j) (PARTITION p1 VALUES IN (('10','10')) ENGINE = InnoDB, PARTITION p2 DEFAULT ENGINE = InnoDB) */ | +-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ 1 row in set (0.01 sec) MariaDB [j4]> select * from information_schema.partitions where table_name='t10'\G *************************** 1. row *************************** TABLE_CATALOG: def TABLE_SCHEMA: j4 TABLE_NAME: t10 PARTITION_NAME: p1 SUBPARTITION_NAME: NULL PARTITION_ORDINAL_POSITION: 1 SUBPARTITION_ORDINAL_POSITION: NULL PARTITION_METHOD: LIST COLUMNS SUBPARTITION_METHOD: NULL PARTITION_EXPRESSION: `i`,`j` SUBPARTITION_EXPRESSION: NULL PARTITION_DESCRIPTION: ('10','10') TABLE_ROWS: 0 AVG_ROW_LENGTH: 0 DATA_LENGTH: 16384 MAX_DATA_LENGTH: NULL INDEX_LENGTH: 0 DATA_FREE: 0 CREATE_TIME: 2016-09-08 21:22:45 UPDATE_TIME: NULL CHECK_TIME: NULL CHECKSUM: NULL PARTITION_COMMENT: NODEGROUP: default TABLESPACE_NAME: NULL *************************** 2. row *************************** TABLE_CATALOG: def TABLE_SCHEMA: j4 TABLE_NAME: t10 PARTITION_NAME: p2 SUBPARTITION_NAME: NULL PARTITION_ORDINAL_POSITION: 2 SUBPARTITION_ORDINAL_POSITION: NULL PARTITION_METHOD: LIST COLUMNS SUBPARTITION_METHOD: NULL PARTITION_EXPRESSION: `i`,`j` SUBPARTITION_EXPRESSION: NULL PARTITION_DESCRIPTION: (MAXVALUE,MAXVALUE) TABLE_ROWS: 0 AVG_ROW_LENGTH: 0 DATA_LENGTH: 16384 MAX_DATA_LENGTH: NULL INDEX_LENGTH: 0 DATA_FREE: 0 CREATE_TIME: 2016-09-08 21:22:45 UPDATE_TIME: NULL CHECK_TIME: NULL CHECKSUM: NULL PARTITION_COMMENT: NODEGROUP: default TABLESPACE_NAME: NULL 2 rows in set (0.01 sec) BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

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

08 Sep '16

Rich Prohaska <prohaska7(a)gmail.com> writes: > The group lock retry algorithm is on the https://github.com/ > prohaska7/tokuft/tree/killwait branch. Its unit tests pass. Needed to add > some test only functions to get reproducible behaviour. > > The group lock retry algorithm is integrated into my mariadb server on the > https://github.com/prohaska7/mariadb-server/tree/toku_opr3 branch. Ran > sysbench oltp on a small 1000 row table successfully. Looks great, thanks! It passes tests for me, as well. > I am going to write up the tokudb lock tree races that were fixed and email > to George Lorch @ Percona so that this code can be integrated into > PerconaFT. Ok, sounds great! I will push the replication part of the patch to MariaDB 10.1, then (the async deadlock kill). >From the git history, it looks like new TokuDB releases (from Percona Server) are regularly merged into MariaDB 10.1, so I'm thinking that we can get your TokuDB/tokuft changes into MariaDB that way, in the next regular TokuDB merges. I will check it and add any missing MariaDB stuff, if it is not part of the changes that go upstream. Does that sounds ok to you? > Removed the lock wait for report from the lock request start method since > it is redundant with the report that will occur when the lock request is > retried in the lock request wait method. The reason I added this reporting originally was for the case where a deadlock is detected. If transaction T1 tries to get a lock with lock_request::start(), but a deadlock is detected (DB_LOCK_DEADLOCK is returned), the lock request wait method will not be called (if I understand the code correctly), so the reporting in lock_request::start() was not redundant. The rationale is that if T1 gets aborted due to a deadlock with T2, and T2 is later in the replication commit order, then when T1 is run again by replication, it will almost certainly conflict with T2 again. So we might as well get T2 killed early (by doing the report already in start()). But on the other hand, things will work correctly without any reporting in start(), and with only a slight delay in case of a conflict. And the assumption in optimistic parallel replication is that conflicts will be relatively rare. So I'm fine without reporting in start(), as you have in your current code. Looks like we are close now to having optimistic parallel replication working with TokuDB. Thanks for all your work on this, Rich! - Kristian.

1 0