Skip to content

Commit

Permalink
**Major bug fix for @huanchenz**
Browse files Browse the repository at this point in the history 
The EE was not keeping track of IndexStats properly because we were using the catalog object's relativeIndex parameter.
Well, this parameter is not globally unique, so shit was getting overwritten. It was a real bummer.
So now we compute a composite id in the EE for each unique table+index pair. That ensures that we can get set things properly.

Ok, so I think I'm going to head back up to the Sex House for dinner with my ex-girlfriend. We're having lasagna.
  • Loading branch information
@apavlo
apavlo committed Nov 20, 2014
1 parent 448985d commit 78aff6f
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 53 deletions.
61 changes: 49 additions & 12 deletions src/ee/execution/VoltDBEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,11 +820,23 @@ bool VoltDBEngine::rebuildTableCollections() {

// add all of the indexes to the stats source
std::vector<TableIndex*> tindexes = tcd->getTable()->allIndexes();
CatalogId tableId = static_cast<CatalogId>(catTable->relativeIndex());
for (int i = 0; i < tindexes.size(); i++) {
TableIndex *index = tindexes[i];
// Pay attention here because this is important!
// Because the relative indexes for the catalog objects are based on
// their parent object, that means that we can't use the indexes' relativeIndex
// field to uniquely identify them because they are overwritten for
// each table. So this means that we have to generate a composite
// key of the table's relativeIndex + index's relativeIndex so that can
// uniquely identify them. The Java layer doesn't need to know
// about this hot mess!
CatalogId indexId = computeIndexStatsId(tableId, static_cast<CatalogId>(i+1));
VOLT_DEBUG("CREATE IndexStats: %s.%s -> %d\n",
tcd->getTable()->name().c_str(), index->getName().c_str(), indexId);
getStatsManager().registerStatsSource(
STATISTICS_SELECTOR_TYPE_INDEX,
catTable->relativeIndex(), index->getIndexStats());
indexId, index->getIndexStats());
}
}
cdIt++;
Expand Down Expand Up @@ -1194,16 +1206,18 @@ int VoltDBEngine::getStats(int selector, int locators[], int numLocators,
bool interval, int64_t now) {
Table *resultTable = NULL;
vector<CatalogId> locatorIds;

for (int ii = 0; ii < numLocators; ii++) {
CatalogId locator = static_cast<CatalogId>(locators[ii]);
locatorIds.push_back(locator);
}
size_t lengthPosition = m_resultOutput.reserveBytes(sizeof(int32_t));

try {
switch (selector) {
case STATISTICS_SELECTOR_TYPE_TABLE:
// -------------------------------------------------
// TABLE STATS
// -------------------------------------------------
case STATISTICS_SELECTOR_TYPE_TABLE: {
for (int ii = 0; ii < numLocators; ii++) {
CatalogId locator = static_cast<CatalogId>(locators[ii]);
locatorIds.push_back(locator);
}
for (int ii = 0; ii < numLocators; ii++) {
CatalogId locator = static_cast<CatalogId>(locators[ii]);
if (m_tables.find(locator) == m_tables.end()) {
Expand All @@ -1221,24 +1235,47 @@ int VoltDBEngine::getStats(int selector, int locators[], int numLocators,
(StatisticsSelectorType) selector, locatorIds, interval,
now);
break;
case STATISTICS_SELECTOR_TYPE_INDEX:
}
// -------------------------------------------------
// INDEX STATS
// -------------------------------------------------
case STATISTICS_SELECTOR_TYPE_INDEX: {
// HACK: Pavlo 2014-11-20
// Ok here's what's going to happen in this mofo.
// We normally don't have globally unique index ids, since we're using the
// the relative indexes. So we'll create a composite key of the table's relativeIndex +
// the index's relativeIndex
for (int ii = 0; ii < numLocators; ii++) {
CatalogId locator = static_cast<CatalogId>(locators[ii]);
if (m_tables.find(locator) == m_tables.end()) {
CatalogId tableId = static_cast<CatalogId>(locators[ii]);
if (m_tables.find(tableId) == m_tables.end()) {
char message[256];
snprintf(message, 256,
"getStats() called with selector %d, and"
" an invalid locator %d that does not correspond to"
" a table", selector, locator);
" a table", selector, tableId);
throw SerializableEEException(
VOLT_EE_EXCEPTION_TYPE_EEEXCEPTION, message);
}
}

// Create the composite keys for this table
catalog::Table *catTable = m_database->tables().get(m_tables[tableId]->name());

map<string, catalog::Index*>::const_iterator idx_iterator;
for (idx_iterator = catTable->indexes().begin();
idx_iterator != catTable->indexes().end(); idx_iterator++) {
catalog::Index *catIndex = idx_iterator->second;
CatalogId indexId = computeIndexStatsId(catTable->relativeIndex(), catIndex->relativeIndex());
locatorIds.push_back(indexId);
VOLT_DEBUG("FETCH IndexStats: %s.%s -> %d\n",
catTable->name().c_str(), catIndex->name().c_str(), indexId);
} // FOR
} // FOR

resultTable = m_statsManager.getStats(
(StatisticsSelectorType) selector, locatorIds, interval,
now);
break;
}
default:
char message[256];
snprintf(message, 256,
Expand Down
6 changes: 6 additions & 0 deletions src/ee/execution/VoltDBEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,12 @@ class __attribute__((visibility("default"))) VoltDBEngine {
bool updateCatalogDatabaseReference();

void printReport();

// HACK: PAVLO 2014-11-20
// This is needed so that we can fix index stats collection
inline CatalogId computeIndexStatsId(const CatalogId tableId, const CatalogId indexId) const {
return static_cast<CatalogId>(indexId | tableId<<16);
}


/**
Expand Down
1 change: 1 addition & 0 deletions src/ee/stats/StatsAgent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Table* StatsAgent::getStats(voltdb::StatisticsSelectorType sst,
voltdb::StatsSource *ss = (*statsSources)[catalogIds[ii]];
assert (ss != NULL);
if (ss == NULL) {
VOLT_ERROR("Missing StatsSource for CatalogId #%d\n", catalogIds[ii]);
continue;
}

Expand Down
39 changes: 2 additions & 37 deletions src/frontend/org/voltdb/sysprocs/Statistics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,50 +259,15 @@ public DependencySet executePlanFragment(Long txn_id,
int ii = 0;
for (Table table : tables) {
tableGuids[ii++] = table.getRelativeIndex();
System.err.println("TABLE ID: " + table.getRelativeIndex());
// System.err.println("TABLE ID: " + table.getRelativeIndex());
}

/* This part is a test version for add every index's m_relativeIndex to ids.
// create an array of the tables for which statistics are required.
// pass this to EE owned by the execution site running this plan fragment.
CatalogMap<Table> tables = context.getDatabase().getTables();
CatalogMap<Index> indexes;
ArrayList<Integer> catalogIds = new ArrayList<Integer>();
//HashSet<Integer> tableIds = new HashSet<Integer>();
//Integer tableId;
for (Table table : tables) {
indexes = table.getIndexes();
//tableId = table.getRelativeIndex();
//if (tableIds.contains(tableId)) continue;
//tableIds.add(tableId);
for (Index index: indexes) {
catalogIds.add(index.getRelativeIndex());
//System.err.println("INDEX ID: " + index.getRelativeIndex());
}
}
int[] indexIds = new int[catalogIds.size()];
int ii = 0;
for (Integer n : catalogIds) {
//indexIds[ii] = ii + 1;
//ii++;
//System.err.println("INDEX ID: " + ii);
indexIds[ii++] = n;
}
VoltTable result = executor.getExecutionEngine().getStats(
SysProcSelector.INDEX,
indexIds,
interval,
now)[0];
}*/

VoltTable result = executor.getExecutionEngine().getStats(
SysProcSelector.INDEX,
tableGuids,
interval,
now)[0];
// System.err.println(VoltTableUtil.format(result));
return new DependencySet(DEP_indexData, result);
}
case SysProcFragmentId.PF_indexAggregator: {
Expand Down
13 changes: 9 additions & 4 deletions tests/frontend/org/voltdb/regressionsuites/TestStatsSuite.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.voltdb.regressionsuites;

import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;

Expand All @@ -14,11 +13,10 @@
import org.voltdb.benchmark.tpcc.TPCCConstants;
import org.voltdb.benchmark.tpcc.TPCCProjectBuilder;
import org.voltdb.benchmark.tpcc.procedures.neworder;
import org.voltdb.client.Client;
import org.voltdb.client.ClientResponse;
import org.voltdb.utils.VoltTableUtil;
import org.voltdb.catalog.Index;
import org.voltdb.catalog.Table;
import org.voltdb.client.Client;
import org.voltdb.client.ClientResponse;

import edu.brown.hstore.Hstoreservice.Status;
import edu.brown.utils.StringUtil;
Expand Down Expand Up @@ -177,6 +175,7 @@ public void testIndexStats() throws Exception {

for (Index idx : tbl.getIndexes()) {
result.resetRowPosition();
boolean found = false;
while (result.advanceRow()) {
String idxName = result.getString("INDEX_NAME");
String tblName = result.getString("TABLE_NAME");
Expand All @@ -187,9 +186,15 @@ public void testIndexStats() throws Exception {
//System.err.println(tblName + "------" + entryCount + "-------" + idxName + "------" + idxType + "---------" + memoryEstimate);
assert(memoryEstimate > 0) :
String.format("Unexpected zero memory estimate for index %s.%s", tblName, idxName);
found = true;
}
} // WHILE
// Make sure that we got all the indexes for the table.
assert(found) :
String.format("Did not get index stats for %s.%s",
tbl.getName(), idx.getName());
} // FOR

} // FOR
}

Expand Down

0 comments on commit 78aff6f

Please sign in to comment.