Skip to content

Commit

Permalink
Fix performance regression due to QI for DDL
Browse files Browse the repository at this point in the history
This check-in omits object UIDs from query plans for the tables
SB_HISTOGRAMS and SB_HISTOGRAMS_INTERVALS. Previously, when the
code generator tried to add the object UIDs for these, it had to
make a special query to the metadata, since the corresponding
internal cached structure omitted object UIDs when they were
created via methods like Generator::createVirtualTableDesc. The
special query to lookup these object UIDs was shown to be
responsible for a large pathlengh regression.

Change-Id: Id5046c5c55a4fc8dd2ba3f891449ea87d35a5534
Closes-Bug: #1398600
  • Loading branch information
mikehanlonathpdotcom committed Dec 3, 2014
1 parent f445366 commit 4313fc2
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 18 deletions.
2 changes: 1 addition & 1 deletion sql/generator/GenRelExeUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5518,7 +5518,7 @@ short ExeUtilHbaseCoProcAggr::codeGen(Generator * generator)
TableDesc *tableDesc = getUtilTableDesc();
if (tableDesc->getNATable()->isSeabaseTable())
{
if (!tableDesc->getNATable()->isSeabaseMDTable())
if (tableDesc->getNATable()->isEnabledForDDLQI())
generator->objectUids().insert(
tableDesc->getNATable()->objectUid().get_value());
}
Expand Down
6 changes: 3 additions & 3 deletions sql/generator/GenRelScan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ short Describe::codeGen(Generator * generator)
CmpCommon::diags()->rewind(daMark, TRUE);

if (describedTable && describedTable->isSeabaseTable() &&
(!describedTable->isSeabaseMDTable()))
describedTable->isEnabledForDDLQI())
generator->objectUids().insert(
describedTable->objectUid().get_value());
}
Expand Down Expand Up @@ -2546,7 +2546,7 @@ short HbaseAccess::codeGen(Generator * generator)

if (getTableDesc()->getNATable()->isSQLMXAlignedTable())
hbasescan_tdb->setAlignedFormat(TRUE);
if (!getTableDesc()->getNATable()->isSeabaseMDTable())
if (getTableDesc()->getNATable()->isEnabledForDDLQI())
generator->objectUids().insert(
getTableDesc()->getNATable()->objectUid().get_value());
}
Expand Down Expand Up @@ -2798,7 +2798,7 @@ short HbaseAccessCoProcAggr::codeGen(Generator * generator)
if (getTableDesc()->getNATable()->isSeabaseTable())
{
hbasescan_tdb->setSQHbaseTable(TRUE);
if (!getTableDesc()->getNATable()->isSeabaseMDTable())
if (getTableDesc()->getNATable()->isEnabledForDDLQI())
generator->objectUids().insert(
getTableDesc()->getNATable()->objectUid().get_value());
}
Expand Down
6 changes: 3 additions & 3 deletions sql/generator/GenRelUpdate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@ short HbaseDelete::codeGen(Generator * generator)
(NOT noCheck()))
hbasescan_tdb->setHbaseSqlIUD(TRUE);

if (!getTableDesc()->getNATable()->isSeabaseMDTable())
if (getTableDesc()->getNATable()->isEnabledForDDLQI())
generator->objectUids().insert(
getTableDesc()->getNATable()->objectUid().get_value());
}
Expand Down Expand Up @@ -2086,7 +2086,7 @@ short HbaseUpdate::codeGen(Generator * generator)
if (CmpCommon::getDefault(HBASE_SQL_IUD_SEMANTICS) == DF_ON)
hbasescan_tdb->setHbaseSqlIUD(TRUE);

if (!getTableDesc()->getNATable()->isSeabaseMDTable())
if (getTableDesc()->getNATable()->isEnabledForDDLQI())
generator->objectUids().insert(
getTableDesc()->getNATable()->objectUid().get_value());
}
Expand Down Expand Up @@ -2724,7 +2724,7 @@ short HbaseInsert::codeGen(Generator *generator)


}
if (!getTableDesc()->getNATable()->isSeabaseMDTable())
if (getTableDesc()->getNATable()->isEnabledForDDLQI())
generator->objectUids().insert(
getTableDesc()->getNATable()->objectUid().get_value());
}
Expand Down
24 changes: 24 additions & 0 deletions sql/optimizer/NATable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6790,6 +6790,30 @@ void NATable::lookupObjectUid()
objectUID_ = ::lookupObjectUid(qualName, objectType_);
}

bool NATable::isEnabledForDDLQI() const
{
if (isSeabaseMD_ || isSMDTable_)
return false;
else
{
if (objectUID_.get_value() == 0)
{
// Looking up object UIDs at code-gen time was shown to cause
// more than 10% performance regression in YCSB benchmark. In
// that investigation, we learned that metadata and histogram
// NATables would have no object UID at code-gen and would
// require the lookup. We're pretty sure these are the only
// types of tables but will abend here otherwise. If this
// causes problems, the envvar below can be used as a
// temporary workaround.
char *noAbendOnLp1398600 = getenv("NO_ABEND_ON_LP_1398600");
if (!noAbendOnLp1398600 || *noAbendOnLp1398600 == '0')
abort();
}
return true;
}
}

NATable::~NATable()
{
// remove the map entries of associated table identifers in
Expand Down
3 changes: 2 additions & 1 deletion sql/optimizer/NATable.h
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,8 @@ class NATable : public NABasicObject
return objectUID_;
}

void lookupObjectUid(); // Used to look up uid on demand for metadata tables
void lookupObjectUid(); // Used to look up uid on demand for metadata tables
bool isEnabledForDDLQI() const;

const ComObjectType &getObjectType() const { return objectType_; }

Expand Down
14 changes: 11 additions & 3 deletions sql/regress/executor/EXPECTED122
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@

--- SQL command prepared.
>>
>>update statistics for table t122t1 on every column;

--- SQL operation complete.
>>
>>obey TEST122(uid_in_plan);
>>log;
look for one uid only
ObjectUIDs .......
look for more than one uid
>>-- lp 1398600 -- test that there are no object UIDs for MD or histograms
>>log;
>>
>>
>>execute s1;
Expand All @@ -37,6 +43,8 @@ look for more than one uid
look for one uid only
ObjectUIDs .......
look for more than one uid
>>-- lp 1398600 -- test that there are no object UIDs for MD or histograms
>>log;
>>
>>
>>execute s1;
Expand All @@ -53,7 +61,7 @@ A
>>invoke t122t1;

-- Definition of Trafodion table TRAFODION.SCH.T122T1
-- Definition current Wed Nov 19 17:36:02 2014
-- Definition current Wed Dec 3 17:47:33 2014

(
A INT NO DEFAULT NOT NULL NOT DROPPABLE
Expand Down Expand Up @@ -105,7 +113,7 @@ A
>>execute s3;

-- Definition of Trafodion table TRAFODION.SCH.T122T1
-- Definition current Wed Nov 19 17:36:02 2014
-- Definition current Wed Dec 3 17:47:33 2014

(
A INT NO DEFAULT NOT NULL NOT DROPPABLE
Expand Down Expand Up @@ -214,7 +222,7 @@ iv


-- Definition of Trafodion table TRAFODION.SCH.T122T1
-- Definition current Wed Nov 19 17:36:17 2014
-- Definition current Wed Dec 3 17:48:08 2014

(
A CHAR(4) CHARACTER SET ISO88591 COLLATE
Expand Down
11 changes: 11 additions & 0 deletions sql/regress/executor/TEST122
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ prepare s1 from insert into t122t1 values
, (1)
;

update statistics for table t122t1 on every column;

obey TEST122(uid_in_plan);

execute s1;
Expand Down Expand Up @@ -321,6 +323,15 @@ sh grep "^ ObjectUIDs .* [1-9][0-9]*$" PLAN122 | cut -c 1-20 >> LOG122;
sh echo "look for more than one uid" >> LOG122;
sh grep "^ ObjectUIDs .* [1-9][0-9]*," PLAN122;
log LOG122;
-- lp 1398600 -- test that there are no object UIDs for MD or histograms
log;
log PLAN122 clear;
explain select * from SB_HISTOGRAMS;
explain select * from SB_HISTOGRAM_INTERVALS;
explain select * from "_MD_".OBJECTS;
log;
sh grep "^ ObjectUIDs .* [1-9][0-9]*" PLAN122 | cut -c 1-20 >> LOG122;
log LOG122;

?section drop_tab

Expand Down
13 changes: 6 additions & 7 deletions sql/sqlcomp/CmpMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,8 @@ void CmpMain:: getAndProcessAnySiKeys(TimeVal begTime)
#endif
;

int numRetriesRemaining = 16;
Int32 sqlcode = -1;
while (sqlcode < 0 && numRetriesRemaining)
while (sqlcode < 0)
{
SQL_QIKEY sikKeyArray[arraySize];

Expand All @@ -1129,11 +1128,11 @@ void CmpMain:: getAndProcessAnySiKeys(TimeVal begTime)
}
else if (sqlcode < 0)
{
// The CLI sends messages to RMS processes. The only errors we
// would expect would be retryable, as these processes are
// persistent.
sleep(1);
numRetriesRemaining--;
// No other error is expected. The CLI call
// simply accesses memory. If the implementation
// changes, then the abort below should remind us
// to change the error handling.
abort();
}
else if ( returnedNumSiKeys > 0 )
{
Expand Down

0 comments on commit 4313fc2

Please sign in to comment.