- developers - lists.mariadb.org

[Maria-developers] InnoDB: Instant ADD COLUMN that works on old data files
by Marko Mäkelä 11 Jan '17

11 Jan '17

I would like to seek technical review and feedback for an idea. In the long term, I would like InnoDB to support instant ALTER TABLE for a larger set of operations: https://jira.mariadb.org/browse/MDEV-11424 Instant ALTER TABLE of failure-free record format changes Probably one of the most common operations is ADD COLUMN. For that, a ticket had already been filed and some patches exist: https://jira.mariadb.org/browse/MDEV-11369 Instant add column for InnoDB Tencent and Alibaba have ADD COLUMN patches that basically introduce new InnoDB record formats so that each record will carry the number of columns, similar to how ROW_FORMAT=REDUNDANT (the original InnoDB row format) works. A problem with that approach is that we would only be able to support instant ADD COLUMN on data files that already are in a suitable format. The whole purpose of instant ALTER TABLE is to avoid conversions of huge data files. Requiring a conversion before the feature can be used would in my opinion defeat the purpose. I came up with a solution that only requires some changes to the InnoDB page format, not to the record formats: Require each record in a clustered index leaf page to contain the same number of fields (on write, convert the whole page). We can repurpose the previously unused (always 0) field PAGE_MAX_TRX_ID on clustered index leaf pages so that the number of fields in each leaf page can be determined. This is the idea that I posted in MDEV-11369 last week and edited today. As far as I can tell, it should work on any existing data file, and it would be a step towards the full-blown MDEV-11424 in a distant future version of MariaDB. One challenge with the page-at-a-time conversion is that inserting or updating records into a leaf page may require the page to be split into several pages, especially in the case where multiple CHAR(xxx) NOT NULL columns would need to be materialized as a result of the insert or update. Does this seem to make sense to you? I am looking forward to your feedback. Best regards, Marko -- DON’T MISS M|17 April 11 - 12, 2017 The Conrad Hotel New York City https://m17.mariadb.com/ Marko Mäkelä, Lead Developer InnoDB MariaDB Corporation

2 2

[Maria-developers] Fwd: Total No of events in event group.
by Sachin Setiya 10 Jan '17

10 Jan '17

Hi Kristian! On Tue, Jan 10, 2017 at 4:41 PM, Kristian Nielsen <knielsen(a)knielsen-hq.org> wrote: > Sachin Setiya <sachin.setiya(a)mariadb.com> writes: > >> Can we do something like send total no of Rows_log_events in a event >> group to slave. We can write this info in Gtid_log_event. Reason for >> doing this is that on slave we can show how much progress we have >> made. > > My immediate impression is that this sounds like a bad idea. > > It increases the binlog size without adding any real information - after all > the event count can be determined easily by just reading the events. > You mean counting the all events in event group without applying them. > It also requires careful design and testing for impact on forwards- and > backwards compatibility. > > And event count is not a very meaningful progress indicator. One event group > could have 10 Rows_log_event each with a single row operation. While another > could have a single Rows_log_event that contains a hundred row operations. > Okay, So it there a better way to show progress in slave ? > And why only rows event? What about Query_log_events in a transaction in > statement-based replication? We can generalize this idea to Query_log_events also. > > - Kristian. Regards sachin

2 3

[Maria-developers] Total No of events in event group.
by Sachin Setiya 10 Jan '17

10 Jan '17

Hi Kristian! Can we do something like send total no of Rows_log_events in a event group to slave. We can write this info in Gtid_log_event. Reason for doing this is that on slave we can show how much progress we have made. -- Regards sachin

2 1

[Maria-developers] Status of MDEV-4557 / 7409
by Sachin Setiya 08 Jan '17

08 Jan '17

Hi Daniel! I was thinking of taking 4557 , but in comment I see you have ported the patch for 10.0 but it was not merged. Are you going to take 4557 , or can I take it ? -- Regards Sachin Setiya Software Engineer at MariaDB

2 1

[Maria-developers] Mdev-10664 Add statuses about optimistic parallel replication stalls.
by Sachin Setiya 08 Jan '17

08 Jan '17

Hi Kristian, I am stuck at one problem in Mdev-10664. Suppose there is multisource replication('master1' and 'master2' ) and we want to update status var, How to know which master_info to update ?. Does slave threads have current replication channel name? (I was not able to find any , I have looked in rpl_group_info) I am attaching patch file, only above problem need to solved in patch. -- Regards Sachin Setiya Software Engineer at MariaDB

3 5

Re: [Maria-developers] Review for MDEV-10332
by Sergei Golubchik 29 Dec '16

29 Dec '16

Hi, Georg! First, I've looked a bit at EVP_CIPHER_CTX_new/etc. With the CRYPTO_set_mem_functions() one can overwrite malloc/realloc/free functions that OpenSSL is using. We absolutely should do it. At the very least, OpenSSL should use my_malloc/my_realloc/my_free. But a more interesting approach would be to allocate a buffer on the stack and let our openssl m/r/f helpers take the memory from there. And print a warning with a fallback to malloc if that buffer isn't enough. This way openssl won't need mallocs is most cases. Still, I'd suggest to do this in a separate commit. In your main "OpenSSL 1.1 support" commit, just use CRYPTO_set_mem_functions to put OpenSSL through my_malloc/my_realloc/my_free. On Nov 15, Georg Richter wrote: > From 6103b311c4c5740998d342afbba9ac70f8ec5731 Mon Sep 17 00:00:00 2001 > From: Georg Richter <georg(a)mariadb.com> > Date: Tue, 15 Nov 2016 18:39:54 +0100 > Subject: [PATCH] MDEV-10332: Added support for OpenSSL 1.1 > > diff --git a/mysql-test/r/openssl_6975,tlsv10.result b/mysql-test/r/openssl_6975,tlsv10.result > index a65167f..e216ec2 100644 > --- a/mysql-test/r/openssl_6975,tlsv10.result > +++ b/mysql-test/r/openssl_6975,tlsv10.result > @@ -1,11 +1,11 @@ > create user ssl_sslv3@localhost; > -grant select on test.* to ssl_sslv3@localhost require cipher "RC4-SHA"; > +grant select on test.* to ssl_sslv3@localhost require cipher "DES-CBC3-SHA"; did you verify that after your changes all tests also work with older openssl versions and with yassl? > create user ssl_tls12@localhost; > grant select on test.* to ssl_tls12@localhost require cipher "AES128-SHA256"; > TLS1.2 ciphers: user is ok with any cipher > ERROR 2026 (HY000): SSL connection error: sslv3 alert handshake failure > diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc > index 49bd9af..d72bc72 100644 > --- a/mysys_ssl/my_crypt.cc > +++ b/mysys_ssl/my_crypt.cc > @@ -36,9 +36,20 @@ > class MyCTX > { > public: > - EVP_CIPHER_CTX ctx; > - MyCTX() { EVP_CIPHER_CTX_init(&ctx); } > - virtual ~MyCTX() { EVP_CIPHER_CTX_cleanup(&ctx); ERR_remove_state(0); } > + EVP_CIPHER_CTX *ctx; > +#if defined(HAVE_YASSL) > + MyCTX() { ctx= new EVP_CIPHER_CTX; EVP_CIPHER_CTX_init(ctx); } > + virtual ~MyCTX() { EVP_CIPHER_CTX_cleanup(ctx); delete ctx; ERR_remove_state(0); } > +#else > + MyCTX() {ctx= EVP_CIPHER_CTX_new(); EVP_CIPHER_CTX_init(ctx); } > + virtual ~MyCTX() { EVP_CIPHER_CTX_free(ctx); > +#if OPENSSL_VERSION_NUMBER < 0x10100000L > + ERR_remove_state(0); > +#else > + ERR_clear_error(); > +#endif > + } > +#endif No #ifdef's here, please. YaSSL-specific code from mysys_ssl/yassl.cc makes yassl to look sufficiently like openssl, so that mysys_ssl/my_crypt.cc wouldn't need any ifdefs. Please, keep it that way. And, certainly, don't put any #if's that check openssl version in the code, keep them in the headers or in the beginning of the file (there is, already #define ERR_remove_state(X) there) > virtual int init(const EVP_CIPHER *cipher, int encrypt, const uchar *key, > uint klen, const uchar *iv, uint ivlen) > @@ -72,7 +83,10 @@ class MyCTX_nopad : public MyCTX > { > public: > const uchar *key; > + const uchar *src; > int klen; > + uchar oiv[90]; 90 bytes for an IV? > + uint extra_bytes; > > MyCTX_nopad() : MyCTX() { } > ~MyCTX_nopad() { } > @@ -285,7 +310,11 @@ int my_random_bytes(uchar *buf, int num) > instead of whatever random engine is currently set in OpenSSL. That way > we are guaranteed to have a non-blocking random. > */ > +#if OPENSSL_VERSION_NUMBER < 0x10100000L > RAND_METHOD *rand = RAND_SSLeay(); > +#else > + RAND_METHOD *rand = RAND_OpenSSL(); > +#endif no #if's in the code, please > if (rand == NULL || rand->bytes(buf, num) != 1) > return MY_AES_OPENSSL_ERROR; > return MY_AES_OK; > diff --git a/mysys_ssl/my_md5.cc b/mysys_ssl/my_md5.cc > index 7139ea9..4885033 100644 > --- a/mysys_ssl/my_md5.cc > +++ b/mysys_ssl/my_md5.cc > @@ -58,10 +57,11 @@ static void md5_result(MD5_CONTEXT *context, uchar digest[MD5_HASH_SIZE]) > > #elif defined(HAVE_OPENSSL) > #include <openssl/evp.h> > -typedef EVP_MD_CTX MD5_CONTEXT; > +typedef void MD5_CONTEXT; > > -static void md5_init(MD5_CONTEXT *context) > +static void md5_init(MD5_CONTEXT *ctx) > { > + EVP_MD_CTX *context= (EVP_MD_CTX *)ctx; > EVP_MD_CTX_init(context); > #ifdef EVP_MD_CTX_FLAG_NON_FIPS_ALLOW > /* Ok to ignore FIPS: MD5 is not used for crypto here */ > @@ -123,21 +143,43 @@ void my_md5_multi(uchar *digest, ...) > { > va_list args; > va_start(args, digest); > - > - MD5_CONTEXT md5_context; > +#ifdef HAVE_YASSL > + MD5_CONTEXT *md5_context= new MD5_CONTEXT; > +#else > +#if OPENSSL_VERSION_NUMBER < 0x10100000L > + EVP_MD_CTX *md5_context= (EVP_MD_CTX *)alloca(sizeof(EVP_MD_CTX)); > +#else > + EVP_MD_CTX *md5_context= EVP_MD_CTX_new(); > +#endif > +#endif > const uchar *str; > > - md5_init_fast(&md5_context); > + md5_init_fast(md5_context); > for (str= va_arg(args, const uchar*); str; str= va_arg(args, const uchar*)) > - md5_input(&md5_context, str, va_arg(args, size_t)); > + md5_input(md5_context, str, va_arg(args, size_t)); > > - md5_result(&md5_context, digest); > + md5_result(md5_context, digest); > va_end(args); > +#if defined(HAVE_YASSL) > + delete md5_context; > +#else > +#if OPENSSL_VERSION_NUMBER >= 0x10100000L > + EVP_MD_CTX_free(md5_context); > +#endif > +#endif > } > > size_t my_md5_context_size() > { > +#if defined(HAVE_YASSL) > return sizeof(MD5_CONTEXT); > +#elif OPENSSL_VERSION_NUMBER < 0x10100000L > + return sizeof(EVP_MD_CTX); > +#else > + /* Since OpenSSL 1.1 we can't determine the length of EVP_MD_CTX anymore, > + so we need to return a number (48 bytes + 16 bytes reserved) */ > + return 64; How can you be sure that it's enough? I'd just return sizeof(void*) and in md5_init stored a return value of EVP_MD_CTX_new() there. Until we do that optimization that I mentioned in the beginning of the email. And get rid of #if's please. > +#endif > } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] TRUE LOCK=NONE for slave
by Sachin Setiya 29 Dec '16

29 Dec '16

Hello everyone! I am thinking of implementing true lock=none for slaves. Currently Alter command is only executed on slave when master commits it. I(thanks marko for suggesting this idea) am thinking of writing BEGIN_DDL_EVENT in binlog after master hits ha_prepare_inplace_alter_table. Then master will write QUERY_EVENT on binlog with actual alter query . On commit/rollback master will write COMMIT_DDL_EVENT/ROLLBACK_DDL_EVENT. On slave there will be pool of threads(configurable global variable), which will apply these DDLs. On receiving BEGIN_DDL_EVENT slave thread will pass the QUERY_EVENT to one of the worker thread. Worker thread will execute until ha_inplace_alter_table. Actual commit_inplace_alter will be called by sql thread. If sql thread recieve some kind of rollback event , then it will somehow signal worker thread to stop executing alter. If none of the worker threads are available then event will be en-queued, then If we received rollback event the we will simply discard event from queue, If we received commit event then SQL thread will synchronously process DDL event. Please let me know what you think. Credits for idea Marko and Lixun. Regards sachin

3 3

Re: [Maria-developers] [Commits] d158f55659c: MDEV-11679 Remove redundant function fsp_header_get_crypt_offset()
by Jan Lindström 29 Dec '16

29 Dec '16

On Wed, Dec 28, 2016 at 5:16 PM, <marko.makela(a)mariadb.com> wrote: > > diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/ > fil0crypt.cc > index 75efcdfdab0..095b9f706c0 100644 > --- a/storage/innobase/fil/fil0crypt.cc > +++ b/storage/innobase/fil/fil0crypt.cc > @@ -1179,10 +1179,11 @@ fil_crypt_start_encrypting_space( > > /* 3 - compute location to store crypt data */ > byte* frame = buf_block_get_frame(block); > - ulint maxsize = 0; > ut_ad(crypt_data); > - crypt_data->page0_offset = > - fsp_header_get_crypt_offset(page_size, &maxsize); > + crypt_data->page0_offset = FSP_HEADER_OFFSET > + + fsp_header_get_encryption_offset(page_size); > + const ulint maxsize = page_size.logical() > + - crypt_data->page0_offset - FIL_PAGE_DATA_END; > > This I do not like, why fsp_header_get_encryption_offset does not calculate the actual offset so that there is no need for adjustments? Now every place where you call this you need to add FSP_HEADER_OFFSET to returned offset. Similarly for maxsize actually meaning free space on the page. This is naturally technically correct, but is it maintainable? R: Jan

1 0

Re: [Maria-developers] e687577: MDEV-10340: support COM_RESET_CONNECTION
by Sergei Golubchik 28 Dec '16

28 Dec '16

Hi, Oleksandr! On Dec 12, Oleksandr Byelkin wrote: > revision-id: e6875772b0d65a6341456a6f4aca3353bbdae3e4 (mariadb-10.2.2-131-ge687577) > parent(s): 7ca1e2abad42a7436e6b668b4568d6fadc2ca165 > committer: Oleksandr Byelkin > timestamp: 2016-12-12 14:41:45 +0100 > message: > > MDEV-10340: support COM_RESET_CONNECTION > > draft to check with client > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index a2a4ed2..b760aeb 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -1679,6 +1680,13 @@ bool dispatch_command(enum enum_server_command command, THD *thd, > break; > } > #endif > + case COM_RESET_CONNECTION: > + { > + thd->status_var.com_other++; > + thd->change_user(); > + my_ok(thd, 0, 0, 0); > + break; > + } Looks ok, but I suspect that, just like for COM_CHANGE_USER, you might need here: thd->clear_error(); // if errors from rollback ok to push with the change as above. But please make sure that C/C part is pushed too Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 33b01257190: MDEV-11584: GRANT inside an SP does not work well on 2nd execution
by Sergei Golubchik 28 Dec '16

28 Dec '16

Hi, Oleksandr! On Dec 21, Oleksandr Byelkin wrote: > revision-id: 33b01257190cef414a4b0f00bf36ee6a3ceda0a7 (mariadb-10.1.20-6-g33b01257190) > parent(s): 9e032d6150b39cf36a22e7c1503e06fae9c4016d > committer: Oleksandr Byelkin > timestamp: 2016-12-21 20:11:14 +0100 > message: > > MDEV-11584: GRANT inside an SP does not work well on 2nd execution > > Allocate password hash in statment memory > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 3f26bc2e08a..b44b120cf04 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -1067,7 +1067,14 @@ static bool fix_lex_user(THD *thd, LEX_USER *user) > make_scramble= my_make_scrambled_password; > } > > - char *buff= (char *) thd->alloc(scramble_length + 1); > + char *buff; > + { > + Query_arena *arena, backup; > + arena= thd->activate_stmt_arena_if_needed(&backup); > + buff= (char *) thd->alloc(scramble_length + 1); > + if (arena) > + thd->restore_active_arena(arena, &backup); > + } Ok to push, just remove the unnecessary pair of curly braces and the indentation. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0