Bug #60261 Are transaction hints needed in these situation?
Submitted: 26 Feb 2011 0:59 Modified: 12 Nov 2016 22:48
Reporter: Linhai Song Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Cluster: Cluster (NDB) storage engine Severity:S5 (Performance)
Version:mysql-cluster-gpl-7.1.9a OS:Any
Assigned to: CPU Architecture:Any
Tags: performance, transaction hint
Triage: Triaged: D5 (Feature request) / R6 (Needs Assessment) / E6 (Needs Assessment)

[26 Feb 2011 0:59] Linhai Song
Description:
After reading bug report of MysqlBug39268 (http://bugs.mysql.com/bug.php?id=39268), I feel that if we can provide startTransaction() function with a NdbDictionary::Table parameter as hint, we can get a better performance, and we can get the NdbDictionary::Table parameter from the parameter of getNdbOperation() function. 

According to this rule, I check the source code of mysql-cluster-gpl-7.1.9a, and I find some code fragments which are the same situation as mysqlbug39268. I think these code fragments will cause performance problem, and they should be patched in the same way as mysqlbug39268.

The places I find are:

mysql-cluster-gpl-7.1.9a/storage/ndb/tools/restore/consumer_restore.cpp : 917
	  NdbTransaction * trans= m_ndb->startTransaction();
mysql-cluster-gpl-7.1.9a/storage/ndb/tools/restore/consumer_restore.cpp : 924
	  NdbOperation * op= trans->getNdbOperation(ndbtab);

  NdbTransaction * trans= m_ndb->startTransaction();
  if (!trans)
  {
    err << NDB_APPLY_TABLE << ": "
	<< m_ndb->getNdbError() << endl;
    return false;
  }
  NdbOperation * op= trans->getNdbOperation(ndbtab);  

==============================================================================
mysql-cluster-gpl-7.1.9a/storage/ndb/tools/restore/consumer_restore.cpp : 2177
	  NdbTransaction * trans = m_ndb->startTransaction();
mysql-cluster-gpl-7.1.9a/storage/ndb/tools/restore/consumer_restore.cpp : 2191
	  NdbOperation * op = trans->getNdbOperation(table);

  NdbTransaction * trans = m_ndb->startTransaction();
  if (trans == NULL) 
  {
    errobj = m_ndb->getNdbError();
    if (errobj.status == NdbError::TemporaryError)
    {
      goto retry;
    }
    err << "Cannot start transaction: " << errobj << endl;
    exitHandler();
  } // if
  
  TransGuard g(trans);
  const NdbDictionary::Table * table = get_table(tup.m_table->m_dictTable);
  NdbOperation * op = trans->getNdbOperation(table);

===============================================================================
ysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 1732
	    if ((trans= ndb->startTransaction()) == 0)
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 1782
	      r|= (op= trans->getNdbOperation(ndbtab)) == 0;

false positive:
while (1)
  {
    if ((trans= ndb->startTransaction()) == 0)
      goto err;
    {
      NdbOperation *op= 0;
      int r= 0;

      /* read the bitmap exlusive */
      r|= (op= trans->getNdbOperation(ndbtab)) == 0;
      DBUG_ASSERT(r == 0);
      r|= op->readTupleExclusive();
      DBUG_ASSERT(r == 0);
    
      /* db */
      ndb_pack_varchar(col[SCHEMA_DB_I], tmp_buf, db, strlen(db));
      r|= op->equal(SCHEMA_DB_I, tmp_buf);
      DBUG_ASSERT(r == 0);
      /* name */
      ndb_pack_varchar(col[SCHEMA_NAME_I], tmp_buf, table_name,
                       strlen(table_name));
      r|= op->equal(SCHEMA_NAME_I, tmp_buf);
      DBUG_ASSERT(r == 0);
      /* slock */
      r|= op->getValue(SCHEMA_SLOCK_I, (char*)slock.bitmap) == 0;
      DBUG_ASSERT(r == 0);
    }
    if (trans->execute(NdbTransaction::NoCommit))
      goto err;

    if (opt_ndb_extra_logging > 19)
    {
      uint32 copy[SCHEMA_SLOCK_SIZE/4];
      memcpy(copy, bitbuf, sizeof(copy));
      bitmap_clear_bit(&slock, node_id);
      sql_print_information("NDB: reply to %s.%s(%u/%u) from %x%x to %x%x",
                            db, table_name,
                            table_id, table_version,
                            copy[0], copy[1],
                            slock.bitmap[0],
                            slock.bitmap[1]);
    }
    else
    {
      bitmap_clear_bit(&slock, node_id);
    }

    {
      NdbOperation *op= 0;
      int r= 0;

      /* now update the tuple */
      r|= (op= trans->getNdbOperation(ndbtab)) == 0;
      DBUG_ASSERT(r == 0);
      r|= op->updateTuple();
      DBUG_ASSERT(r == 0);

================================================================================
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 1732
	    if ((trans= ndb->startTransaction()) == 0)
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 1739
	      r|= (op= trans->getNdbOperation(ndbtab)) == 0;

while (1)
  {
    if ((trans= ndb->startTransaction()) == 0)
      goto err;
    {
      NdbOperation *op= 0;
      int r= 0;

      /* read the bitmap exlusive */
      r|= (op= trans->getNdbOperation(ndbtab)) == 0;
      DBUG_ASSERT(r == 0);
      r|= op->readTupleExclusive();
      DBUG_ASSERT(r == 0);
================================================================================
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 4057
	    NdbTransaction *trans= ndb->startTransaction();
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 4075
	      if ((_op= trans->getNdbOperation(reptab)) == NULL) abort();

NdbTransaction *trans= ndb->startTransaction();
    if (trans == NULL)
    {
      ndberror= ndb->getNdbError();
      break;
    }
    NdbRecAttr *col_binlog_type_rec_attr[2];
    NdbRecAttr *col_conflict_fn_rec_attr[2]= {NULL, NULL};
    uint32 ndb_binlog_type[2];
    const uint sz= 256;
    char ndb_conflict_fn_buf[2*sz];
    char *ndb_conflict_fn[2]= {ndb_conflict_fn_buf, ndb_conflict_fn_buf+sz};
    NdbOperation *op[2];
    uint32 i, id= 0;
    for (i= 0; i < 2; i++)
    {
      NdbOperation *_op;
      DBUG_PRINT("info", ("reading[%u]: %s,%s,%u", i, db, table_name, id));
      if ((_op= trans->getNdbOperation(reptab)) == NULL) abort();
===============================================================================
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 2121
	    if ((trans= ndb->startTransaction()) == 0)
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_binlog.cc : 2127
	      r|= (op= trans->getNdbOperation(ndbtab)) == 0;

 if ((trans= ndb->startTransaction()) == 0)
      goto err;
    while (1)
    {
      NdbOperation *op= 0;
      int r= 0;
      r|= (op= trans->getNdbOperation(ndbtab)) == 0;
===============================================================================
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_lock_ext.h : 63
	    if ((trans= ndb->startTransaction()) == 0)
mysql-cluster-gpl-7.1.9a/sql/ha_ndbcluster_lock_ext.h : 70   

    trans= ndb->startTransaction();
    if (trans == NULL)
    {
      ndb_error= ndb->getNdbError();
      goto error_handler;
    }
    op= trans->getNdbOperation(ndbtab);

How to repeat:
code review

Suggested fix:
call startTransaction() with the parameter of getNdbOperation(), and call getNdbOperation() with no parameter.
[12 Mar 2011 20:08] Linhai Song
how's the bug going? do you need some help on unit test?