List:Internals« Previous MessageNext Message »
From:Guilhem Bichot Date:June 13 2007 4:35pm
Subject:Re: CREATE INDEX is not committed?
View as plain text  
Hello,

On Wed, Jun 13, 2007 at 03:56:34PM +0300, Marko Mäkelä wrote:
> On Wed, Jun 13, 2007 at 02:27:56PM +0200, Guilhem Bichot wrote:
> > On Wed, Jun 13, 2007 at 12:05:32PM +0300, Marko Mäkelä wrote:
> > > I wonder if the following is correct behavior.  This is from
> > > innodb.test:
> > > 
> > > create table t1 (i int, j int ) ENGINE=innodb;
> > > insert into t1 values (1,2);
> > > select * from t1 where i=1 and j=2;
> > > create index ax1 on t1 (i,j);
> > > select * from t1 where i=1 and j=2;
> > > 
> > > If I set a breakpoint on innobase_commit, it gets invoked for the
> > > INSERT, but not for the CREATE TABLE or CREATE INDEX statements.
> > > 
> > > I have the impression that this behavior has been changed in the past
> > > few weeks.
> > > 
> > > The documentation of CREATE INDEX doesn't mention transactions.  I
> > > suppose that InnoDB is not the only transactional engine where data
> > > dictionary operations are not truly transactional.  Is it OK for the
> > > engine to commit the transaction when it feels like that, or should
> > > MySQL instead invoke commit?
> > 
> > Strange, I see innobase_commit called by this CREATE INDEX both in 5.1
> > and 5.0. There is a ha_commit_stmt() in copy_data_between_tables().
> > Which version and which source tree are you on?
> 
> You're right, innobase_commit is indeed called for CREATE INDEX in 5.1,
> more specifically, in this revision:
> 
> ChangeSet@stripped, 2007-06-07 16:37:15+02:00, joerg@trift2. +8 -0
>   Merge trift2.:/MySQL/M50/push-5.0
>   into  trift2.:/MySQL/M51/push-5.1
>   MERGE: 1.1810.2945.41
> 
> However, I should have said that I have been debugging fast index creation
> that we are planning to release as a 5.1 plugin.  There, innobase_commit is
> called *before* ha_innobase::add_index() but not after it.  If I remove
> the call to innobase_commit_low() at the end of that function, many tests
> will fail.

Strange. I'm looking at sql_table.cc in the latest 5.1 bk tree,
mysql_alter_table(), and a few lines after it calls
table->file->add_index(table, key_info, index_add_count)) there is:

    DBUG_PRINT("info", ("Committing before unlocking table"));
    if (ha_commit_stmt(thd) || ha_commit(thd))
      goto err1;
    committed= 1;

Could you please check that:
- you indeed come to the line above (ha_commit etc)
- it then calls innobase_commit() ? (it might be that InnoDB is not
registered as a participant of the "transaction")

> I'm sorry for misleading you.  I didn't enable the breakpoint on
> innobase_commit() until after ha_innobase::add_index() was called.

no problem.

> I wonder what the correct behavior is.  I would expect MySQL to commit
> before and after handler::add_index().

After: I agree. Before: why?
Thread
CREATE INDEX is not committed?Marko Mäkelä13 Jun
  • Re: CREATE INDEX is not committed?Guilhem Bichot13 Jun
    • Re: CREATE INDEX is not committed?Marko Mäkelä13 Jun
      • Re: CREATE INDEX is not committed?Guilhem Bichot13 Jun
        • Re: CREATE INDEX is not committed?Marko Mäkelä18 Jun
          • Re: CREATE INDEX is not committed?Marko Mäkelä18 Jun
            • Re: CREATE INDEX is not committed?Guilhem Bichot18 Jun
              • Re: CREATE INDEX is not committed?Marko Mäkelä20 Jun
                • Re: CREATE INDEX is not committed?Ramil Kalimullin20 Jun
                  • Re: CREATE INDEX is not committed?Marko Mäkelä20 Jun
          • Re: CREATE INDEX is not committed?Guilhem Bichot18 Jun
Re: CREATE INDEX is not committed?Marko Mäkelä21 Jun
  • Smart ALTER TABLE dropping non-existing tmp table (Re: CREATEINDEX is not committed?)Marko Mäkelä13 Aug