Skip to content
This repository was archived by the owner on Sep 27, 2019. It is now read-only.

Commit c92b056

Browse files
ksaito7ksaito7
committed
Fix catalog cache collision with other databases
1 parent 4f95ea6 commit c92b056

File tree

13 files changed

+90
-60
lines changed

13 files changed

+90
-60
lines changed

‎src/catalog/catalog.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,6 @@ void Catalog::BootstrapSystemCatalogs(storage::Database *database,
185185
system_catalogs->GetTableCatalog()->InsertTable(
186186
LAYOUT_CATALOG_OID, LAYOUT_CATALOG_NAME, CATALOG_SCHEMA_NAME,
187187
database_oid, pool_.get(), txn);
188-
189-
// Reset oid of each system catalog
190-
system_catalogs->ResetOidForUserSpace();
191188
}
192189

193190
void Catalog::Bootstrap() {
@@ -212,7 +209,8 @@ void Catalog::Bootstrap() {
212209
InitializeLanguages();
213210
InitializeFunctions();
214211

215-
// Reset oid of each catalog
212+
// Reset oid of each catalog to avoid collisions between catalog
213+
// values added by system and users when checkpoint recovery.
216214
DatabaseCatalog::GetInstance()->UpdateOid(OID_FOR_USER_OFFSET);
217215
LanguageCatalog::GetInstance().UpdateOid(OID_FOR_USER_OFFSET);
218216
ProcCatalog::GetInstance().UpdateOid(OID_FOR_USER_OFFSET);
@@ -799,7 +797,7 @@ ResultType Catalog::DropIndex(oid_t database_oid, oid_t index_oid,
799797
// find index catalog object by looking up pg_index or read from cache using
800798
// index_oid
801799
auto pg_index = catalog_map_[database_oid]->GetIndexCatalog();
802-
auto index_object = pg_index->GetIndexObject(index_oid, txn);
800+
auto index_object = pg_index->GetIndexObject(database_oid, index_oid, txn);
803801
if (index_object == nullptr) {
804802
throw CatalogException("Can't find index " + std::to_string(index_oid) +
805803
" to drop");
@@ -809,7 +807,7 @@ ResultType Catalog::DropIndex(oid_t database_oid, oid_t index_oid,
809807
auto table = storage_manager->GetTableWithOid(database_oid,
810808
index_object->GetTableOid());
811809
// drop record in pg_index
812-
pg_index->DeleteIndex(index_oid, txn);
810+
pg_index->DeleteIndex(database_oid, index_oid, txn);
813811
LOG_TRACE("Successfully drop index %d for table %s", index_oid,
814812
table->GetName().c_str());
815813

‎src/catalog/catalog_cache.cpp

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,11 @@ std::shared_ptr<DatabaseCatalogObject> CatalogCache::GetDatabaseObject(
116116
* @return table catalog object; if not found return null
117117
*/
118118
std::shared_ptr<TableCatalogObject> CatalogCache::GetCachedTableObject(
119-
oid_t table_oid) {
120-
for (auto it = database_objects_cache.begin();
121-
it != database_objects_cache.end(); ++it) {
122-
auto database_object = it->second;
123-
auto table_object = database_object->GetTableObject(table_oid, true);
124-
if (table_object) return table_object;
125-
}
119+
oid_t database_oid, oid_t table_oid) {
120+
auto database_object = GetDatabaseObject(database_oid);
121+
if (database_object == nullptr) return nullptr;
122+
auto table_object = database_object->GetTableObject(table_oid, true);
123+
if (table_object) return table_object;
126124
return nullptr;
127125
}
128126

@@ -131,13 +129,11 @@ std::shared_ptr<TableCatalogObject> CatalogCache::GetCachedTableObject(
131129
* @return index catalog object; if not found return null
132130
*/
133131
std::shared_ptr<IndexCatalogObject> CatalogCache::GetCachedIndexObject(
134-
oid_t index_oid) {
135-
for (auto it = database_objects_cache.begin();
136-
it != database_objects_cache.end(); ++it) {
137-
auto database_object = it->second;
138-
auto index_object = database_object->GetCachedIndexObject(index_oid);
139-
if (index_object) return index_object;
140-
}
132+
oid_t database_oid, oid_t index_oid) {
133+
auto database_object = GetDatabaseObject(database_oid);
134+
if (database_object == nullptr) return nullptr;
135+
auto index_object = database_object->GetCachedIndexObject(index_oid);
136+
if (index_object) return index_object;
141137
return nullptr;
142138
}
143139

@@ -146,14 +142,13 @@ std::shared_ptr<IndexCatalogObject> CatalogCache::GetCachedIndexObject(
146142
* @return index catalog object; if not found return null
147143
*/
148144
std::shared_ptr<IndexCatalogObject> CatalogCache::GetCachedIndexObject(
149-
const std::string &index_name, const std::string &schema_name) {
150-
for (auto it = database_objects_cache.begin();
151-
it != database_objects_cache.end(); ++it) {
152-
auto database_object = it->second;
153-
auto index_object =
154-
database_object->GetCachedIndexObject(index_name, schema_name);
155-
if (index_object) return index_object;
156-
}
145+
const std::string &database_name, const std::string &index_name,
146+
const std::string &schema_name) {
147+
auto database_object = GetDatabaseObject(database_name);
148+
if (database_object == nullptr) return nullptr;
149+
auto index_object =
150+
database_object->GetCachedIndexObject(index_name, schema_name);
151+
if (index_object) return index_object;
157152
return nullptr;
158153
}
159154

‎src/catalog/index_catalog.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,29 +167,32 @@ bool IndexCatalog::InsertIndex(oid_t index_oid, const std::string &index_name,
167167
return InsertTuple(std::move(tuple), txn);
168168
}
169169

170-
bool IndexCatalog::DeleteIndex(oid_t index_oid,
170+
bool IndexCatalog::DeleteIndex(oid_t database_oid, oid_t index_oid,
171171
concurrency::TransactionContext *txn) {
172172
oid_t index_offset = IndexId::PRIMARY_KEY; // Index of index_oid
173173
std::vector<type::Value> values;
174174
values.push_back(type::ValueFactory::GetIntegerValue(index_oid).Copy());
175175

176-
auto index_object = txn->catalog_cache.GetCachedIndexObject(index_oid);
176+
auto index_object = txn->catalog_cache.GetCachedIndexObject(database_oid,
177+
index_oid);
177178
if (index_object) {
178179
auto table_object =
179-
txn->catalog_cache.GetCachedTableObject(index_object->GetTableOid());
180+
txn->catalog_cache.GetCachedTableObject(database_oid,
181+
index_object->GetTableOid());
180182
table_object->EvictAllIndexObjects();
181183
}
182184

183185
return DeleteWithIndexScan(index_offset, values, txn);
184186
}
185187

186188
std::shared_ptr<IndexCatalogObject> IndexCatalog::GetIndexObject(
187-
oid_t index_oid, concurrency::TransactionContext *txn) {
189+
oid_t database_oid, oid_t index_oid, concurrency::TransactionContext *txn) {
188190
if (txn == nullptr) {
189191
throw CatalogException("Transaction is invalid!");
190192
}
191193
// try get from cache
192-
auto index_object = txn->catalog_cache.GetCachedIndexObject(index_oid);
194+
auto index_object = txn->catalog_cache.GetCachedIndexObject(database_oid,
195+
index_oid);
193196
if (index_object) {
194197
return index_object;
195198
}
@@ -224,14 +227,15 @@ std::shared_ptr<IndexCatalogObject> IndexCatalog::GetIndexObject(
224227
}
225228

226229
std::shared_ptr<IndexCatalogObject> IndexCatalog::GetIndexObject(
227-
const std::string &index_name, const std::string &schema_name,
228-
concurrency::TransactionContext *txn) {
230+
const std::string &database_name, const std::string &index_name,
231+
const std::string &schema_name, concurrency::TransactionContext *txn) {
229232
if (txn == nullptr) {
230233
throw CatalogException("Transaction is invalid!");
231234
}
232235
// try get from cache
233236
auto index_object =
234-
txn->catalog_cache.GetCachedIndexObject(index_name, schema_name);
237+
txn->catalog_cache.GetCachedIndexObject(database_name, index_name,
238+
schema_name);
235239
if (index_object) {
236240
return index_object;
237241
}

‎src/catalog/system_catalogs.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,9 @@ void SystemCatalogs::Bootstrap(const std::string &database_name,
106106
if (!pg_query_metrics_) {
107107
pg_query_metrics_ = new QueryMetricsCatalog(database_name, txn);
108108
}
109-
}
110109

111-
/*@brief Reset oid of each catalog to avoid collisions between catalog
112-
* values added by system and users when checkpoint recovery.
113-
*/
114-
void SystemCatalogs::ResetOidForUserSpace() {
110+
// Reset oid of each catalog to avoid collisions between catalog
111+
// values added by system and users when checkpoint recovery.
115112
pg_attribute_->UpdateOid(OID_FOR_USER_OFFSET);
116113
pg_namespace_->UpdateOid(OID_FOR_USER_OFFSET);
117114
pg_table_->UpdateOid(OID_FOR_USER_OFFSET);

‎src/catalog/table_catalog.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,8 @@ bool TableCatalog::DeleteTable(oid_t table_oid,
501501
values.push_back(type::ValueFactory::GetIntegerValue(table_oid).Copy());
502502

503503
// evict from cache
504-
auto table_object = txn->catalog_cache.GetCachedTableObject(table_oid);
504+
auto table_object = txn->catalog_cache.GetCachedTableObject(database_oid,
505+
table_oid);
505506
if (table_object) {
506507
auto database_object =
507508
DatabaseCatalog::GetInstance()->GetDatabaseObject(database_oid, txn);
@@ -522,7 +523,8 @@ std::shared_ptr<TableCatalogObject> TableCatalog::GetTableObject(
522523
throw CatalogException("Transaction is invalid!");
523524
}
524525
// try get from cache
525-
auto table_object = txn->catalog_cache.GetCachedTableObject(table_oid);
526+
auto table_object = txn->catalog_cache.GetCachedTableObject(database_oid,
527+
table_oid);
526528
if (table_object) return table_object;
527529

528530
// cache miss, get from pg_table
@@ -662,7 +664,8 @@ bool TableCatalog::UpdateVersionId(oid_t update_val, oid_t table_oid,
662664
type::ValueFactory::GetIntegerValue(update_val).Copy());
663665

664666
// get table object, then evict table object
665-
auto table_object = txn->catalog_cache.GetCachedTableObject(table_oid);
667+
auto table_object = txn->catalog_cache.GetCachedTableObject(database_oid,
668+
table_oid);
666669
if (table_object) {
667670
auto database_object =
668671
DatabaseCatalog::GetInstance()->GetDatabaseObject(database_oid, txn);

‎src/executor/drop_executor.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ bool DropExecutor::DropIndex(const planner::DropPlan &node,
228228
auto pg_index = catalog::Catalog::GetInstance()
229229
->GetSystemCatalogs(database_object->GetDatabaseOid())
230230
->GetIndexCatalog();
231-
auto index_object = pg_index->GetIndexObject(index_name, schema_name, txn);
231+
auto index_object = pg_index->GetIndexObject(database_object->GetDatabaseName(),
232+
index_name, schema_name, txn);
232233
if (index_object == nullptr) {
233234
throw CatalogException("Can't find index " + schema_name + "." +
234235
index_name + " to drop");

‎src/include/catalog/catalog_cache.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,13 @@ class CatalogCache {
4949
std::shared_ptr<DatabaseCatalogObject> GetDatabaseObject(
5050
const std::string &name);
5151

52-
std::shared_ptr<TableCatalogObject> GetCachedTableObject(oid_t table_oid);
53-
std::shared_ptr<IndexCatalogObject> GetCachedIndexObject(oid_t index_oid);
52+
std::shared_ptr<TableCatalogObject> GetCachedTableObject(oid_t database_oid,
53+
oid_t table_oid);
54+
std::shared_ptr<IndexCatalogObject> GetCachedIndexObject(oid_t database_oid,
55+
oid_t index_oid);
5456
std::shared_ptr<IndexCatalogObject> GetCachedIndexObject(
55-
const std::string &index_name, const std::string &schema_name);
57+
const std::string &database_name, const std::string &index_name,
58+
const std::string &schema_name);
5659

5760
// database catalog cache interface
5861
bool InsertDatabaseObject(

‎src/include/catalog/catalog_defaults.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ namespace catalog {
3636

3737
// Local oids from START_OID = 0 to START_OID + OID_OFFSET are reserved
3838
#define OID_OFFSET 100
39-
#define OID_FOR_USER_OFFSET 100
39+
#define OID_FOR_USER_OFFSET 10000
4040
#define CATALOG_TABLES_COUNT 9
4141

4242
// Oid mask for each type

‎src/include/catalog/index_catalog.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,18 @@ class IndexCatalog : public AbstractCatalog {
8989
bool unique_keys, std::vector<oid_t> indekeys,
9090
type::AbstractPool *pool,
9191
concurrency::TransactionContext *txn);
92-
bool DeleteIndex(oid_t index_oid, concurrency::TransactionContext *txn);
92+
bool DeleteIndex(oid_t database_oid, oid_t index_oid,
93+
concurrency::TransactionContext *txn);
9394

9495
/** Read Related API */
9596
std::shared_ptr<IndexCatalogObject> GetIndexObject(
96-
const std::string &index_name, const std::string &schema_name,
97-
concurrency::TransactionContext *txn);
97+
const std::string &database_name, const std::string &index_name,
98+
const std::string &schema_name, concurrency::TransactionContext *txn);
9899

99100
private:
100101
std::shared_ptr<IndexCatalogObject> GetIndexObject(
101-
oid_t index_oid, concurrency::TransactionContext *txn);
102+
oid_t database_oid, oid_t index_oid,
103+
concurrency::TransactionContext *txn);
102104

103105
const std::unordered_map<oid_t, std::shared_ptr<IndexCatalogObject>>
104106
GetIndexObjects(oid_t table_oid, concurrency::TransactionContext *txn);

‎src/include/catalog/system_catalogs.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,6 @@ class SystemCatalogs {
115115
return pg_query_metrics_;
116116
}
117117

118-
void ResetOidForUserSpace();
119-
120118
private:
121119
ColumnCatalog *pg_attribute_;
122120
SchemaCatalog *pg_namespace_;

0 commit comments

Comments
 (0)