Comment 3 for bug 721611

Revision history for this message
Baron Schwartz (baron-xaprb) wrote : Re: [Bug 721611] Re: Change XtraDB variables in 5.5

My comments are inline. In general, I propose names that users will
understand, which express the behavior of the server. I think that
this is better than names that developers who know the InnoDB source
will understand, or names which express an operation on an internal
data structure.

>> INNODB_AUTO_LRU_DUMP
>> to
>> innodb_enable_fast_warmup
>
> fast? warmup?
> I don't like to cheat users. It is really not concrete name.
> The name should express the effect cleanly.

Users will not understand auto_lru_dump. The effect of the variable
is that the server becomes ready to answer queries more quickly after
starting. This is what users will understand and care about.
However, I think you are correct. There could be many different ways
to make the server warm up more quickly, and we should name them
specifically. I think that we might consider a name such as
"innodb_buffer_pool_save_restore". It is also a bad name, but
"buffer_pool" is much better than "lru" for users. (We should also
not use "dump" because it has a slightly different and unprofessional
meaning in English, and the behavior is not only to save the LRU, it
is also to restore it from the saved information.)

>> INNODB_EXPAND_IMPORT
>> to
>> innodb_enable_import_tables
>
> "IMPORT TABLE" is supported basically. It is wrong name.

I agree. But I do not have a better suggestion. The current variable
name is very cryptic. It does not explain what the change in server
behavior is. Maybe we need a variable that explains "when importing a
tablespace, also do ________" ? What, exactly, does the variable
change? I think it renames the tablespace ID, right? I do not know
this feature's behavior very well.

>> INNODB_OVERWRITE_RELAY_LOG_INFO
>> to
>> innodb_provide_replication_position
>
> I don't like to cheat users. It is really not concrete name.
> The name should express the effect cleanly.

The behavior of the server is that after a crash, replication gets its
position information from the storage engine, not the relay log file.
The word "overwrite" sounds like something bad in English. It means
that some data is being destroyed. I propose
"innodb_recovery_update_relay_log". This does not seem perfect, but
it is better.

>> INNODB_PASS_CORRUPT_TABLE
>> Rename to innodb_stop_on_corrupt_table=0|1 ( default is 1)
>
> The user should understand passing corrupt table is abnormal for RDBMS...
> It may not the problem only about the table.
> ,as I described before.
>
> InnoDB should abort for all corruption detections.
>
> You should explain the abnormal name for me, again.
> You didn't explain any at the previous report.
> I don't accept one-side intrusion...

I do not propose to change the behavior. By default, InnoDB should
abort, as you say. But the word "pass" is very ambiguous in English,
and the meaning will not be clear to users. I propose
"innodb_abort_on_corrupt_table=0|1", with a default of 1.

>> And next status variables:
>>> INNODB_CHECKPOINT_AGE
>>> INNODB_CHECKPOINT_MAX_AGE
>>> INNODB_CHECKPOINT_TARGET_AGE
>>
>> Should be
>> INNODB_CHECKPOINT_AGE
>> INNODB_CHECKPOINT_AGE_MAX
>> INNODB_CHECKPOINT_AGE_TARGET
>
> I intended that
> values about checkpoint
> "age", "max age" and "target age".

These variables are not important to rename, in my opinion.