Skip to content

Commit 5f5c290

Browse files
apkarapkar
authored
Merge pull request #139 from apkar/schema-changes
Resolves #138: Cache should always include collection context with indexes
2 parents c4cb6a3 + fdb1a3e commit 5f5c290

7 files changed

Lines changed: 59 additions & 38 deletions

File tree

‎src/Constants.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ const uint64_t INDEX_KEY_LENGTH_LIMIT = (uint64_t)1e4;
2727
const uint64_t FDB_KEY_LENGTH_LIMIT = (uint64_t)1e4;
2828
const uint64_t FDB_VALUE_LENGTH_LIMIT = (uint64_t)1e5;
2929

30+
const uint64_t METADATA_CACHE_SIZE = (uint64_t)100;
31+
3032
const std::string METADATA = "metadata";
3133
const std::string VERSION_KEY = "version";
3234
const std::string INDICES_KEY = "indices";

‎src/Constants.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ extern const uint64_t INDEX_KEY_LENGTH_LIMIT; // This is set to 10K bytes, inste
3434
extern const uint64_t FDB_KEY_LENGTH_LIMIT;
3535
extern const uint64_t FDB_VALUE_LENGTH_LIMIT;
3636

37+
// Size of metadata cache in entries, number of collections
38+
extern const uint64_t METADATA_CACHE_SIZE;
39+
3740
// KVS DocLayer internal keys
3841
extern const std::string METADATA;
3942
extern const std::string VERSION_KEY;

‎src/ExtCmd.actor.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ ACTOR static Future<std::pair<int, int>> dropIndexMatching(Reference<DocTransact
5050
state std::vector<bson::BSONObj> indexes = wait(getIndexesTransactionally(indexesPlan, tr));
5151
state int count = 0;
5252
state bool any = false;
53+
state bson::BSONObj matchingIndexObj;
5354
state Reference<QueryContext> matchingIndex;
5455
state std::string matchingName;
5556

@@ -63,13 +64,15 @@ ACTOR static Future<std::pair<int, int>> dropIndexMatching(Reference<DocTransact
6364
if (value.getBSONType() == bson::BSONType::String) {
6465
if (value.getString() == indexObj.getStringField(field.c_str())) {
6566
any = true;
67+
matchingIndexObj = indexObj;
6668
matchingIndex = indexesCollection->bindCollectionContext(tr)->cx->getSubContext(
6769
DataValue(indexObj.getField(DocLayerConstants::ID_FIELD)).encode_key_part());
6870
matchingName = std::string(indexObj.getStringField(DocLayerConstants::NAME_FIELD));
6971
}
7072
} else if (value.getBSONType() == bson::BSONType::Object) {
7173
if (value.getPackedObject().woCompare(indexObj.getObjectField(field.c_str())) == 0) {
7274
any = true;
75+
matchingIndexObj = indexObj;
7376
matchingIndex = indexesCollection->bindCollectionContext(tr)->cx->getSubContext(
7477
DataValue(indexObj.getField(DocLayerConstants::ID_FIELD)).encode_key_part());
7578
matchingName = std::string(indexObj.getStringField(DocLayerConstants::NAME_FIELD));
@@ -82,6 +85,11 @@ ACTOR static Future<std::pair<int, int>> dropIndexMatching(Reference<DocTransact
8285
matchingIndex->clearDescendants();
8386
matchingIndex->clearRoot();
8487

88+
TraceEvent(SevInfo, "BumpMetadataVersion")
89+
.detail("reason", "dropIndex")
90+
.detail("ns", fullCollNameToString(ns))
91+
.detail("index", matchingIndexObj.toString());
92+
8593
Void _ = wait(matchingIndex->commitChanges());
8694

8795
Key indexKey = targetedCollection->getIndexesSubspace().withSuffix(StringRef(encodeMaybeDotted(matchingName)));
@@ -356,6 +364,9 @@ ACTOR static Future<int> internal_doDropIndexesActor(Reference<DocTransaction> t
356364
Key indexes = unbound->getIndexesSubspace();
357365
tr->tr->clear(FDB::KeyRangeRef(indexes, strinc(indexes)));
358366
unbound->bindCollectionContext(tr)->bumpMetadataVersion();
367+
TraceEvent(SevInfo, "BumpMetadataVersion")
368+
.detail("reason", "dropAllIndexes")
369+
.detail("ns", fullCollNameToString(ns));
359370

360371
return count;
361372
}

‎src/ExtMsg.actor.cpp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@ Namespace getDBCollectionPair(const char* ns, std::pair<std::string, std::string
6363
return std::make_pair(std::string(ns, dotPtr - ns), std::string(dotPtr + 1));
6464
}
6565

66-
std::string fullCollNameToString(Namespace const& ns) {
67-
return ns.first + "." + (ns.second.empty() ? "$cmd" : ns.second);
68-
}
69-
7066
/**
7167
* query := { $bool_op : [ query* ], * (a predicate)
7268
* path : literal_match, *
@@ -1208,8 +1204,7 @@ ACTOR Future<WriteCmdResult> doDeleteCmd(Namespace ns,
12081204

12091205
// If collection not found then just return success from here.
12101206
try {
1211-
Reference<UnboundCollectionContext> _cx =
1212-
wait(ec->mm->getUnboundCollectionContext(dtr, ns, false, true, false));
1207+
Reference<UnboundCollectionContext> _cx = wait(ec->mm->getUnboundCollectionContext(dtr, ns, false, false));
12131208
cx = _cx;
12141209
} catch (Error& e) {
12151210
if (e.code() == error_code_collection_not_found)

‎src/MetadataManager.actor.cpp‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525

2626
using namespace FDB;
2727

28+
std::string fullCollNameToString(Namespace const& ns) {
29+
return ns.first + "." + (ns.second.empty() ? "$cmd" : ns.second);
30+
}
31+
2832
Future<uint64_t> getMetadataVersion(Reference<DocTransaction> tr, Reference<DirectorySubspace> metadataDirectory) {
2933
std::string versionKey = metadataDirectory->key().toString() +
3034
DataValue(DocLayerConstants::VERSION_KEY, DVTypeCode::STRING).encode_key_part();
@@ -82,12 +86,8 @@ IndexInfo MetadataManager::indexInfoFromObj(const bson::BSONObj& indexObj, Refer
8286
}
8387
}
8488

85-
ACTOR static Future<std::pair<Reference<UnboundCollectionContext>, uint64_t>> constructContext(
86-
Namespace ns,
87-
Reference<DocTransaction> tr,
88-
DocumentLayer* docLayer,
89-
bool includeIndex,
90-
bool createCollectionIfAbsent) {
89+
ACTOR static Future<std::pair<Reference<UnboundCollectionContext>, uint64_t>>
90+
constructContext(Namespace ns, Reference<DocTransaction> tr, DocumentLayer* docLayer, bool createCollectionIfAbsent) {
9191
try {
9292
// The initial set of directory reads take place in a separate transaction with the same read version as `tr'.
9393
// This hopefully prevents us from accidentally RYWing a directory that `tr' itself created, and then adding it
@@ -108,20 +108,15 @@ ACTOR static Future<std::pair<Reference<UnboundCollectionContext>, uint64_t>> co
108108
state Reference<UnboundCollectionContext> cx =
109109
Reference<UnboundCollectionContext>(new UnboundCollectionContext(collectionDirectory, metadataDirectory));
110110

111-
// Only include existing indexes into the context when it's NOT building a new index.
112-
// When it's building a new index, it's unnecessary and inefficient to pass each recorded returned by a
113-
// TableScan through the existing indexes.
114-
if (includeIndex) {
115-
state Reference<UnboundCollectionContext> indexCx = Reference<UnboundCollectionContext>(
116-
new UnboundCollectionContext(indexDirectory, Reference<DirectorySubspace>()));
117-
state Reference<Plan> indexesPlan = getIndexesForCollectionPlan(indexCx, ns);
118-
std::vector<bson::BSONObj> allIndexes = wait(getIndexesTransactionally(indexesPlan, tr));
111+
state Reference<UnboundCollectionContext> indexCx = Reference<UnboundCollectionContext>(
112+
new UnboundCollectionContext(indexDirectory, Reference<DirectorySubspace>()));
113+
state Reference<Plan> indexesPlan = getIndexesForCollectionPlan(indexCx, ns);
114+
std::vector<bson::BSONObj> allIndexes = wait(getIndexesTransactionally(indexesPlan, tr));
119115

120-
for (const auto& indexObj : allIndexes) {
121-
IndexInfo index = MetadataManager::indexInfoFromObj(indexObj, cx);
122-
if (index.status != IndexInfo::IndexStatus::INVALID) {
123-
cx->addIndex(index);
124-
}
116+
for (const auto& indexObj : allIndexes) {
117+
IndexInfo index = MetadataManager::indexInfoFromObj(indexObj, cx);
118+
if (index.status != IndexInfo::IndexStatus::INVALID) {
119+
cx->addIndex(index);
125120
}
126121
}
127122

@@ -157,6 +152,9 @@ ACTOR static Future<std::pair<Reference<UnboundCollectionContext>, uint64_t>> co
157152
// collectionName.c_str(), printable(tcollectionDirectory->key()).c_str(),
158153
// printable(tmetadataDirectory->key()).c_str(), "");
159154
tcx->bindCollectionContext(tr)->bumpMetadataVersion(); // We start at version 1.
155+
TraceEvent(SevInfo, "BumpMetadataVersion")
156+
.detail("reason", "createCollection")
157+
.detail("ns", fullCollNameToString(ns));
160158

161159
return std::make_pair(tcx, -1); // So we don't pollute the cache in case this transaction never commits
162160
}
@@ -165,20 +163,22 @@ ACTOR static Future<std::pair<Reference<UnboundCollectionContext>, uint64_t>> co
165163
ACTOR static Future<Reference<UnboundCollectionContext>> assembleCollectionContext(Reference<DocTransaction> tr,
166164
Namespace ns,
167165
Reference<MetadataManager> self,
168-
bool includeIndex,
169166
bool createCollectionIfAbsent) {
170-
if (self->contexts.size() > 100)
167+
if (self->contexts.size() > DocLayerConstants::METADATA_CACHE_SIZE)
171168
self->contexts.clear();
172169

173170
auto match = self->contexts.find(ns);
174171

175172
if (match == self->contexts.end()) {
176173
std::pair<Reference<UnboundCollectionContext>, uint64_t> unboundPair =
177-
wait(constructContext(ns, tr, self->docLayer, includeIndex, createCollectionIfAbsent));
174+
wait(constructContext(ns, tr, self->docLayer, createCollectionIfAbsent));
178175

179176
// Here and below don't pollute the cache if we just created the directory, since this transaction might
180177
// not commit.
181178
if (unboundPair.second != -1) {
179+
TraceEvent(SevInfo, "MetadataCacheAdd")
180+
.detail("ns", fullCollNameToString(ns))
181+
.detail("version", unboundPair.second);
182182
auto insert_result = self->contexts.insert(std::make_pair(ns, unboundPair));
183183
// Somebody else may have done the lookup and finished ahead of us. Either way, replace it with ours (can no
184184
// longer optimize this by only replacing if ours is newer, because the directory may have moved or
@@ -194,19 +194,21 @@ ACTOR static Future<Reference<UnboundCollectionContext>> assembleCollectionConte
194194
uint64_t version = wait(getMetadataVersion(tr, oldUnbound->metadataDirectory));
195195
if (version != oldVersion) {
196196
std::pair<Reference<UnboundCollectionContext>, uint64_t> unboundPair =
197-
wait(constructContext(ns, tr, self->docLayer, includeIndex, createCollectionIfAbsent));
197+
wait(constructContext(ns, tr, self->docLayer, createCollectionIfAbsent));
198198
if (unboundPair.second != -1) {
199199
// Create the iterator again instead of making the previous value state, because the map could have
200200
// changed during the previous wait. Either way, replace it with ours (can no longer optimize this by
201201
// only replacing if ours is newer, because the directory may have moved or vanished.
202-
// std::map<std::pair<std::string, std::string>, std::pair<Reference<UnboundCollectionContext>,
203-
// uint64_t>>::iterator match = self->contexts.find(ns);
204202
auto match = self->contexts.find(ns);
205-
206203
if (match != self->contexts.end())
207204
match->second = unboundPair;
208205
else
209206
self->contexts.insert(std::make_pair(ns, unboundPair));
207+
208+
TraceEvent(SevInfo, "MetadataCacheUpdate")
209+
.detail("ns", fullCollNameToString(ns))
210+
.detail("oldVersion", oldVersion)
211+
.detail("newVersion", unboundPair.second);
210212
}
211213
return unboundPair.first;
212214
} else {
@@ -219,19 +221,17 @@ Future<Reference<UnboundCollectionContext>> MetadataManager::getUnboundCollectio
219221
Reference<DocTransaction> tr,
220222
Namespace const& ns,
221223
bool allowSystemNamespace,
222-
bool includeIndex,
223224
bool createCollectionIfAbsent) {
224225
if (!allowSystemNamespace && startsWith(ns.second.c_str(), "system."))
225226
throw write_system_namespace();
226-
return assembleCollectionContext(tr, ns, Reference<MetadataManager>::addRef(this), includeIndex,
227-
createCollectionIfAbsent);
227+
return assembleCollectionContext(tr, ns, Reference<MetadataManager>::addRef(this), createCollectionIfAbsent);
228228
}
229229

230230
Future<Reference<UnboundCollectionContext>> MetadataManager::refreshUnboundCollectionContext(
231231
Reference<UnboundCollectionContext> cx,
232232
Reference<DocTransaction> tr) {
233233
return assembleCollectionContext(tr, std::make_pair(cx->databaseName(), cx->collectionName()),
234-
Reference<MetadataManager>::addRef(this), false, false);
234+
Reference<MetadataManager>::addRef(this), false);
235235
}
236236

237237
ACTOR static Future<Void> buildIndex_impl(bson::BSONObj indexObj,
@@ -242,7 +242,7 @@ ACTOR static Future<Void> buildIndex_impl(bson::BSONObj indexObj,
242242
state IndexInfo info;
243243
try {
244244
state Reference<DocTransaction> tr = ec->getOperationTransaction();
245-
state Reference<UnboundCollectionContext> mcx = wait(ec->mm->getUnboundCollectionContext(tr, ns, false, false));
245+
state Reference<UnboundCollectionContext> mcx = wait(ec->mm->getUnboundCollectionContext(tr, ns, false));
246246
info = MetadataManager::indexInfoFromObj(indexObj, mcx);
247247
info.status = IndexInfo::IndexStatus::BUILDING;
248248
info.buildId = build_id;

‎src/MetadataManager.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131

3232
using Namespace = std::pair<std::string, std::string>;
3333

34+
std::string fullCollNameToString(Namespace const& ns);
35+
3436
struct MetadataManager : ReferenceCounted<MetadataManager>, NonCopyable {
3537
explicit MetadataManager(struct DocumentLayer* docLayer) : docLayer(docLayer) {}
3638
~MetadataManager() = default;
3739

3840
Future<Reference<UnboundCollectionContext>> getUnboundCollectionContext(Reference<DocTransaction> tr,
3941
Namespace const& ns,
4042
bool allowSystemNamespace = false,
41-
bool includeIndex = true,
4243
bool createCollectionIfAbsent = true);
4344
Future<Reference<UnboundCollectionContext>> refreshUnboundCollectionContext(Reference<UnboundCollectionContext> cx,
4445
Reference<DocTransaction> tr);

‎src/QLPlan.actor.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,6 +1418,10 @@ ACTOR static Future<Void> doIndexInsert(PlanCheckpoint* checkpoint,
14181418

14191419
Reference<IReadWriteContext> doc = wait(indexInsert->insert(unbound->bindCollectionContext(tr)));
14201420
mcx->bindCollectionContext(tr)->bumpMetadataVersion();
1421+
TraceEvent(SevInfo, "BumpMetadataVersion")
1422+
.detail("reason", "createIndex")
1423+
.detail("ns", fullCollNameToString(ns))
1424+
.detail("index", indexObj.toString());
14211425
output.send(ref(new ScanReturnedContext(doc, -1, Key())));
14221426
throw end_of_stream();
14231427
} catch (Error& e) {
@@ -1590,6 +1594,11 @@ ACTOR static Future<Void> updateIndexStatus(PlanCheckpoint* checkpoint,
15901594
DataValue(DocLayerConstants::CURRENTLY_PROCESSING_DOC_FIELD, DVTypeCode::STRING).encode_key_part());
15911595
indexDoc->clear(DataValue(DocLayerConstants::BUILD_ID_FIELD, DVTypeCode::STRING).encode_key_part());
15921596
mcx->bumpMetadataVersion();
1597+
TraceEvent(SevInfo, "BumpMetadataVersion")
1598+
.detail("reason", "updateIndexStatus")
1599+
.detail("ns", fullCollNameToString(ns))
1600+
.detail("indexID", printable(encodedIndexId))
1601+
.detail("status", newStatus);
15931602
output.send(ref(new ScanReturnedContext(indexDoc, -1, Key())));
15941603
throw end_of_stream();
15951604
} else {

0 commit comments

Comments
 (0)