- developers - lists.mariadb.org

[Maria-developers] New Hacker
by Aniket Shukla 27 Aug '15

27 Aug '15

Hey MariaDB team,this is Aniket.I'm a final year undegraduate student and would like to contribute to MariaDB.Since this is my first open source project, I have no idea where to begin.I have good knowledge of python,c++ and have benn using MySQL for my previous projects. Any help would be welcomed.

2 1

[Maria-developers] [GSoC] MDEV-4674 Port InnoDB memcached interface to MariaDB - Final report
by Piotr Jurkiewicz 25 Aug '15

25 Aug '15

Hi all, as the pencils down date has passed, I am posting a report on what I have during the whole project. Repository of my project is available here: https://github.com/piotrjurkiewicz/mariadb-server/commits/10.1-memcached Some stats: 33 commits before the midterm 60 commits after the midterm 93 commits total What have been done in details: =============================== BEFORE MIDTERM: --------------- 1. Code of InnoDB Memcached plugin has been imported from MySQL into the MariaDB tree. 2. There were some differences between MariaDB and MySQL, which blocked the plugin from out-of-the-box compilation and correct operation. They have been resolved. 3. Test suite for the plugin has been imported from MySQL. Compatibility changes were introduced into these test cases which required them. The test suite was modified to assign memcached port number dynamically from mtr worker's pool, what allows to run the same test in parallel for many combinations. The plugin fully passes the test suite, both for 'innodb_plugin' and 'xtradb' combinations. 4. Some enhancements and bug fixes have been introduced into the plugin: - detection of memcached daemon initialization has been fixed -- this eliminated unnecessary wait (15 seconds) on plugin shutdown and memory leak (in cases when plugin failed to initialize properly before) - memcached arithmetic commands (incr/decr) handling has been improved -- now numeric columns values are handled in a consistent and predictable way - incorrect handling of unsigned not null integers columns has been fixed AFTER MIDTERM: -------------- 5. Memcached daemon code has been updated to the latest version from upstream memcached/engine-pu branch. 6. Plugin architecture has been changed to "one table per socket", as originally proposed in MDEV-4674 bug report. This allows usage of well known security solutions (firewalls, unix file permissions) to restrict access to different tables for different users. Now user can specify which tables should be accessible on which sockets. Memcached daemon will listen on all these sockets and execute commands on appropriate table, depending on which socket client connected. Supported protocols are TCP, UDP and UNIX stream sockets. Sockets are configured in 'daemon_memcached/containers' table: INSERT INTO containers VALUES ("tcp://*:11211", "db0", "table0", "key_col", "val_col", "c3", "c4", "c5", "PRIMARY", "|"); INSERT INTO containers VALUES ("udp://localhost:11222", "db0", "table1", "key_col", "val_col", "c3", "c4", "c5", "PRIMARY", "|"); INSERT INTO containers VALUES ("unix:/tmp/mc.sock", "db1", "table0", "key_col", "val_col", "c3", "c4", "c5", "PRIMARY", "|"); 7. Plugin has been decoupled from InnoDB. Now plugin is split into two parts: - generic 'daemon_memcached.so' - InnoDB specific 'daemon_memcached_engine_ib.so' The first library is a MariaDB plugin, which should be installed with command: install plugin daemon_memcached soname "daemon_memcached.so"; This library is generic. It provides Memcached protocol handling and it loads the second library, which is API-specific engine for accessing the database. Which engine library should be loaded can be specified in 'daemon_memcached_engine_lib_name' MariaDB server option. Now, the only existing engine library is daemon_memcached_engine_ib.so, which utilizes InnoDB API to access the data. It can operate on InnoDB and XtraDB tables. 8. Some enhancements and bug fixes have been introduced into the plugin: - CMake options and #cmakedefines associated with the plugin have been cleaned up - embedded default Memcached engine has been removed - duplicated code in Memcached source has been removed - fixed MySQL-originating bug with flush_all of non default table -- command flush_all always flushed the default table, not the current one - removed support for in-connection table switching -- after introduction of one table/port architecture it was redundant - plugin shutdown time has been improved -- from constant time of 2 seconds to average time of 0.5 second - logging of plugin has been refactored -- now plugin send log messages to MariaDB log instead of stderr - fixed Memcached-originating bug with get_thread_stats() -- bug resulted in out of bound array accesses and writes, what leaded to SEGFAULT - fixed MySQL-originating crash during 'flush_all' memcached command bug -- crash happened when table was heavily loaded with DML operations and because of that ib_cb_cursor_set_lock() failed to obtain a lock on the table, returning DB_LOCK_WAIT_TIMEOUT error - fixed MySQL-originating multiple memcached_sync_count for the same transaction bug -- the memcached_sync_count was incremented multiple times for the same transaction and after the end of transaction, it was not reset to zero, what blocked TRUNCATE command on any InnoDB table, on which Memcached had operated during program lifetime, even after uninstalling the plugin - support for numeric values has been finished -- now BIGINT overflows in the same way as smaller types, test to check over/under-flowing of all integer types has been added For details of all changes please see commit messages associated with them: https://github.com/piotrjurkiewicz/mariadb-server/commits/10.1-memcached ----------------------------------------------------------------------- Moreover, I planned to implement a new interface inside the Memcached plugin, which will use Handler API for database access. This would make possible to use the plugin to access databases based on storage engines other than InnoDB/XtraDB. However, during testing I discovered many serious bugs, either in Memcached or InnoDB API engine, which originated from upstream Memcached or MySQL and resulted in crashes or deadlocks. I decided that first I should fix them before implementing new functionalities. I lost a lot of time on investigating these bugs and I was unable to finish the implementation of Handler API engine. Nevertheless, I think the whole plugin is ready to be merged into MariaDB. All user-facing changes (like one port/table architecture) have been completed. Eventual later introduction of Handler API engine will not be associated with any user-facing compatibility changes. It will just lift the requirement that all databases accessed via the Memcached plugin must be InnoDB/XtraDB. Now I am going to write a separate email to coordinate the merge of my code into MariaDB repository. Piotr

1 0

Re: [Maria-developers] MDEV-7502 Provisioning of slave - review
by Kristian Nielsen 24 Aug '15

24 Aug '15

Martin Kaluznik <martin.kaluznik(a)gmail.com> writes: > good to do now. And I would also like to ask you for review of > existing code. > > Github link: https://github.com/f4rnham/server/tree/MDEV-7502 > Diff: https://github.com/MariaDB/server/compare/10.1...f4rnham:MDEV-7502.diff Please find my review inline in patch, below. General stuff: > I have created new log event as way to signalize slave successful end > of provisioning, it is last event IO thread writes to relay log before > termination, and last event SQL thread reads from relay log. Test > cases are using Slave_SQL_Running value from SHOW SLAVE STATUS command > to wait until provisioning is completed. It works there, but I am not > sure about its convinience in real world usage. Hm, I am not sure either. Another option would be for LOAD DATA FROM MASTER to block until provisioning is done (failed or successful). Other than review, please check Buildbot results carefully for any failures. Pay particular attention to the valgrind build (for memory leaks). Also pay special attention to random failures in your new test cases that happen only occasionally. Such sporadic / random failures often turn up in replication tests. You need in test cases that use debug_dbug: --source include/have_debug.inc What is the largest data set you tested with? Please do a (manual) test with a large data set. For example, you could run sysbench with a 10GB or 50GB database, let it run for say 10 minutes, and 5 minutes in you start a LOAD DATA FROM MASTER on a slave off the server running the sysbench. Please check comments below on how (and if) the whole approach with provisioning in parallel with active transactions is correct and makes the slave always identical with the master at the end. Is there any issue with foreign keys? An insert to one table could fail if a foreign key constraint against another table is not yet satisfied due to rows not yet provisioned. A CREATE TABLE referencing another table might fail. Maybe look at if mysqldump is doing something to avoid these kinds of issues. A minimal solution is to investigate possible problems, and document them as limitations. Eg. it is reasonable to document that users must disable foreign keys during provisioning, if necessary. What happens if the server that does LOAD DATA FROM MASTER provisioning has --log-bin and --log-slave-updates? What happens if a third-level slave is connected to the server that does LOAD DATA FROM MASTER? I think this needs to be tested. Comments on the patch: > +BOOLEAN _db_keyword_locked_(CODE_STATE *cs, const char *keyword, int strict) Can you explain the race that results without this locking? Is it because _db_keyword_() is scanning the list without any locking, so if the list changes during the scan we can crash? > +#define DBUG_EXECUTE_LOCKED(keyword,a1) \ > + do {if (_db_keyword_locked_(0, (keyword), 0)) { a1 }} while(0) You need empty definition for non-debug build (but I guess you already saw in buildbot run). > +--let $slave_param= Slave_SQL_Running > +--let $slave_param_value= No > +--let $slave_error_param= Last_SQL_Errno > +--source include/wait_for_slave_param.inc Is there a reason not just to use include/wait_for_slave_sql_to_stop.inc here? > +# Ensure that master and slave log names and positions are equal > +--connection master > +--let $binlog_file_master= query_get_value(SHOW MASTER STATUS, File, 1) > +--let $binlog_pos_master= query_get_value(SHOW MASTER STATUS, Position, 1) > +--connection slave > +--let $binlog_file_slave= query_get_value(SHOW SLAVE STATUS, Master_Log_File, 1) > +--let $binlog_pos_slave= query_get_value(SHOW SLAVE STATUS, Read_Master_Log_Pos, 1) Is there a reason you do not use GTIDs here instead (or in addition)? > +# echo Binlog file name master - $binlog_file_master; > +# echo Binlog file name slave - $binlog_file_slave; > +# echo Binlog position master - $binlog_pos_master; > +# echo Binlog position slave - $binlog_pos_slave; Please remove this left-over debugging code. > +++ b/mysql-test/suite/rpl/t/rpl_provisioning_base.inc > +eval CREATE TABLE test.test_1 (a int, PRIMARY KEY (a)) ENGINE=$engine_type; > +eval CREATE TABLE test.test_2 (a int, b int, c int, PRIMARY KEY (a)) ENGINE=$engine_type; Try adding also some foreign key contraints to the table for the test. > +# Start provisioning > +--connection slave > +LOAD DATA FROM MASTER; At this point, master has full binlogs. So just START SLAVE would do the same thing, just by re-playing the binlog. I think the test should use FLUSH LOGS on the master a couple of times during the initial setup of data, and then PURGE BINARY LOGS to delete some (but not all) of that binlog data, to test that the slave can still provision without complete binary logs. > +SET GLOBAL debug_dbug="+d,provisioning_wait"; > +SET GLOBAL debug_dbug="-d,provisioning_wait"; Earlier, when I did this, I ended up with tons of debug output in the error log after setting the "-d,..." which is annoying. Setting an empty "+d" option was enabling debug trace. Maybe this is now fixed, if so then all the better. But if you get this problem (with stray debug trace in error log), you can use the following pattern instead: SET @old_dbug= @@GLOBAL.debug_dbug; ... SET GLOBAL debug_dbug= @old_dbug; > +# > +# Attempt to start provisioning while not in GTID mode > +# Is this an old comment? It seems none of the tests really use any GTID. Does provisioning work in GTID mode? It should be tested. > +# > +# Attempt to start provisioning while SQL thread is running > +# What if IO thread is running? Should probably be tested also. > +# Creation of database `test` depends purely on timing and it has nothing to do with this test case > +--disable_warnings > +DROP DATABASE IF EXISTS test; What is the purpose of this? It sounds like there is a potential race in the test case? Maybe there is a better way to handle it. > +START SLAVE; > + > +# Cleanup after test case > +--connection slave > +STOP SLAVE; > +RESET SLAVE; > +# Creation of database `test` depends purely on timing and it has nothing to do with this test case It is better to use the include/start_slave.inc and include/stop_slave.inc. They prevent some races, which might solve some of the problems that your comment about timing seems to hint at. Also, it is usually best to synchronise the slave with the master before stopping, to avoid it being stopped at a random point. > +# > +# Bug: Deadlock occurs when IO thread fails with error before dump request > +# What do you mean? That the current patch has a bug triggered by this test case? Or that there was a bug, fixed now, and this is a test for it? Please explain better in the comment. > diff --git a/mysql-test/suite/rpl/t/rpl_provisioning_stress.test b/mysql-test/suite/rpl/t/rpl_provisioning_stress.test > new file mode 100644 > index 0000000..8141015 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_provisioning_stress.test > @@ -0,0 +1,48 @@ > +--skip Test requires manual setup > + > +# Stress test for MDEV-7502 Automatic provisioning of slave > +# Using Random Query Generator v2.2.0 release, downloadable from https://launchpad.net/randgen/+download I think it is great that you made this kind of test. But maybe it would be better to put it somewhere else, to not have a test in the normal test suite that is always unconditionally skipped. Eg. it could be put in /tests/, and one would copy it into the test suite, instead of removing the --skip. > +void Provisioning_done_log_event::print(FILE* file, > + print_header(&cache, print_event_info, FALSE); > + my_b_write_string(&cache, "\tProvisioning done\n"); Hm, this can occur in the relay log, but not in the binlog, right? I think it is ok not to have a test of this in mysql-test-run, we generally can only test the "short-form" in there to avoid dependency on time and such. Just be sure to check manually that the output is correct. > @@ -9718,23 +9760,29 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi) > + // In provisioning mode, silently ignore table not found error > + if (!rgi->thd->slave_thread || !rli->mi->provisioning_mode || > + !provisioning_error_code(actual_error)) Better use thd than rgi->thd here (just for consistency). > + // Ignore certain errors during provisioning What thoughts did you have on whether just ignoring these kinds of errors would always result in the correct result of provisioning? I suppose since we are copying data fresh from the master, it should not be possible that we miss a "real" error in this way. Will things work correctly, if we have multiple rows in a single Rows_log_event, and some of them succeed while others fail? In both MyISAM and InnoDB? Can an argument be made to prove that, if we ignore an insert or update here, we are sure to later write the correct data as part of provisioning? For both InnoDB and MyISAM? Similarly, can it be proven that it is impossible to ignore a delete and still later wrongly provision the deleted row? I suppose it would be something with binlog ordering and isolation modes for the InnoDB case - and some table locking semantics wrt. MyISAM? > + > + /* > + Semaphore used by LOAD DATA FROM MASTER command for test cases, its role is > + to ensure, that slave's IO thread is connected to master before test case > + can continue. > + volatile bool dump_requested_semaphore; Shouldn't this be under #ifndef DBUG_OFF ? > diff --git a/sql/rpl_provisioning.cc b/sql/rpl_provisioning.cc > +void quote_name(char const *name, char *buff) Try to use append_identifier() instead. Or maybe my_snprintf("%`s"). I notice that append_identifier() handles a configuration where "" is used to quote identifiers instead of ``, you should probably test if this causes a bug in your code, btw. > +provisioning_send_info::provisioning_send_info(THD *thd_arg) > + : thd(thd_arg) > + , connection(new Ed_connection(thd)) > + , row_batch_end(NULL) > + , row_buffer(my_malloc(ROW_BUFFER_DEFAULT_SIZE, MYF(0))) > + , row_buffer_size(ROW_BUFFER_DEFAULT_SIZE) > + , phase(PROV_PHASE_INIT) > + , error(0) > + , error_text(NULL) > + , event_conversion_cache(NULL) > +{ > +} So this constructor will probably cause the server to crash in case of out-of-memory, right? Usually, code tries to fail gracefully with ER_OUT_OF_RESOURCES / ER_OUTOFMEMORY if malloc() fails (though its not well tested, probably). > +void provisioning_send_info::close_tables() > +{ > + trans_commit_stmt(thd); I see no call to commit the entire transaction (as opposed to just the statement here). This may be intentional (autocommit?), in any case please add a comment here explaining how the transaction is committed wrt. statement-transaction and full transaction. Maybe add a test where the server is running with auto-commit globally disabled? Though I am not sure if the short mysql-test-run.pl tests would notice it if everything was done as one long transaction (though large production sites undobtedly would ;-). > +bool provisioning_send_info::send_query_log_event(DYNAMIC_STRING const *query, Is there a particular reason that you chose DYNAMIC_STRING over the String class? I think String is more widely used for building queries and such? A common use pattern with String is to allocate a reasonably-sized buffer on the stack and initialise a String object from it. Then in the common case of smallish result, no my_malloc() calls are needed. > + if (cs_info) > + { > + // thd->variables.character_set_client->number > + int2store(evt.charset, cs_info->cs_client); > + // thd->variables.collation_connection->number > + int2store(evt.charset + 2, cs_info->cl_connection); > + // cs_info->cl_server > + int2store(evt.charset + 4, thd->variables.collation_server->number); > + // thd->variables.collation_database->number > + evt.charset_database_number= cs_info->cl_db; Can you explain why you need to consider the charsets/collations of the connection and client? Could you not just always use the system character set for generating queries? > + if (my_strcasecmp(system_charset_info, "VIEW", type->str) == 0) > + { > + char name_buff[NAME_LEN * 2 + 3]; > + DYNAMIC_STRING *str= (DYNAMIC_STRING*)my_malloc(sizeof(DYNAMIC_STRING), > + MYF(0)); > + > + quote_name(database->str, name_buff); > + init_dynamic_string(str, name_buff, > + // Both lengths + dot and quotes, will be reallocated > + // only in special cases > + database->length + name->length + 5, > + 5); > + dynstr_append(str, "."); > + quote_name(name->str, name_buff); > + dynstr_append(str, name_buff); > + > + views.push_back(str); > + } > + else if (my_strcasecmp(system_charset_info, "BASE TABLE", type->str) == 0) > + tables.push_back(my_strdup(name->str, MYF(0))); Why do you need to prepend the database name in VIEW case but not in TABLE case? Please add a comment explaining that. > + else > + DBUG_ASSERT(false && > + "Unexpected type of table returned by 'SHOW FULL TABLES'"); Probably you should add an error return here, that will cause provisioning to fail in the non-debug build, rather than silently press on. > +bool provisioning_send_info::event_to_packet(Log_event &evt, String &packet) > +{ > + // Reset cache for writing > + reinit_io_cache(event_conversion_cache, WRITE_CACHE, 0, false, true); > + > + if (evt.write(event_conversion_cache)) > + { > + error_text= "Failed to write event to conversion cache"; > + return true; > + } > + > + if (reinit_io_cache(event_conversion_cache, READ_CACHE, 0, false, false)) > + { > + error_text= "Failed to switch event conversion cache mode"; > + return true; > + } > + > + packet.set("\0", 1, &my_charset_bin); > + if (packet.append(event_conversion_cache, > + event_conversion_cache->write_pos - event_conversion_cache->buffer)) > + { > + error_text= "Failed to write event data to packet"; > + return true; > + } Hm. So first the event is constructed in-memory. Then it is written to an IO_CACHE. Then it is read back in again into a byte buffer. And then it is written to the slave's client socket. Hm, I see, the client socket we have only as a NET, not an IO_CACHE. But the Log_event subclasses need an IO_CACHE to construct a serialised event. Hm, that's clunky, but I don't see an easy way to avoid it. But please add a comment explaining why this is necessary. Also, doesn't the IO_CACHE spill to disk if the data written becomes large? So wouldn't a very large event (eg. a row containing a large BLOB) fail here? Because the IO_CACHE only contains part of the data in-memory? I am not 100% sure about how this works, please investigate and test with a large row (if you did not already). > + // Set current server as source of event, when slave registers on master, > + // it overwrites thd->variables.server_id with its own server id - and it is > + // then used when writing event > + int4store(&packet[SERVER_ID_OFFSET + 1], global_system_variables.server_id); > + return false; I think this will fail if event checksums are enabled? Or rather, the patch does not seem to handle binlog checksums at all. Try adding a test that enables binlog checksums on master and slave, and check that everything works. And also - why do you need to change this in the first place? Doesn't the server_id get set correctly when the event is initially created? > +bool provisioning_send_info::send_event(Log_event &evt) > +{ > + String packet; > + > + // Slave will accept this event even if it modifies data structures > + evt.flags|= LOG_EVENT_PROVISIONING_F; > + > + if (event_to_packet(evt, packet)) > + return true; > + > + if (my_net_write(&thd->net, (uchar*)packet.ptr(), packet.length())) > + { > + error_text= "Failed to send event packet to slave"; > + return true; > + } Yeah... It would be so much more elegant if we could wrap the thd->net in an IO_CACHE, and write directly to the client. Did you investigate if this would be a possibility, without too much extra work? > +int8 provisioning_send_info::send_table_data() > + if ((error= hdl->prepare_range_scan(row_batch_end, NULL)) != 0) > + if (table->s->primary_key == MAX_KEY) > + { > + error_text= "Table does not contain primary key"; > + close_tables(); > + return 1; > + } It seems more natural to me to check for missing primary key _before_ prepare_range_scan(). (Otherwise one wonders how it is possible to prepare a range scan on a table without _any_ key? I suppose it works to do a table scan instead, but better do the check first to avoid having to wonder). > + while (packed_rows < PROV_ROW_BATCH_SIZE && > + (error= hdl->read_range_next()) == 0); I think there should be two stop criterias. One is max number of rows reached, another is size of rows. At least, you need to protect against exceeding --max-packet-size. Otherwise the slave will be unable to receive the too large packet. Please add a test case that checks large rows and that it correctly splits row events to not exceed max-packet-size (of course at least one row must fit in one Rows_log_event). Check the code for normal binlogging to see how it splits up Row_log_events to avoid creating too large an event. > + table->default_column_bitmaps(); > + close_tables(); So the critical issue here is: how can we be sure that we will not miss rows during provisioning, by racing between provisioning events and normal binlog events? This needs to be clearly thought out, and then explained in a comment somewhere in the code. For example, you write the events to the slave client socket before closing the table and releasing locks. This seems critical (and correct) - because this ensures that if there is a DELETE made just after the provisioning read of the table, that DELETE will be sent only _after_ sending the provisioning rows (so we do not lose the DELETE). This should be explained in a comment. But what about isolation level (for InnoDB)? Don't we need to run with a sufficiently high isolation level to be correct? Intuitively, I thought so, but maybe not, at least you have nothing in the patch about isolation level that I can see. What about this scenario? 1. Provisioning reads rows 20-29, sends them to slave. 2. An UPDATE runs, it updates row 32 to be now row 25, it is written to the binary log and sent to the slave. 3. Provisioning reads rows 30-39, sends them to the slave. On the slave, the UPDATE from 2 should fail - because row 32 does not exist at this point. But then, it seems to me that the row 25 will never appear on the slave, right? So provisioning will be incorrect. Or am I missing something? I wonder if the solution to this particular case is that during provisioning, an UPDATE (at least one that changes primary key value) must be logged as a DELETE (of the old row) + an INSERT (of the new row). Alternatively, maybe the slave can during provisioning execute the UPDATE regardless of the missing before image, still writing the after image (row 25) to the table. More generally, we really need an explanation of how the provisioning ensures that the end result on the slave is identical to the master - otherwise there may be other problems like this hidden. Also, it would be really good if you could make a test case for the above potential issue. You can get some ideas from how I use DBUG_EXECUTE_IF together with DEBUG_SYNC in the rpl_parallel_*.test test cases. But I know it can be a lot of work to get such tricky race-proviking test cases to work reliably. > diff --git a/sql/rpl_provisioning.h b/sql/rpl_provisioning.h > +// FIXME - Farnham > +// Header > + > +#pragma once We do not use this #pragma once in mariadb (maybe that is what you intended to fix with the FIXME, so mentioning just in case). I assume you will take care of other such FIXME's in the patch yourself before pushing. > +/* > + How much rows from table will be sent in one provisioning tick > + FIXME - Farnham > + Dynamic value > +*/ > +#define PROV_ROW_BATCH_SIZE 5 Right. Probably just a matter of adding something to sql/sys_vars.cc > - if (rpl_load_gtid_slave_state(thd)) > + if (!mi->provisioning_mode && rpl_load_gtid_slave_state(thd)) Actually, what should happen with the GTID position for LOAD DATA FROM MASTER? Just clearing it probably is not correct, in case user wants to LOAD DATA FROM MASTER from multiple masters. Maybe the user should clear it herself, if necessary, and then it will be updated by the normal replication stream from master. But why do you skip loading it here for provisioning mode? That does not seem right. First, it will normally be loaded already anyway. And second, if not loaded, then just having the wrong value in-memory (as compared to in the mysql.gtid_slave_pos table) does not seem correct? > + DBUG_EXECUTE_IF("provisioning_test_running", > + if (!slave_errno) > + while (!mi->dump_requested_semaphore) > + /* pass */;); Maybe put a my_sleep(20000) in with the /* pass */. > + case PROVISIONING_DONE_EVENT: > + mi->abort_slave= true; > + /* > + Fall through to default case ... and through debug code, nothing wrong > + should happen > + */ That seems a bit fragile. Better do like in other places: goto default_action; > + // In provisioning mode, master starts sending data from current position > + if (flags & BINLOG_DUMP_PROVISIONING_MODE) So provisioning assumes non-GTID mode? Maybe that is ok... But it should still be tested what happens if provisioning is attempted in GTID mode. Perhaps either GTID or non-GTID mode can work the same, since provisioning does not need to start at any particular position - and at the end of provisioning, both GTID and non-GTID position should be correct. > +bool > +Ed_connection::execute_direct(char *sql_text, size_t length) > +{ > + LEX_STRING tmp_query; > + tmp_query.str= sql_text; > + tmp_query.length= length ? length : strlen(sql_text); > + > + return execute_direct(tmp_query); > +} You are not actually ever calling this without passing length. So please make this an inline method in the .h file, with mandatory length argument, that does not check for zero length or call strlen(). - Kristian.

1 0

[Maria-developers] Fixing Some Trivial Bugs
by Alangi Derick 21 Aug '15

21 Aug '15

Hi, I just found some trivial bugs on jira that i will be fixing for the time being. Once i am done with each one, i will submit a patch or push to my forked repo so it can be merged. My question is, after i fix the bugs, will they accepted, if yes i should continue working on them thanks. I need a respond from the development team. Regards Alangi Derick Ndimnain

3 4

[Maria-developers] [Patch] Fix to mysql.server init script
by Alangi Derick 21 Aug '15

21 Aug '15

This is a fix to the bug in the mysql.server init script which uses a wrong server name (MySQL) instead of MariaDB. It is not fixed and this is the patch below. Regards Alangi Derick Ndimnain

1 0

[Maria-developers] Please review: MDEV-8433 Make field<'broken-string' use indexes
by Alexander Barkov 20 Aug '15

20 Aug '15

Hi Sergei, Please review my patch for MDEV-8433. I had doubts about one detail: how to make Field::store() that we're currently creating a value for range search rather than regular doing a INSERT/UPDATE. I could think of two options: 1. Using sql_mode, similar to how temporal types pass MODE_INVALID_DATES. 2. Using a new parameter to this method: int Field::store(const char *str, uint length, CHARSET_INFO *cs); The former needed less changes, so I chose to use sql_mode. But probably N2 would be better. Please suggest. Thanks.

1 0

Re: [Maria-developers] [JIRA] (MDEV-7063) Split String into logical components
by Alexander Barkov 20 Aug '15

20 Aug '15

Hi Rasmus, I noticed you put this task on Sprint 10.1.7-2. Did I miss some discussion about this? From my understanding, Sergei and Monty were not enthusiastic about this task (but I still think it should be helpful). Thanks. On 08/19/2015 06:26 PM, Rasmus Johansson (JIRA) wrote: > > [ https://mariadb.atlassian.net/browse/MDEV-7063?page=com.atlassian.jira.plug… ] > > Rasmus Johansson updated MDEV-7063: > ----------------------------------- > Sprint: 10.1.7-2 > >> Split String into logical components >> ------------------------------------ >> >> Key: MDEV-7063 >> URL: https://mariadb.atlassian.net/browse/MDEV-7063 >> Project: MariaDB Server >> Issue Type: Task >> Reporter: Alexander Barkov >> Assignee: Michael Widenius >> Fix For: 10.1 >> >> >> The string class currently consists of the following components: >> - memory allocation routines (using mysys) >> - binary string routines >> - character set aware routines. >> Storage engines (e.g. ConnectSE) often need String-alike classes, but cannot use the current String implementation because of their own memory management system. So they have to reimplement the same set of functionality, but with their own memory managers. This is bug prone and makes duplicate code. >> Also, in some cases it is useful to have character-set unaware strings (just array of bytes), and static strings without allocation. >> This task is to split String into logical pieces: >> 1. class Static_binary_string >> 2. class Charset >> 3. class Static_character_string (#1 + #2) >> 4. class Memory_mysys >> 5. template<typename MEM> class String_with_alloc >> 6. class String (#3 + #5) >> ConnectSE will use #5 (templace class String_with_alloc), in combination with their own memory allocator. > > > > -- > This message was sent by Atlassian JIRA > (v7.0.0-OD-01-193#70101) >

1 0

Re: [Maria-developers] MDEV-8389 patch review
by Sergey Petrunia 19 Aug '15

19 Aug '15

Also, I'm getting a crash when trying to debug a slightly complex example: delimiter | CREATE FUNCTION f10(param1 VARCHAR(11)) RETURNS TABLE return_table(name VARCHAR(11)) deterministic BEGIN insert into return_table values('foo'); END| DELIMITER ; select name from f21('aaa') where name like '%foo%'; Causes a crash: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff7efa700 (LWP 5142)] 0x0000555555cb6000 in Item_field::fix_fields (this=0x7fff80006db8, thd=0x555558161ca0, reference=0x7fff80007070) at /home/psergey/dev-git/10.1-gsoc-repo/sql/item.cc:4928 (gdb) wher #0 0x0000555555cb6000 in Item_field::fix_fields (this=0x7fff80006db8, thd=0x555558161ca0, reference=0x7fff80007070) at /home/psergey/dev-git/10.1-gsoc-repo/sql/item.cc:4928 #1 0x0000555555cf86e8 in Item_func::fix_fields (this=0x7fff80006fe0, thd=0x555558161ca0, ref=0x7fff8000b060) at /home/psergey/dev-git/10.1-gsoc-repo/sql/item_func.cc:208 #2 0x0000555555cd9747 in Item_func_like::fix_fields (this=0x7fff80006fe0, thd=0x555558161ca0, ref=0x7fff8000b060) at /home/psergey/dev-git/10.1-gsoc-repo/sql/item_cmpfunc.cc:4856 #3 0x00005555559ff1b0 in setup_conds (thd=0x555558161ca0, tables=0x7fff80006780, leaves=..., conds=0x7fff8000b060) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_base.cc:8657 #4 0x0000555555aeb4f7 in setup_without_group (thd=0x555558161ca0, ref_pointer_array=0x7fff8000b198, tables=0x7fff80006780, leaves=..., fields=..., all_fields=..., conds=0x7fff8000b060, order=0x0, group=0x0, hidden_group_fields=0x7fff8000af40, reserved=0x555558165ffc) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:631 #5 0x0000555555aa6d99 in JOIN::prepare (this=0x7fff8000ac48, rref_pointer_array=0x555558165fd8, tables_init=0x7fff80006780, wild_num=0, conds_init=0x7fff80006fe0, og_num=0, order_init=0x0, skip_order_by=false, group_init=0x0, having_init=0x0, proc_param_init=0x0, select_lex_arg=0x555558165d60, unit_arg=0x555558165660) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:786 #6 0x0000555555aafcab in mysql_select (thd=0x555558161ca0, rref_pointer_array=0x555558165fd8, tables=0x7fff80006780, wild_num=0, fields=..., conds=0x7fff80006fe0, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0, select_options=2147748608, result=0x7fff8000ac28, unit=0x555558165660, select_lex=0x555558165d60) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:3301 #7 0x0000555555aa5fd7 in handle_select (thd=0x555558161ca0, lex=0x555558165598, result=0x7fff8000ac28, setup_tables_done_option=0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_select.cc:371 #8 0x0000555555a65fbf in execute_sqlcom_select (thd=0x555558161ca0, all_tables=0x7fff80006780) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:5799 #9 0x0000555555a5c25f in mysql_execute_command (thd=0x555558161ca0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:2939 #10 0x0000555555a69554 in mysql_parse (thd=0x555558161ca0, rawbuf=0x7fff800064a8 "select name from f21('aaa') where name like '%foo%'", length=51, parser_state=0x7ffff7ef90f0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:7175 #11 0x0000555555a584a7 in dispatch_command (command=COM_QUERY, thd=0x555558161ca0, packet=0x555558168ef1 "select name from f21('aaa') where name like '%foo%'", packet_length=51) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:1470 #12 0x0000555555a571c1 in do_command (thd=0x555558161ca0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_parse.cc:1093 #13 0x0000555555b95b3d in do_handle_one_connection (thd_arg=0x555558161ca0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_connect.cc:1347 #14 0x0000555555b95882 in handle_one_connection (arg=0x555558161ca0) at /home/psergey/dev-git/10.1-gsoc-repo/sql/sql_connect.cc:1258 #15 0x00007ffff691de9a in start_thread (arg=0x7ffff7efa700) at pthread_create.c:308 #16 0x00007ffff604e3fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #17 0x0000000000000000 in ?? () (gdb) I am nut sure about the cause of this. It needs to be investigated. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] MDEV-8389 patch review
by Sergey Petrunia 19 Aug '15

19 Aug '15

On Wed, Aug 12, 2015 at 01:58:40AM +0530, Diwas Joshi wrote: > hello sergei, > After I mailed you the last patch, I kept working further. I guess the code > has come to a point where it is giving some results. So, I did the > following things: > Earlier we were storing all the parameters in a select query inside > lex->value_list, but since there can be multiple functions in a query, I > declared a list for parameters inside table_list so we can store params > corresponding to each table function in it. Good. > Then instead of function name i created a table with table alias defined in > the function and change the table name to table alias inside select query > as well. This makes it possible for the queries inside the function > definition to access the temporary table we created. There is a problem > with this approach that there might already be a table present in the > database with that name, I am still thinking of a solution for it but we > can discuss a solution for it later. Yes, you're right. There is a problem here. It is not limited to the already existing tables with the same name. Let's work with this example: delimiter | CREATE FUNCTION f10(param1 VARCHAR(11)) RETURNS TABLE return_table(name VARCHAR(11)) deterministic BEGIN insert into return_table values('foo'); END| DELIMITER ; MariaDB [test]> select name from f10('aaa'); +------+ | name | +------+ | foo | +------+ 1 row in set (0.29 sec) Good. However, attempts to invoke the same function multiple times fail: MariaDB [test]> select name from f10('aaa') as TBL1 , f10('bbb') as TBL2; ERROR 1050 (42S01): Table 'return_table' already exists Your code uses this approach: 1. Create a temporary table in a manner similar to how user-specified CREATE TEMPORARY TABLE works. 2. Rely on regular MySQL's name resolution process to "see" the temporary table. The properties of this solution are: - If a temporary table has the same name as a regular db table, temporary table will "shadow" the regular table (this is ok). - user-created temporary tables are per-thread. Other threads do not see the temp. table (good). However, everything that executes within the thread will see the temp. table. For example, if a query calls two Table Functions, they will both see each others return tables. This is a problem. I think, a better approach would be: - switch to something like create_tmp_table() for creation of temporary tables. Tables that are created this way are normally not visible to the name resolution code. - Patch the name resolution code so that the statements inside a Table Function see the return table of that Table Function. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] MDEV-8389 patch review
by Sergey Petrunia 19 Aug '15

19 Aug '15

Hi Diwas, While playing around, I've found and filed: https://mariadb.atlassian.net/browse/MDEV-8649 https://mariadb.atlassian.net/browse/MDEV-8650 I've filed these separately because I think they are not related to MDEV-8389. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0