Re: [Maria-developers] [Commits] 334323b: MDEV-8542 - The "aria_recover" variable should be renamed "aria_recover_options"
Hi, Sergey! On Nov 20, Sergey Vojtovich wrote:
revision-id: 334323bb3e5ca973cf239dce21a7d5510407f230 (mariadb-10.1.8-59-g334323b) parent(s): a1ab4314d1f88fa954a774c322709822d7b95344 committer: Sergey Vojtovich timestamp: 2015-11-20 17:36:23 +0400 message:
MDEV-8542 - The "aria_recover" variable should be renamed "aria_recover_options" to match MyISAM
Added aria_recover_options, marked aria_recover as deprecated.
diff --git a/mysql-test/suite/maria/maria-recover-master.opt b/mysql-test/suite/maria/maria-recover-master.opt index 7582a38..976c388 100644 --- a/mysql-test/suite/maria/maria-recover-master.opt +++ b/mysql-test/suite/maria/maria-recover-master.opt @@ -1 +1 @@ ---loose-aria-recover=backup --loose-aria-log-dir-path=$MYSQLTEST_VARDIR/tmp +--loose-aria-recover-options=backup --loose-aria-log-dir-path=$MYSQLTEST_VARDIR/tmp
I'm not going to grep the code to see whether you've renamed all instances of 'aria[-_]recover' - I assume you did that and run tests too.
diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index 2e2aa71..064d759 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -256,6 +256,11 @@ static MYSQL_SYSVAR_ULONG(pagecache_file_hash_size, pagecache_file_hash_size, 512, 128, 16384, 1);
static MYSQL_SYSVAR_SET(recover, maria_recover_options, PLUGIN_VAR_OPCMDARG, + "Deprecated and will be removed in a future release. Please use " + "--aria-recover-options instead.", + NULL, NULL, HA_RECOVER_DEFAULT, &maria_recover_typelib); +
Why? my_getopt does unambigous prefix matching, so --aria-recover will contnue to work after the rename without any explicit deprecated variables. So this one doesn't do any good. In fact, this variable has bad effects, without it 'aria-recov' (or any other unambigous prefix of aria-recover) would continue to work, but with this new deprecated variable 'aria-recov' will no longer be unambigous.
+static MYSQL_SYSVAR_SET(recover_options, maria_recover_options, PLUGIN_VAR_OPCMDARG, "Specifies how corrupted tables should be automatically repaired", NULL, NULL, HA_RECOVER_DEFAULT, &maria_recover_typelib);
Regards, Sergei
Hi Sergei, On Mon, Nov 23, 2015 at 09:00:45AM +0100, Sergei Golubchik wrote:
Hi, Sergey!
On Nov 20, Sergey Vojtovich wrote:
revision-id: 334323bb3e5ca973cf239dce21a7d5510407f230 (mariadb-10.1.8-59-g334323b) parent(s): a1ab4314d1f88fa954a774c322709822d7b95344 committer: Sergey Vojtovich timestamp: 2015-11-20 17:36:23 +0400 message:
MDEV-8542 - The "aria_recover" variable should be renamed "aria_recover_options" to match MyISAM
Added aria_recover_options, marked aria_recover as deprecated.
diff --git a/mysql-test/suite/maria/maria-recover-master.opt b/mysql-test/suite/maria/maria-recover-master.opt index 7582a38..976c388 100644 --- a/mysql-test/suite/maria/maria-recover-master.opt +++ b/mysql-test/suite/maria/maria-recover-master.opt @@ -1 +1 @@ ---loose-aria-recover=backup --loose-aria-log-dir-path=$MYSQLTEST_VARDIR/tmp +--loose-aria-recover-options=backup --loose-aria-log-dir-path=$MYSQLTEST_VARDIR/tmp
I'm not going to grep the code to see whether you've renamed all instances of 'aria[-_]recover' - I assume you did that and run tests too. Yes, I did it all.
diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index 2e2aa71..064d759 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -256,6 +256,11 @@ static MYSQL_SYSVAR_ULONG(pagecache_file_hash_size, pagecache_file_hash_size, 512, 128, 16384, 1);
static MYSQL_SYSVAR_SET(recover, maria_recover_options, PLUGIN_VAR_OPCMDARG, + "Deprecated and will be removed in a future release. Please use " + "--aria-recover-options instead.", + NULL, NULL, HA_RECOVER_DEFAULT, &maria_recover_typelib); +
Why? my_getopt does unambigous prefix matching, so --aria-recover will contnue to work after the rename without any explicit deprecated variables. So this one doesn't do any good.
In fact, this variable has bad effects, without it 'aria-recov' (or any other unambigous prefix of aria-recover) would continue to work, but with this new deprecated variable 'aria-recov' will no longer be unambigous.
You're right. Two questions: - I think MySQL is getting rid of unambiguous prefix matching. I just wonder if we plan to do the same? - If some day we decide to add --aria-recover-something, won't it break --aria-recover? If the above points aren't worthy, then we definitely should just rename this option. Thanks, Sergey
Hi, Sergey! On Nov 23, Sergey Vojtovich wrote: > > > diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc > > > index 2e2aa71..064d759 100644 > > > --- a/storage/maria/ha_maria.cc > > > +++ b/storage/maria/ha_maria.cc > > > @@ -256,6 +256,11 @@ static MYSQL_SYSVAR_ULONG(pagecache_file_hash_size, pagecache_file_hash_size, > > > 512, 128, 16384, 1); > > > > > > static MYSQL_SYSVAR_SET(recover, maria_recover_options, PLUGIN_VAR_OPCMDARG, > > > + "Deprecated and will be removed in a future release. Please use " > > > + "--aria-recover-options instead.", > > > + NULL, NULL, HA_RECOVER_DEFAULT, &maria_recover_typelib); > > > + > > > > Why? my_getopt does unambigous prefix matching, so --aria-recover > > will contnue to work after the rename without any explicit > > deprecated variables. So this one doesn't do any good. > > > > In fact, this variable has bad effects, without it 'aria-recov' (or > > any other unambigous prefix of aria-recover) would continue to work, > > but with this new deprecated variable 'aria-recov' will no longer be > > unambigous. > You're right. Two questions: > - I think MySQL is getting rid of unambiguous prefix matching. I just > wonder if we plan to do the same? We might. In 10.1 I've added an option --getopt-prefix-matching and in mysql-test we use --disable-getopt-prefix-matching. But by default it's still enabled. > - If some day we decide to add --aria-recover-something, won't it break > --aria-recover? Yes, of course. That's why 5.7 disabled unambiguous prefix matching and that's why we have --disable-getopt-prefix-matching. On the other hand, I don't think we should break aria-recover now just because we might perhaps eventually break it some day in the future :) > If the above points aren't worthy, then we definitely should just > rename this option. I think "just rename the option" is a good approach here Regards, Sergei
participants (2)
-
Sergei Golubchik -
Sergey Vojtovich