List:Summer of Code« Previous MessageNext Message »
From:Sergei Golubchik Date:July 10 2009 2:51pm
Subject:Re: GSoC I_S/P_S storage engine midterm report
View as plain text  
Hi, scut_tang!

On Jul 10, scut_tang wrote:
> Hi, Sergei. Thanks you very much for your midterm review.
> 
> >>3.1 add support for I_S plugins.
> What does "add support for I_S plugins" mean?

"I_S plugins" are plugins that add new I_S table.

see, for example,

http://rpbouman.blogspot.com/2008/02/mysql-information-schema-plugins-best.html
http://rpbouman.blogspot.com/2008/02/reporting-mysql-internals-with.html
http://krow.net/talks/PluggageInformationSchemaVancouver2007.pdf

don't worry too much about it now, but eventually your list of
infoschema tables will need to be modifiable run-time.

> >>3.2 make sure all SELECT queries work - for example, without position
>  >>   and rnd_pos you cannot support filesort (ORDER BY), so there should
>  >>   be some workaround for this case.

> I knew position and rnd_pos are for non-sequential reads, and position
> stores the current position of data file or something else about
> current record, and rnd_pos reads record according by pos stored by
> position. But how do they work for non-sequential reads? I want to
> know more them in depth.
> Do they connect record with its position and sort the records in MySQL
> server?

in filesort, for example, MySQL accumulates the list of pairs {sort key,
position}, sorts this list by the 'sort key', and then retrieves rows by
the stored positions.

that is, for every row MySQL calls position(), copies in a special data
structure this position and the sort key, and later it calls rnd_pos()
with this value of position().
 
> >>select table_schema, table_name from information_schema.tables does
> >>not show your tables - as expected, because you didn't implement
> >>hton->find_files() method, see below.
> I have finished infoschema_find_files now, and it works.
> "SHOW DATABASES" can list INFOSCHEMA.

great!

> But "use INFOSHEMA" does not work, and what functions deal with "use
> database-name"?

Don't worry about it. You cannot really do it from the engine, MySQL
verifies that there's a directory with the same name as the database
name. For INFORMATION_SCHEMA there's a special exception in the code.

We won't touch it, let's live with the fact that "use" doesn't work.
When "INFOSCHEMA" will be renamed to INFORMATION_SCHEMA, "use" will
start working.

In case you're curious, the function is mysql_change_db() that calls
check_db_dir_existence().

> >>SHOW CREATE TABLE INFOSCHEMA.TABLES crashes. not good.
> I added "DBUG_RETURN(HA_ERR_COMMAND)" in ha_infoschema::create.
> It won't crash.

Does it show the correct table structure ?
 
> >>Did you try both static and dynamic builds of your plugin ?
> >>I mean, you declare here that your infoschema can be statically compiled
> >>into the server or build as a shared library and loaded dynamically.
> >>Are you sure that both work ? If you declare both ways, you
> >>need to know that they both work. Otherwise, just keep the static one
> >>and remove MYSQL_PLUGIN_DYNAMIC line if dynamic loading doesn't work.
> >>I don't think that it makes sense to make infoschema dynamically loaded
> anyway,
> >>so feel free to remove MYSQL_PLUGIN_DYNAMIC line.

> I am sorry. I have only tried static builds of the plugin.
> Why "I don't think that it makes sense to make infoschema dynamically loaded
> anyway"?

I see it as an integral part of the server, not as a separate
plugin. And it'll be using server internals (bypassing public API's) so
it'll make little sense to load it run-time and version separately.
 
> >>> +    private:
> >>> +        /** MySQL lock */
> >>> +        THR_LOCK_DATA m_thr_lock;
> >>> +        /** INFORMATION_SCHEMA table share for this handler. */
> >>> +        const infos_table_share *m_table_share;
> 
> >>you don't need it here, store it in TABLE_SHARE::ha_data
> Why? For saving memory? I think it is just a pointer and it looks more clearly.

okay.

TABLE_SHARE::ha_data was specifically added to store storage engine
"shared" data structures. If you want to store it here, fine, do it.

But please also store it in the TABLE_SHARE::ha_data - because MySQL may
open many handlers (many instances of the ha_infoschema class) for the
same table, and the same TABLE_SHARE. Then in ::open you could do

   if (!table->s->ha_data)
     table->s->ha_data= find_table_share(table_share->db.str, ...

   m_table_share= table->s->ha_data;

and you'll avoid going to the list of shares (which is a global array or
a global hash, possibly protected with a mutex) in cases when the value
is already known.

> >>okay. why did you decide to have a separate hierarchy for the I_S tables,
> >>deriving them from infos_table instead of using the handler hierarchy and
> >>deriving them from ha_infoschema ?
>
> I have changed the hierarchy a little.
> I will send the code diff file to you in this weekly report.

Thanks!
 
> >>are you sure you need #define MYSQL_SERVER here ?
> I really do not understand "#define MYSQL_SERVER". Can you explain it? 
> I saw almost every storage engine contain it, so I do this.

if you'll look into mysql_priv.h you'll see that with MYSQL_SERVER
defined you'll see all internal server definitions, without it - only a
subset of it (that we thought should be "fine" for storage engines).

Try without MYSQL_SERVER - if it won't compile, then add it back but
explain in a comment or email why do you need it (what declarations from
mysql_priv.h you need to see, how the compilation fails without
MYSQL_SERVER).
 
> I am sorry for copied so much. My mind is it starts from a skeleton,
> and remove useless functions in the development.

No problem.
For a first prototype it was excellent!

Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring
Thread
GSoC I_S/P_S storage engine midterm reportscut_tang5 Jul