← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 334323b: MDEV-8542 - The "aria_recover" variable should be renamed "aria_recover_options"

 

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


Follow ups

References