maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Zardosht!

Thanks for your feedback!

I'm looking at the storage engine API from the other side, but my
opinion is mostly the same as yours.

Current API works, but it could be simplified/clarified in few places.

There are part of it that I needed to explain many times. And I was
saying, like, "it's historically so, just accept the way it is" or
something along these lines.

In particular, external_lock/start_stmt - that's, perhaps, the worst
part in this regard.

There's unnecessary boilerplate code - lines that every engine has to
have, and they just copy it from each other. Even if minor, like
incrementing status variables, setting auto-increment and timestamp
columns, table->status, SHARE's. This is getting fixed though.

I don't like the fact that some methods assume that char *buf is
table->record[0]. I'd rather fix it (remove either the assumption or the
method's argument).

It never occurred to me that one can be confuse TABLE::fields and
TABLE_SHARE::fields, but now I see that you're right. TABLE_SHARE should
have kind of a Field_share object, while TABLE - Field_instance.
Currently Field object inclused both, so the shared part is unnecessary
duplicated and the "instance" part is unused in the TABLE_SHARE::fields.

Come to think of it, almost everything I had to explain about storage
engine API needs fixing :) Good parts of the API just work, there's no
need to explain them. So, thanks for asking - it highlights problems!

Besides, we're trying to add more features too. Like engine-specific
attributes in the CREATE TABLE or the new table discovery API.

Regards,
Sergei

On Aug 16, Zardosht Kasheff wrote:
> I've worked on the TokuDB storage engine for quite a while now. I have
> had many experiences over the years, so I guess it's hard to know
> where to begin. I guess I will start small, and if the conversation
> evolves, I can contribute more thoughts. I think the current API is
> really good, as evidenced by the fact that many storage engines have
> used it to plug into MySQL. The two areas that I see we can really
> benefit from are the following:
>  - documentation of the API, discussing clearly what the contracts for
> the functions are. There are still several functions I am not fully
> clear on, but am glad that they "just work". This includes
> documentation on the locking mechanisms. For instance, clear
> documentation on what external_lock, store_lock, and other related
> functions do would be great.
>  - general API cleanup. In too many places, the functions just assume
> things, such as the buffer passed into write_row is the same as
> table->record[0]. In other instances, the well defined way to answer
> certain questions is not obvious. For instance:
>   - If I want information on the fields, do I use the table object or
> table_share?
>   - There seem to be several ways to determine if a field is NULL
>   - There is redundant information in the KEY* structure and the
> fields that make up the KEY. I recall when working on MySQL 5.1, this
> information was inconsistent and it led to bugs, such as MySQL bug
> 37292
>   - I am sure there are plenty of other instances of this that I can find.
>  - Transactions. The API seems to not be designed with transactions
> taken into account. Transactions are started in some cases of
> external_lock, or in start_stmt. Transactions are committed either in
> the handlerton's commit function or in some cases in external_lock. A
> clear API that takes transactions into consideration would be quite
> helpful.
> 
> Those are my early thoughts. I am happy to share more if people are interested.