Skip to content

Commit a4430b9

Browse files
sbodagalasbodagala
authored
Compare storage replicas on reads (apple#11235)
* - Compare storage replicas on reads (in "loadBalance()") * - Do consistency check on reads in loadbalance * - Do replica consistency check in the case where loadBalance issues requests to multiple storage servers * - Address a state variable related bug * - Code formatting * - API simplification * - Simplify code * - Code formatting * - Address a review comment
1 parent 2f50c49 commit a4430b9

14 files changed

+245
-64
lines changed

‎fdbclient/BackupAgentBase.actor.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,8 @@ ACTOR Future<Void> readCommitted(Database cx,
549549
tr.setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
550550
if (lockAware)
551551
tr.setOption(FDBTransactionOptions::LOCK_AWARE);
552+
if (CLIENT_KNOBS->ENABLE_REPLICA_CONSISTENCY_CHECK_ON_BACKUP_READS)
553+
tr.setOption(FDBTransactionOptions::ENABLE_REPLICA_CONSISTENCY_CHECK);
552554

553555
// add lock
554556
releaser.release();

‎fdbclient/ClientKnobs.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ void ClientKnobs::initialize(Randomize randomize) {
197197
init( RESTORE_RANGES_READ_BATCH, 10000 );
198198
init( BLOB_GRANULE_RESTORE_CHECK_INTERVAL, 10 );
199199
init( BACKUP_CONTAINER_LOCAL_ALLOW_RELATIVE_PATH, false );
200+
init( ENABLE_REPLICA_CONSISTENCY_CHECK_ON_BACKUP_READS, true );
200201

201202
// Configuration
202203
init( DEFAULT_AUTO_COMMIT_PROXIES, 3 );

‎fdbclient/NativeAPI.actor.cpp

Lines changed: 41 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,10 @@ Future<REPLY_TYPE(Request)> loadBalance(
131131
TaskPriority taskID = TaskPriority::DefaultPromiseEndpoint,
132132
AtMostOnce atMostOnce =
133133
AtMostOnce::False, // if true, throws request_maybe_delivered() instead of retrying automatically
134-
QueueModel* model = nullptr) {
134+
QueueModel* model = nullptr,
135+
bool compareReplicas = false) {
135136
if (alternatives->hasCaches) {
136-
return loadBalance(alternatives->locations(), channel, request, taskID, atMostOnce, model);
137+
return loadBalance(alternatives->locations(), channel, request, taskID, atMostOnce, model, compareReplicas);
137138
}
138139
return fmap(
139140
[ctx](auto const& res) {
@@ -142,7 +143,7 @@ Future<REPLY_TYPE(Request)> loadBalance(
142143
}
143144
return res;
144145
},
145-
loadBalance(alternatives->locations(), channel, request, taskID, atMostOnce, model));
146+
loadBalance(alternatives->locations(), channel, request, taskID, atMostOnce, model, compareReplicas));
146147
}
147148
} // namespace
148149

@@ -3685,21 +3686,22 @@ ACTOR Future<Optional<Value>> getValue(Reference<TransactionState> trState,
36853686
when(wait(trState->cx->connectionFileChanged())) {
36863687
throw transaction_too_old();
36873688
}
3688-
when(GetValueReply _reply = wait(loadBalance(
3689-
trState->cx.getPtr(),
3690-
locationInfo.locations,
3691-
&StorageServerInterface::getValue,
3692-
GetValueRequest(span.context,
3693-
useTenant ? trState->getTenantInfo() : TenantInfo(),
3694-
key,
3695-
trState->readVersion(),
3696-
trState->cx->sampleReadTags() ? trState->options.readTags
3697-
: Optional<TagSet>(),
3698-
readOptions,
3699-
ssLatestCommitVersions),
3700-
TaskPriority::DefaultPromiseEndpoint,
3701-
AtMostOnce::False,
3702-
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr))) {
3689+
when(GetValueReply _reply = wait(
3690+
loadBalance(trState->cx.getPtr(),
3691+
locationInfo.locations,
3692+
&StorageServerInterface::getValue,
3693+
GetValueRequest(span.context,
3694+
useTenant ? trState->getTenantInfo() : TenantInfo(),
3695+
key,
3696+
trState->readVersion(),
3697+
trState->cx->sampleReadTags() ? trState->options.readTags
3698+
: Optional<TagSet>(),
3699+
readOptions,
3700+
ssLatestCommitVersions),
3701+
TaskPriority::DefaultPromiseEndpoint,
3702+
AtMostOnce::False,
3703+
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr,
3704+
trState->options.enableReplicaConsistencyCheck))) {
37033705
reply = _reply;
37043706
}
37053707
}
@@ -3832,14 +3834,15 @@ ACTOR Future<Key> getKey(Reference<TransactionState> trState, KeySelector k, Use
38323834
when(wait(trState->cx->connectionFileChanged())) {
38333835
throw transaction_too_old();
38343836
}
3835-
when(GetKeyReply _reply = wait(loadBalance(
3836-
trState->cx.getPtr(),
3837-
locationInfo.locations,
3838-
&StorageServerInterface::getKey,
3839-
req,
3840-
TaskPriority::DefaultPromiseEndpoint,
3841-
AtMostOnce::False,
3842-
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr))) {
3837+
when(GetKeyReply _reply = wait(
3838+
loadBalance(trState->cx.getPtr(),
3839+
locationInfo.locations,
3840+
&StorageServerInterface::getKey,
3841+
req,
3842+
TaskPriority::DefaultPromiseEndpoint,
3843+
AtMostOnce::False,
3844+
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr,
3845+
trState->options.enableReplicaConsistencyCheck))) {
38433846
reply = _reply;
38443847
}
38453848
}
@@ -4365,7 +4368,8 @@ Future<RangeResultFamily> getExactRange(Reference<TransactionState> trState,
43654368
req,
43664369
TaskPriority::DefaultPromiseEndpoint,
43674370
AtMostOnce::False,
4368-
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr))) {
4371+
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr,
4372+
trState->options.enableReplicaConsistencyCheck))) {
43694373
rep = _rep;
43704374
}
43714375
}
@@ -4769,7 +4773,8 @@ Future<RangeResultFamily> getRange(Reference<TransactionState> trState,
47694773
req,
47704774
TaskPriority::DefaultPromiseEndpoint,
47714775
AtMostOnce::False,
4772-
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr));
4776+
trState->cx->enableLocalityLoadBalance ? &trState->cx->queueModel : nullptr,
4777+
trState->options.enableReplicaConsistencyCheck));
47734778
rep = _rep;
47744779
++trState->cx->transactionPhysicalReadsCompleted;
47754780
} catch (Error&) {
@@ -5041,7 +5046,7 @@ static Future<Void> tssStreamComparison(Request request,
50415046
(g_network->isSimulated() && g_simulator->tssMode == ISimulator::TSSMode::EnabledDropMutations)
50425047
? SevWarnAlways
50435048
: SevError,
5044-
TSS_mismatchTraceName(request));
5049+
LB_mismatchTraceName(request, TSS_COMPARISON));
50455050
mismatchEvent.setMaxEventLength(FLOW_KNOBS->TSS_LARGE_TRACE_SIZE);
50465051
mismatchEvent.detail("TSSID", tssData.tssId);
50475052

@@ -5064,7 +5069,7 @@ static Future<Void> tssStreamComparison(Request request,
50645069
g_simulator->tssMode == ISimulator::TSSMode::EnabledDropMutations)
50655070
? SevWarnAlways
50665071
: SevError,
5067-
TSS_mismatchTraceName(request));
5072+
LB_mismatchTraceName(request, TSS_COMPARISON));
50685073
summaryEvent.detail("TSSID", tssData.tssId).detail("MismatchId", mismatchUID);
50695074
}
50705075
} else {
@@ -6202,6 +6207,7 @@ void TransactionOptions::clear() {
62026207
skipGrvCache = false;
62036208
rawAccess = false;
62046209
bypassStorageQuota = false;
6210+
enableReplicaConsistencyCheck = false;
62056211
}
62066212

62076213
TransactionOptions::TransactionOptions() {
@@ -7285,6 +7291,11 @@ void Transaction::setOption(FDBTransactionOptions::Option option, Optional<Strin
72857291
trState->readOptions.withDefault(ReadOptions()).type = ReadType::HIGH;
72867292
break;
72877293

7294+
case FDBTransactionOptions::ENABLE_REPLICA_CONSISTENCY_CHECK:
7295+
validateOptionValueNotPresent(value);
7296+
trState->options.enableReplicaConsistencyCheck = true;
7297+
break;
7298+
72887299
default:
72897300
break;
72907301
}

‎fdbclient/StorageServerInterface.cpp

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ bool TSS_doCompare(const GetValueReply& src, const GetValueReply& tss) {
3939
}
4040

4141
template <>
42-
const char* TSS_mismatchTraceName(const GetValueRequest& req) {
43-
return "TSSMismatchGetValue";
42+
const char* LB_mismatchTraceName(const GetValueRequest& req, const ComparisonType& type) {
43+
return type == TSS_COMPARISON ? "TSSMismatchGetValue" : "ReplicaMismatchGetValue";
4444
}
4545

4646
template <>
@@ -98,8 +98,8 @@ bool TSS_doCompare(const GetKeyReply& src, const GetKeyReply& tss) {
9898
}
9999

100100
template <>
101-
const char* TSS_mismatchTraceName(const GetKeyRequest& req) {
102-
return "TSSMismatchGetKey";
101+
const char* LB_mismatchTraceName(const GetKeyRequest& req, const ComparisonType& type) {
102+
return type == TSS_COMPARISON ? "TSSMismatchGetKey" : "ReplicaMismatchGetKey";
103103
}
104104

105105
template <>
@@ -122,8 +122,8 @@ bool TSS_doCompare(const GetKeyValuesReply& src, const GetKeyValuesReply& tss) {
122122
}
123123

124124
template <>
125-
const char* TSS_mismatchTraceName(const GetKeyValuesRequest& req) {
126-
return "TSSMismatchGetKeyValues";
125+
const char* LB_mismatchTraceName(const GetKeyValuesRequest& req, const ComparisonType& type) {
126+
return type == TSS_COMPARISON ? "TSSMismatchGetKeyValues" : "ReplicaMismatchGetKeyValues";
127127
}
128128

129129
static void traceKeyValuesSummary(TraceEvent& event,
@@ -222,8 +222,8 @@ bool TSS_doCompare(const GetMappedKeyValuesReply& src, const GetMappedKeyValuesR
222222
}
223223

224224
template <>
225-
const char* TSS_mismatchTraceName(const GetMappedKeyValuesRequest& req) {
226-
return "TSSMismatchGetMappedKeyValues";
225+
const char* LB_mismatchTraceName(const GetMappedKeyValuesRequest& req, const ComparisonType& type) {
226+
return type == TSS_COMPARISON ? "TSSMismatchGetMappedKeyValues" : "ReplicaMismatchGetMappedKeyValues";
227227
}
228228

229229
template <>
@@ -252,8 +252,8 @@ bool TSS_doCompare(const GetKeyValuesStreamReply& src, const GetKeyValuesStreamR
252252
}
253253

254254
template <>
255-
const char* TSS_mismatchTraceName(const GetKeyValuesStreamRequest& req) {
256-
return "TSSMismatchGetKeyValuesStream";
255+
const char* LB_mismatchTraceName(const GetKeyValuesStreamRequest& req, const ComparisonType& type) {
256+
return type == TSS_COMPARISON ? "TSSMismatchGetKeyValuesStream" : "ReplicaMismatchGetKeyValuesStream";
257257
}
258258

259259
// TODO this is all duplicated from above, simplify?
@@ -282,7 +282,7 @@ bool TSS_doCompare(const WatchValueReply& src, const WatchValueReply& tss) {
282282
}
283283

284284
template <>
285-
const char* TSS_mismatchTraceName(const WatchValueRequest& req) {
285+
const char* LB_mismatchTraceName(const WatchValueRequest& req, const ComparisonType& type) {
286286
ASSERT(false);
287287
return "";
288288
}
@@ -302,7 +302,7 @@ bool TSS_doCompare(const SplitMetricsReply& src, const SplitMetricsReply& tss) {
302302
}
303303

304304
template <>
305-
const char* TSS_mismatchTraceName(const SplitMetricsRequest& req) {
305+
const char* LB_mismatchTraceName(const SplitMetricsRequest& req, const ComparisonType& type) {
306306
ASSERT(false);
307307
return "";
308308
}
@@ -322,7 +322,7 @@ bool TSS_doCompare(const ReadHotSubRangeReply& src, const ReadHotSubRangeReply&
322322
}
323323

324324
template <>
325-
const char* TSS_mismatchTraceName(const ReadHotSubRangeRequest& req) {
325+
const char* LB_mismatchTraceName(const ReadHotSubRangeRequest& req, const ComparisonType& type) {
326326
ASSERT(false);
327327
return "";
328328
}
@@ -342,7 +342,7 @@ bool TSS_doCompare(const SplitRangeReply& src, const SplitRangeReply& tss) {
342342
}
343343

344344
template <>
345-
const char* TSS_mismatchTraceName(const SplitRangeRequest& req) {
345+
const char* LB_mismatchTraceName(const SplitRangeRequest& req, const ComparisonType& type) {
346346
ASSERT(false);
347347
return "";
348348
}
@@ -363,7 +363,7 @@ bool TSS_doCompare(const OverlappingChangeFeedsReply& src, const OverlappingChan
363363
}
364364

365365
template <>
366-
const char* TSS_mismatchTraceName(const OverlappingChangeFeedsRequest& req) {
366+
const char* LB_mismatchTraceName(const OverlappingChangeFeedsRequest& req, const ComparisonType& type) {
367367
ASSERT(false);
368368
return "";
369369
}
@@ -386,7 +386,7 @@ bool TSS_doCompare(const StorageMetrics& src, const StorageMetrics& tss) {
386386
}
387387

388388
template <>
389-
const char* TSS_mismatchTraceName(const WaitMetricsRequest& req) {
389+
const char* LB_mismatchTraceName(const WaitMetricsRequest& req, const ComparisonType& type) {
390390
ASSERT(false);
391391
return "";
392392
}
@@ -406,7 +406,7 @@ bool TSS_doCompare(const BlobGranuleFileReply& src, const BlobGranuleFileReply&
406406
}
407407

408408
template <>
409-
const char* TSS_mismatchTraceName(const BlobGranuleFileRequest& req) {
409+
const char* LB_mismatchTraceName(const BlobGranuleFileRequest& req, const ComparisonType& type) {
410410
ASSERT(false);
411411
return "";
412412
}

‎fdbclient/include/fdbclient/ClientKnobs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ class SWIFT_CXX_IMMORTAL_SINGLETON_TYPE ClientKnobs : public KnobsImpl<ClientKno
197197
int RESTORE_RANGES_READ_BATCH;
198198
int BLOB_GRANULE_RESTORE_CHECK_INTERVAL;
199199
bool BACKUP_CONTAINER_LOCAL_ALLOW_RELATIVE_PATH;
200+
bool ENABLE_REPLICA_CONSISTENCY_CHECK_ON_BACKUP_READS;
200201

201202
// Configuration
202203
int32_t DEFAULT_AUTO_COMMIT_PROXIES;

‎fdbclient/include/fdbclient/NativeAPI.actor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ struct TransactionOptions {
164164
bool skipGrvCache : 1;
165165
bool rawAccess : 1;
166166
bool bypassStorageQuota : 1;
167+
bool enableReplicaConsistencyCheck : 1;
167168

168169
TransactionPriority priority;
169170

‎fdbclient/vexillographer/fdb.options

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,8 @@ description is not currently required but encouraged.
351351
description="Attach given authorization token to the transaction such that subsequent tenant-aware requests are authorized"
352352
paramType="String" paramDescription="A JSON Web Token authorized to access data belonging to one or more tenants, indicated by 'tenants' claim of the token's payload."
353353
persistent="true" sensitive="true"/>
354+
<Option name="enable_replica_consistency_check" code="4000"
355+
description="Enables replica consistency check, which compares the results returned by all storage server replicas for a given read request, in client-side load balancer." />
354356
</Scope>
355357

356358
<!-- The enumeration values matter - do not change them without

0 commit comments

Comments
 (0)