[Maria-developers] request for feedback on live schema changes patch
Hello all, In another thread, there was a discussion about what it would take to allow the storage engine to support live schema changes in mysql and mariadb. I said that I was working porting this existing feature from MySQL Cluster to MySQL 5.1.46. I would like to show the work I have done so far and solicit feedback. Attached is the diff based off of 5.1.46. Before looking at the patch, please be aware of the following things: - I am not an expert in this code. As a result, I had to use an approach of that minimizes the risk of introducing regressions. \ - My main interest is to get something safe that works and can be integrated in 5.1. As a result, kept the current mysql_alter_table functionality in place (as old_mysql_alter_table), and added MySQL Cluster's as new_mysql_alter_table. For an engine to get new_mysql_alter_table to be used, the engine must expose an alter table flag HA_GENERAL_ONLINE. - Another reason why I kept old_mysql_alter_table was that I did not want to deal with engines that implement handler::add_index and make sure that they work properly under the new API. Also, I did not want to port alter table changes related to partitioned tables. - I have run the current 5,1 mysql test suite (that is in the mysql-test directory of source tarballs), and verified that using old_mysql_alter_table and new_mysql_alter_table works fine with them. That being said, I would love to hear feedback on this patch, and would also like to understand what (if anything) can be done to get this into MariaDB 5.1. I realize that may not be possible, but if it is, I would like to try. Thanks -Zardosht
Zardosht Kasheff <zardosht@gmail.com> writes:
That being said, I would love to hear feedback on this patch, and would also like to understand what (if anything) can be done to get this into MariaDB 5.1. I realize that may not be possible, but if it is, I would like to try.
I didn't look at the patch yet. However, I would oppose putting this into MariaDB if it is a hodge-podge mixture of the old and the new alter table code. (Just imagine if the existing code you had to work in had already contained 4 versions of alter table code, from MySQL 3.x, MySQL 4.0, MySQL 4.1, and MySQL 5.1, say. I'm sure you found the task sufficiently tricky with just one version to understand). On the other hand, it might make good sense to support the old interface in storage engines, for example by implementing a backward-compatible emulation of the old API on top of the new alter table code. How is it done in the original Cluster tree that you based your work on? Does it convert all storage engines to the new interface? Does it implement backwards compatibility on top of the new interface, leaving (some) storage engines unmodified? Or does it contain both the old and the new code, like your patch? - Kristian.
I did it this way for two reasons: - I did not want to try to port partitioning's alter table changes. That was getting into a realm where I could easily mess up - I was concerned with implementing the backwards-compatible emulation of the old API. I figured i would do it this way for 5.1, and then future versions that do not need to be instantly GA-ready could either deprecate some of the old interface or have a backward compatible emulation. So, I agree with what you state below about having one alter table path, but I figured the safe way to do that would be for future versions that are not currently GA. Any thoughts on this? The cluster code does not support the following alter table flags (they do not even exist): #define HA_ONLINE_ADD_INDEX_NO_WRITES (1L << 0) /*add index w/lock*/ #define HA_ONLINE_DROP_INDEX_NO_WRITES (1L << 1) /*drop index w/lock*/ #define HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES (1L << 2) /*add unique w/lock*/ #define HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES (1L << 3) /*drop uniq. w/lock*/ #define HA_ONLINE_ADD_PK_INDEX_NO_WRITES (1L << 4) /*add prim. w/lock*/ #define HA_ONLINE_DROP_PK_INDEX_NO_WRITES (1L << 5) /*drop prim. w/lock*/ #define HA_ONLINE_ADD_INDEX (1L << 6) /*add index online*/ #define HA_ONLINE_DROP_INDEX (1L << 7) /*drop index online*/ #define HA_ONLINE_ADD_UNIQUE_INDEX (1L << 8) /*add unique online*/ #define HA_ONLINE_DROP_UNIQUE_INDEX (1L << 9) /*drop uniq. online*/ #define HA_ONLINE_ADD_PK_INDEX (1L << 10)/*add prim. online*/ #define HA_ONLINE_DROP_PK_INDEX (1L << 11)/*drop prim. online*/ So, what would need to happen is for the default implementations of the new interface to check for and support these flags. -Zarodsht On Fri, Oct 1, 2010 at 3:44 AM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Zardosht Kasheff <zardosht@gmail.com> writes:
That being said, I would love to hear feedback on this patch, and would also like to understand what (if anything) can be done to get this into MariaDB 5.1. I realize that may not be possible, but if it is, I would like to try.
I didn't look at the patch yet. However, I would oppose putting this into MariaDB if it is a hodge-podge mixture of the old and the new alter table code.
(Just imagine if the existing code you had to work in had already contained 4 versions of alter table code, from MySQL 3.x, MySQL 4.0, MySQL 4.1, and MySQL 5.1, say. I'm sure you found the task sufficiently tricky with just one version to understand).
On the other hand, it might make good sense to support the old interface in storage engines, for example by implementing a backward-compatible emulation of the old API on top of the new alter table code.
How is it done in the original Cluster tree that you based your work on? Does it convert all storage engines to the new interface? Does it implement backwards compatibility on top of the new interface, leaving (some) storage engines unmodified? Or does it contain both the old and the new code, like your patch?
- Kristian.
Hello, I am wondering if we can resurrect this discussion. In summary, I have a patch that I think I can apply to 5.1, although I realize it is not the ideal solution. It is a solution that I could code that minimizes the risk of introducing a regression for storage engines that do not use this. The fix is basically to have a new alter table for storage engines that want this feature, and the old alter table for others. The two biggest reasons I did it this way was: - I did not want to complicate the fix by handling storage engines that implement handler::add_index. - I did not understand the partitioning fixes required. That being said, what do people think are the next steps that should be taken? I would love to see this feature in MariaDB 5.3, but I do not have the expertise to be able to complete the feature. I am just moving code from one MySQL branch to another. Thanks -Zardosht On Fri, Oct 1, 2010 at 8:59 AM, Zardosht Kasheff <zardosht@gmail.com> wrote:
I did it this way for two reasons: - I did not want to try to port partitioning's alter table changes. That was getting into a realm where I could easily mess up - I was concerned with implementing the backwards-compatible emulation of the old API. I figured i would do it this way for 5.1, and then future versions that do not need to be instantly GA-ready could either deprecate some of the old interface or have a backward compatible emulation.
So, I agree with what you state below about having one alter table path, but I figured the safe way to do that would be for future versions that are not currently GA. Any thoughts on this?
The cluster code does not support the following alter table flags (they do not even exist):
#define HA_ONLINE_ADD_INDEX_NO_WRITES (1L << 0) /*add index w/lock*/ #define HA_ONLINE_DROP_INDEX_NO_WRITES (1L << 1) /*drop index w/lock*/ #define HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES (1L << 2) /*add unique w/lock*/ #define HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES (1L << 3) /*drop uniq. w/lock*/ #define HA_ONLINE_ADD_PK_INDEX_NO_WRITES (1L << 4) /*add prim. w/lock*/ #define HA_ONLINE_DROP_PK_INDEX_NO_WRITES (1L << 5) /*drop prim. w/lock*/ #define HA_ONLINE_ADD_INDEX (1L << 6) /*add index online*/ #define HA_ONLINE_DROP_INDEX (1L << 7) /*drop index online*/ #define HA_ONLINE_ADD_UNIQUE_INDEX (1L << 8) /*add unique online*/ #define HA_ONLINE_DROP_UNIQUE_INDEX (1L << 9) /*drop uniq. online*/ #define HA_ONLINE_ADD_PK_INDEX (1L << 10)/*add prim. online*/ #define HA_ONLINE_DROP_PK_INDEX (1L << 11)/*drop prim. online*/
So, what would need to happen is for the default implementations of the new interface to check for and support these flags.
-Zarodsht
On Fri, Oct 1, 2010 at 3:44 AM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Zardosht Kasheff <zardosht@gmail.com> writes:
That being said, I would love to hear feedback on this patch, and would also like to understand what (if anything) can be done to get this into MariaDB 5.1. I realize that may not be possible, but if it is, I would like to try.
I didn't look at the patch yet. However, I would oppose putting this into MariaDB if it is a hodge-podge mixture of the old and the new alter table code.
(Just imagine if the existing code you had to work in had already contained 4 versions of alter table code, from MySQL 3.x, MySQL 4.0, MySQL 4.1, and MySQL 5.1, say. I'm sure you found the task sufficiently tricky with just one version to understand).
On the other hand, it might make good sense to support the old interface in storage engines, for example by implementing a backward-compatible emulation of the old API on top of the new alter table code.
How is it done in the original Cluster tree that you based your work on? Does it convert all storage engines to the new interface? Does it implement backwards compatibility on top of the new interface, leaving (some) storage engines unmodified? Or does it contain both the old and the new code, like your patch?
- Kristian.
Hello all, I am hoping to resurrect this discussion. Has there been any discussion on implementing the feature of allowing handlers to do fast schema changes in MariaDB (on any version)? I have put this patch on MySQL 5.1, and I think I have it working. I started looking at MariaDB 5.2, but I do not understand some of the virtual column stuff and field->option_struct and key->option_struct. I am guessing they are related to the new create table syntax, but I do not understand it well enough to be able to integrate this patch in. Any thoughts? Thanks -Zardosht On Thu, Nov 18, 2010 at 2:53 PM, Zardosht Kasheff <zardosht@gmail.com> wrote:
Hello,
I am wondering if we can resurrect this discussion. In summary, I have a patch that I think I can apply to 5.1, although I realize it is not the ideal solution. It is a solution that I could code that minimizes the risk of introducing a regression for storage engines that do not use this.
The fix is basically to have a new alter table for storage engines that want this feature, and the old alter table for others.
The two biggest reasons I did it this way was: - I did not want to complicate the fix by handling storage engines that implement handler::add_index. - I did not understand the partitioning fixes required.
That being said, what do people think are the next steps that should be taken? I would love to see this feature in MariaDB 5.3, but I do not have the expertise to be able to complete the feature. I am just moving code from one MySQL branch to another.
Thanks -Zardosht
On Fri, Oct 1, 2010 at 8:59 AM, Zardosht Kasheff <zardosht@gmail.com> wrote:
I did it this way for two reasons: - I did not want to try to port partitioning's alter table changes. That was getting into a realm where I could easily mess up - I was concerned with implementing the backwards-compatible emulation of the old API. I figured i would do it this way for 5.1, and then future versions that do not need to be instantly GA-ready could either deprecate some of the old interface or have a backward compatible emulation.
So, I agree with what you state below about having one alter table path, but I figured the safe way to do that would be for future versions that are not currently GA. Any thoughts on this?
The cluster code does not support the following alter table flags (they do not even exist):
#define HA_ONLINE_ADD_INDEX_NO_WRITES (1L << 0) /*add index w/lock*/ #define HA_ONLINE_DROP_INDEX_NO_WRITES (1L << 1) /*drop index w/lock*/ #define HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES (1L << 2) /*add unique w/lock*/ #define HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES (1L << 3) /*drop uniq. w/lock*/ #define HA_ONLINE_ADD_PK_INDEX_NO_WRITES (1L << 4) /*add prim. w/lock*/ #define HA_ONLINE_DROP_PK_INDEX_NO_WRITES (1L << 5) /*drop prim. w/lock*/ #define HA_ONLINE_ADD_INDEX (1L << 6) /*add index online*/ #define HA_ONLINE_DROP_INDEX (1L << 7) /*drop index online*/ #define HA_ONLINE_ADD_UNIQUE_INDEX (1L << 8) /*add unique online*/ #define HA_ONLINE_DROP_UNIQUE_INDEX (1L << 9) /*drop uniq. online*/ #define HA_ONLINE_ADD_PK_INDEX (1L << 10)/*add prim. online*/ #define HA_ONLINE_DROP_PK_INDEX (1L << 11)/*drop prim. online*/
So, what would need to happen is for the default implementations of the new interface to check for and support these flags.
-Zarodsht
On Fri, Oct 1, 2010 at 3:44 AM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Zardosht Kasheff <zardosht@gmail.com> writes:
That being said, I would love to hear feedback on this patch, and would also like to understand what (if anything) can be done to get this into MariaDB 5.1. I realize that may not be possible, but if it is, I would like to try.
I didn't look at the patch yet. However, I would oppose putting this into MariaDB if it is a hodge-podge mixture of the old and the new alter table code.
(Just imagine if the existing code you had to work in had already contained 4 versions of alter table code, from MySQL 3.x, MySQL 4.0, MySQL 4.1, and MySQL 5.1, say. I'm sure you found the task sufficiently tricky with just one version to understand).
On the other hand, it might make good sense to support the old interface in storage engines, for example by implementing a backward-compatible emulation of the old API on top of the new alter table code.
How is it done in the original Cluster tree that you based your work on? Does it convert all storage engines to the new interface? Does it implement backwards compatibility on top of the new interface, leaving (some) storage engines unmodified? Or does it contain both the old and the new code, like your patch?
- Kristian.
Zardosht Kasheff <zardosht@gmail.com> writes:
I am hoping to resurrect this discussion. Has there been any discussion on implementing the feature of allowing handlers to do fast schema changes in MariaDB (on any version)?
I have put this patch on MySQL 5.1, and I think I have it working. I started looking at MariaDB 5.2, but I do not understand some of the virtual column stuff and field->option_struct and key->option_struct. I am guessing they are related to the new create table syntax, but I do not understand it well enough to be able to integrate this patch in.
I am sorry you had to wait for an answer for so long. I was kind of hoping that someone else would step in with an opinion also. As no-one did, I will just have to re-state my opinion. As I understand from the previous discussion, you patch does not attempt to fully integrate the fast schema changes in the source code. Rather, it adds an extra code path, so that the server contains both the old and the new alter table framework, with some flag to select between one or the other. I did not look at the patch, but I believe this makes it unsuable for including in MariaDB, whatever the version. For this, we will need a complete patch with a clean integration that can be maintained going forward. I would really like to see fast/online ALTER TABLE in MariaDB, but it needs to be done correctly. I fully understand that this is hard work. If you have concrete questions/problems I will do my best to try to get them answered, but if the full task is too much, then my opinion is that it will have to wait. - Kristian.
On Thu, Nov 18, 2010 at 2:53 PM, Zardosht Kasheff <zardosht@gmail.com> wrote:
Hello,
I am wondering if we can resurrect this discussion. In summary, I have a patch that I think I can apply to 5.1, although I realize it is not the ideal solution. It is a solution that I could code that minimizes the risk of introducing a regression for storage engines that do not use this.
The fix is basically to have a new alter table for storage engines that want this feature, and the old alter table for others.
The two biggest reasons I did it this way was: - I did not want to complicate the fix by handling storage engines that implement handler::add_index. - I did not understand the partitioning fixes required.
That being said, what do people think are the next steps that should be taken? I would love to see this feature in MariaDB 5.3, but I do not have the expertise to be able to complete the feature. I am just moving code from one MySQL branch to another.
participants (2)
-
Kristian Nielsen -
Zardosht Kasheff