- developers - lists.mariadb.org

Re: [Maria-developers] 80749146b7d: MDEV-26238: Remove inconsistent behaviour of --default-* options in my_print_defaults
by Sergei Golubchik 07 Nov '21

07 Nov '21

Hi, Rucha! On Nov 05, Rucha Deodhar wrote: > > On Nov 04, Rucha Deodhar wrote: > > > revision-id: 80749146b7d (mariadb-10.6.1-77-g80749146b7d) > > > parent(s): 76149650764 > > > author: Rucha Deodhar > > > committer: Rucha Deodhar > > > timestamp: 2021-09-12 02:11:19 +0530 > > > message: > > > > > > MDEV-26238: Remove inconsistent behaviour of --default-* options > > > in my_print_defaults > > > … [View More]> > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > > > index b7f52382721..607509a309e 100644 > > > --- a/extra/my_print_defaults.c > > > +++ b/extra/my_print_defaults.c > > > @@ -141,50 +124,39 @@ static int get_options(int *argc,char ***argv) > > ... > > > + /* > > > + We already process --defaults* options at the beginning in > > > + get_defaults_options(). So skip --defaults* options and > > > + pass remaining options to handle_options(). > > > + */ > > > + if (is_prefix(org_argv[1], "--no-defaults")) > > > + args_used--; > > > > I'm sorry, I don't understand. Why do you need that? > > I made it to keep the output consistent with previous version. > > If --no-defaults is present in the command line, then it is counted in > args_used when get_defaults_options() is called. Later we skip > "args_used" number of arguments so that only non-default options > mentioned after --default-* options are passed to get_options(). > > The above code decreases this count when --no-defaults is present so > that we don't skip it unlike --defaults-* options and pass it to > get_options(). It is required to pass --no-defaults to get_options() > because if it is not passed then it produces output which looks like > --help ( if no other options are there in addition to --no-defaults). > If it is passed to get_options(), then --no-defaults produces no > output like it does in 10.6. But it's correct, isn't it? If you run my_print_defaults with no groups, it'll print the help text. Try, for example, my_print_defaults --defaults-file=var/my.cnf (presuming you're in mysql-test dir). You'll get the help text, because you didn't specify any groups. Or my_print_defaults --defaults-extra-file=var/my.cnf But my_print_defaults --no-defaults doesn't do it. Doesn't behave like other *defaults* options. We can easily preserve the inconsisent "if --no-defaults, no group is required" behavior with, like if (my_no_defaults) cleanup_and_exit(0); (and add my_no_defaults to my_default.h). But I don't think it's necessary. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

1 0

Re: [Maria-developers] 50f43d46299: MDEV-14938 make buildbot to include galera into bintars
by Sergei Golubchik 05 Nov '21

05 Nov '21

Hi, Alexey! On Nov 05, Alexey Bychko wrote: > revision-id: 50f43d46299 (mariadb-10.2.40-106-g50f43d46299) > parent(s): 05c3dced861 > author: Alexey Bychko > committer: Alexey Bychko > timestamp: 2021-10-22 16:30:56 +0700 > message: > > MDEV-14938 make buildbot to include galera into bintars > > this commit adds cmake module to process external directory with built > galera to include galera library and binaries into server build. > it introduces new cmake … [View More]variable EXT_GALERA_PATH representing relative > or absolute path to compiled galera tree. No compiled galera tree. This is useless. MDEV description says Currently bintars that buildbot produces don't contain libgalera_smm.so. Publishing scripts repackage bintars to add it. We want to produce correct packages in buildbot and release exactly what buildbot builds. It means, buildbot need to produce *exactly* what `prep` script was generating. `prep` script does not build galera. This is a completely separate step, `prep` only packages pre-existing galera files. It's enough to assume that there's ${EXT_GALERA_PATH}/garbd and ${EXT_GALERA_PATH}/libgalera_smm.so. Or if you want it to be really universal, make two different variables, cmake -DBINTAR_GARBD=/path/to/garbd -DBINTAR_LIBGALERA=/path/to/libgalera_smm.so This will work with a compiled galera tree, with installed galera package, with just garbd and libgalera_smm.so in $HOME, with everything. > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 67216e0e443..1107013860c 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -153,6 +153,7 @@ INCLUDE(mysql_version) > INCLUDE(cpack_source_ignore_files) > INCLUDE(install_layout) > INCLUDE(wsrep) > +INCLUDE(galera_external) > > # Add macros > INCLUDE(character_sets) > diff --git a/cmake/galera_external.cmake b/cmake/galera_external.cmake > new file mode 100644 > index 00000000000..64ddfc9c498 > --- /dev/null > +++ b/cmake/galera_external.cmake > @@ -0,0 +1,75 @@ > +# EXT_GALERA_PATH > +# a path to built galera git tree > +# or a path to unpacked binary tarball > + > +IF(WIN32 OR NOT EXT_GALERA_PATH) why do you need WIN32 check here? > + RETURN() > +ENDIF() > + > +IF(NOT EXISTS ${EXT_GALERA_PATH}) > + MESSAGE(FATAL_ERROR "Galera dir ${EXT_GALERA_PATH} does not exist!") > +ENDIF() > + > +SET(GALERA_PATH ${CMAKE_SOURCE_DIR}/GALERASYM) > +IF(EXISTS ${GALERA_PATH}) > + FILE({REMOVE ${GALERA_PATH}) > +ENDIF() > +EXECUTE_PROCESS(COMMAND ${CMAKE_COMMAND} -E create_symlink ${EXT_GALERA_PATH} ${GALERA_PATH}) Why do you need this symlink? > + > +FIND_LIBRARY(GALERA_LIB > + NAMES galera_smm galera_enterprise_smm > + PATHS ${GALERA_PATH} > + NO_DEFAULT_PATH > + ) > + > +IF(NOT GALERA_LIB) > + MESSAGE(FATAL_ERROR "Galera library not found in ${GALERA_PATH}") > +ENDIF() > + > +INSTALL(PROGRAMS > + ${GALERA_LIB} > + DESTINATION lib > + RENAME libgalera_smm.so > + COMPONENT Server > + ) > + > +INSTALL(PROGRAMS > + ${GALERA_PATH}/garb/garbd > + DESTINATION bin > + COMPONENT Server > + ) > + > +INSTALL(PROGRAMS > + ${GALERA_PATH}/garb/files/garb-systemd > + DESTINATION bin > + COMPONENT Server > + ) > + > +INSTALL(FILES > + ${GALERA_PATH}/garb/files/garb.cnf > + DESTINATION docs/galera > + COMPONENT Server > + ) MDEV description says Currently bintars that buildbot produces don't contain libgalera_smm.so. Publishing scripts repackage bintars to add it. We want to produce correct packages in buildbot and release exactly what buildbot builds. It means, buildbot need to produce *exactly* what `prep` script was generating. Meaning three galera files. Like: -rwxr-xr-x mariadb-10.2.40-linux-glibc_214-x86_64/bin/garbd -rw-r--r-- mariadb-10.2.40-linux-glibc_214-x86_64/lib/libgalera_smm.so lrwxrwxrwx mariadb-10.2.40-linux-glibc_214-x86_64/lib/galera/libgalera_smm.so -> ../libgalera_smm.so Nothing else. Also, please, verify that the generated directory inside the tarball matches the pattern above. And that the tarbal name does too. > + > +INSTALL(FILES > + ${GALERA_PATH}/AUTHORS > + ${GALERA_PATH}/COPYING > + ${GALERA_PATH}/README > + DESTINATION docs/galera/doc > + COMPONENT Server > + ) > + > +INSTALL(FILES > + ${GALERA_PATH}/asio/LICENSE_1_0.txt > + DESTINATION docs/galera/doc > + RENAME LICENSE.asio > + COMPONENT Server > + ) > + > +INSTALL(FILES > + ${GALERA_PATH}/chromium/LICENSE > + DESTINATION docs/galera/doc > + RENAME LICENSE.chromium > + COMPONENT Server > + ) > + > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

1 0

Re: [Maria-developers] MDEV-23766: Optimizer tracing code is prone to producing invalid JSON
by Sergey Petrunia 05 Nov '21

05 Nov '21

Hi Serg, On Tue, Nov 02, 2021 at 02:19:05AM +0200, Sergei Krivonos wrote: > Applied fixes by your tests patch 638930f85f… <https://github.com/MariaDB/server/commit/638930f85ff40bd39354da81e53e7278a4…> > #1: Please check out the commit adding unit tests: http://lists.askmonty.org/pipermail/commits/2021-November/014773.html It turns out it is not that hard to add them. Any comments/ways to improve this? #2: In the above commit, note that two last unit tests show a scenario that … [View More]

1 0

Re: [Maria-developers] 80749146b7d: MDEV-26238: Remove inconsistent behaviour of --default-* options in my_print_defaults
by Sergei Golubchik 04 Nov '21

04 Nov '21

Hi, Rucha! On Nov 04, Rucha Deodhar wrote: > revision-id: 80749146b7d (mariadb-10.6.1-77-g80749146b7d) > parent(s): 76149650764 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-09-12 02:11:19 +0530 > message: > > MDEV-26238: Remove inconsistent behaviour of --default-* options > in my_print_defaults > > Analysis: --defaults* option is recognized anywhere in the commandline > instead of only at the beginning because handle_options() … [View More]recognizes > options in any order. > Fix: use get_defaults_options() which recognizes --defaults* options only at > the beginning. After this is done, we only want to recognize other options > given in any order which can be done using handle_options(). So only skip > --defaults* options and pass rest of them to handle_options(). > Also, removed -e, -g and -c because only my_print_defaults supports them. > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > index b7f52382721..607509a309e 100644 > --- a/extra/my_print_defaults.c > +++ b/extra/my_print_defaults.c > @@ -141,50 +124,39 @@ static int get_options(int *argc,char ***argv) ... > int main(int argc, char **argv) > { > - int count= 0, error, no_defaults= 0; > + int count, error, args_used; > char **load_default_groups= 0, *tmp_arguments[6]; > char **argument, **arguments, **org_argv; > int nargs, i= 0; > MY_INIT(argv[0]); > > org_argv= argv; > - if (*argv && !strcmp(*argv, "--no-defaults")) > - { > - argv++; > - ++count; > - no_defaults= 1; > - } > - /* Copy program name and --no-defaults if present*/ > + args_used= get_defaults_options(argv); > + > + /* Copy defaults-xxx arguments & program name */ > + count= args_used; > arguments= tmp_arguments; > - memcpy((char*) arguments, (char*) org_argv, (++count)*sizeof(*org_argv)); > + memcpy((char*) arguments, (char*) org_argv, count*sizeof(*org_argv)); > arguments[count]= 0; I still cannot understand why you're doing it. Why do you need a copy of argv? > > + /* > + We already process --defaults* options at the beginning in > + get_defaults_options(). So skip --defaults* options and > + pass remaining options to handle_options(). > + */ > + if (is_prefix(org_argv[1], "--no-defaults")) > + args_used--; I'm sorry, I don't understand. Why do you need that? all *defaults* options were handled in get_defaults_options(). > + why do you need org_argv? First you do org_argv= argv; then after some code that doesn't change either argv or org_argv, you copy the vallue back: > + argv=org_argv; > + argv+=args_used-1; > + argc-=args_used-1; > + > /* Check out the args */ > if (get_options(&argc,&argv)) > cleanup_and_exit(1); > > - if (!no_defaults) > - { > - if (opt_defaults_file_used) > - arguments[count++]= make_args("--defaults-file=", config_file); > - if (my_defaults_extra_file) > - arguments[count++]= make_args("--defaults-extra-file=", > - my_defaults_extra_file); > - if (my_defaults_group_suffix) > - arguments[count++]= make_args("--defaults-group-suffix=", > - my_defaults_group_suffix); > - arguments[count]= 0; > - } > - > nargs= argc + 1; > if (opt_mysqld) > nargs+= array_elements(mysqld_groups); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

1 0

[Maria-developers] [MDEV-10654] Review request
by Manohar K B 04 Nov '21

04 Nov '21

Hi All, Good day. I have fixed ticket MDEV-10654 and raised PR. Can someone kindly review it? This is the first time I am contributing to MariaDB, so please guide me if I missed anything. Best regards, Manohar.

2 1

Re: [Maria-developers] bb1737b0482: MDEV-12459: The information_schema tables for getting temporary tables info is missing, at least for innodb there is no INNODB_TEMP_TABLE_INFO
by Sergei Golubchik 01 Nov '21

01 Nov '21

Hi, Anel! On Nov 01, Anel Husakovic wrote: > revision-id: bb1737b0482 (mariadb-10.6.1-73-gbb1737b0482) > parent(s): 1bc82aaf0a7 > author: Anel Husakovic > committer: Anel Husakovic > timestamp: 2021-09-06 08:58:46 +0200 > message: > > MDEV-12459: The information_schema tables for getting temporary tables info is missing, at least for innodb there is no INNODB_TEMP_TABLE_INFO > > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 6b351726320..5444de8c1ca … [View More]100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -69,6 +69,8 @@ > > #include "lex_symbol.h" > #define KEYWORD_SIZE 64 > +#define IS_USER_TEMP_TABLE(A) ((A->tmp_table == TRANSACTIONAL_TMP_TABLE) || \ > + (A->tmp_table == NON_TRANSACTIONAL_TMP_TABLE)) > > extern SYMBOL symbols[]; > extern size_t symbols_length; > @@ -151,6 +153,8 @@ static int show_create_sequence(THD *thd, TABLE_LIST *table_list, > static const LEX_CSTRING *view_algorithm(TABLE_LIST *table); > > bool get_lookup_field_values(THD *, COND *, TABLE_LIST *, LOOKUP_FIELD_VALUES *); > +void process_i_s_table_temporary_tables(THD *thd, TABLE * table, LEX_CSTRING *db_name, > + LEX_CSTRING *table_name, TABLE *tmp_tbl); > > /** > Try to lock a mutex, but give up after a short while to not cause deadlocks > @@ -5161,6 +5166,10 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > uint table_open_method= tables->table_open_method; > bool can_deadlock; > MEM_ROOT tmp_mem_root; > + Dynamic_array<LEX_CSTRING> system_tables(PSI_INSTRUMENT_MEM); > + All_tmp_tables_list *temp_tables= NULL; > + // Scan for temporary tables > + TMP_TABLE_SHARE *share_temp; > DBUG_ENTER("get_all_tables"); > > bzero(&tmp_mem_root, sizeof(tmp_mem_root)); > @@ -5223,6 +5232,12 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > init_alloc_root(PSI_INSTRUMENT_ME, &tmp_mem_root, SHOW_ALLOC_BLOCK_SIZE, > SHOW_ALLOC_BLOCK_SIZE, MY_THREAD_SPECIFIC); > > + system_tables.push(INFORMATION_SCHEMA_NAME); > + system_tables.push(PERFORMANCE_SCHEMA_DB_NAME); > + system_tables.push(MYSQL_SCHEMA_NAME); > + system_tables.push(SYS_SCHEMA_NAME); > + system_tables.push(MTR_SCHEMA_NAME); > + > for (size_t i=0; i < db_names.elements(); i++) > { > LEX_CSTRING *db_name= db_names.at(i); > @@ -5234,6 +5249,36 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > sctx->master_access & (DB_ACLS | SHOW_DB_ACL) || > acl_get(sctx->host, sctx->ip, sctx->priv_user, db_name->str, 0)) > #endif > + > + // Only if there is IS.tables allow temporary tables to be shown > + if (schema_table_idx == SCH_TABLES && !temp_tables) > + { > + for (size_t k=0; k < system_tables.elements(); k++) > + { > + if (db_name != &system_tables.at(k)) > + { > + temp_tables= open_tables_state_backup.temporary_tables; > + if (temp_tables) > + { > + All_tmp_tables_list::Iterator it(*open_tables_state_backup.temporary_tables); > + while ((share_temp= it++)) > + { > + All_share_tables_list::Iterator it2(share_temp->all_tmp_tables); > + while (TABLE *tbl= it2++) > + { > + if (IS_USER_TEMP_TABLE(share_temp)) > + { > + // Now we have the data and we should process_table() manually > + process_i_s_table_temporary_tables(thd, table, db_name, > + &share_temp->table_name, tbl); > + } > + } > + } > + } > + break; > + } > + } > + } hmmmm. What do you do here? I understand, basically, nothing here. Could you explain, line by line? > { > Dynamic_array<LEX_CSTRING*> table_names(PSI_INSTRUMENT_MEM); > int res= make_table_name_list(thd, &table_names, lex, > @@ -5495,6 +5540,8 @@ static int get_schema_tables_record(THD *thd, TABLE_LIST *tables, > table->field[3]->store(STRING_WITH_LEN("SYSTEM VIEW"), cs); > else if (share->table_type == TABLE_TYPE_SEQUENCE) > table->field[3]->store(STRING_WITH_LEN("SEQUENCE"), cs); > + else if (IS_USER_TEMP_TABLE(share)) > + table->field[3]->store(STRING_WITH_LEN("TEMPORARY"), cs); what about INTERNAL_TMP_TABLE and SYSTEM_TMP_TABLE? > else > { > DBUG_ASSERT(share->tmp_table == NO_TMP_TABLE); > @@ -9032,7 +9096,6 @@ ST_FIELD_INFO tables_fields_info[]= > NOT_NULL, "Comment", OPEN_FRM_ONLY), > Column("MAX_INDEX_LENGTH",ULonglong(), NULLABLE, "Max_index_length", > OPEN_FULL_TABLE), > - Column("TEMPORARY", Varchar(1), NULLABLE, "Temporary", OPEN_FRM_ONLY), > CEnd() > }; Let's remove it in a separate commit. After you implement "TEMPORARY" for table type. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

1 0

Re: [Maria-developers] MDEV-23766:Optimizer tracing code is prone to producing invalid JSON
by Sergey Petrunia 31 Oct '21

31 Oct '21

Hi Serg, Ok the patch moving in the right direction but it's not there, yet. Please find the input below. First, input on the commit comment. Please consider it as a request applying to ALL further commits to the MariaDB codebase. > MDEV-23766: implemented requested debug checks Imagine somebody looking at this in a few years. Will they know what checks were requested? They might get a clue by looking at the Jira text, but the idea to make the commit comments concise and self-contained … [View More]

1 1

Re: [Maria-developers] 85aa3f8cf66: MDEV-26102: dgcov: add support for *.gcda.gcov.json.gz files of gcov 9.1+
by Sergei Golubchik 30 Oct '21

30 Oct '21

Hi, Anel! Looks ok. Except that you have that pointless $gcc_version=~ line that I wrote about in a previous review. Did you verify that this script actually delivers meaningful results? On Oct 30, Anel Husakovic wrote: > revision-id: 85aa3f8cf66 (mariadb-10.2.39-96-g85aa3f8cf66) > parent(s): afe00bb7cce > author: Anel Husakovic > committer: Anel Husakovic > timestamp: 2021-07-29 12:42:44 +0200 > message: > > MDEV-26102: dgcov: add support for *.gcda.gcov.json.gz … [View More]

1 0

Re: [Maria-developers] 2c8f52ea53d: MDEV-23328 Server hang due to Galera lock conflict resolution
by Sergei Golubchik 29 Oct '21

29 Oct '21

Hi, Seppo, Jan! this is pretty much the same as in 10.4, so same comments apply, otherwise ok. On Oct 29, sjaakola wrote: > revision-id: 2c8f52ea53d (mariadb-10.5.12-108-g2c8f52ea53d) > parent(s): 910fc72a7f1 > author: sjaakola > committer: Jan Lindström > timestamp: 2021-10-28 12:00:41 +0300 > message: > > MDEV-23328 Server hang due to Galera lock conflict resolution Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 778d88ea2d9: MDEV-23328 Server hang due to Galera lock conflict resolution
by Sergei Golubchik 29 Oct '21

29 Oct '21

Hi, Seppo, Jan! Looks pretty much ok too. A few comments, see below On Oct 29, sjaakola wrote: > revision-id: 778d88ea2d9 (mariadb-10.4.21-76-g778d88ea2d9) > parent(s): e2e4e06f081 > author: sjaakola > committer: Jan Lindström > timestamp: 2021-10-28 11:06:22 +0300 > message: > > MDEV-23328 Server hang due to Galera lock conflict resolution > > diff --git a/sql/log_event.cc b/sql/log_event.cc > index c1c753d69fd..a1562016aa4 100644 > --- a/sql/log_event.… [View More]

1 0