code review changes: additional test checks, cleanup extra code, mark internal option hidden

This commit is contained in:
Jon Fu 2022-02-10 17:01:09 -05:00
parent 458e708272
commit 2e63ac6963
5 changed files with 16 additions and 7 deletions

View File

@ -203,7 +203,7 @@ struct GetReadVersionReply : public BasicLoadBalancedReply {
TransactionTagMap<ClientTagThrottleLimits> tagThrottleInfo;
GetReadVersionReply() : version(invalidVersion), locked(false), rkThrottled(false) {}
GetReadVersionReply() : version(invalidVersion), locked(false) {}
template <class Ar>
void serialize(Ar& ar) {

View File

@ -1066,8 +1066,10 @@ ACTOR static Future<Void> backgroundGrvUpdater(DatabaseContext* cx) {
wait(tr.onError(e));
}
} else {
wait(delay(0.001 + std::min(CLIENT_KNOBS->MAX_PROXY_CONTACT_LAG - (curTime - lastProxyTime),
(CLIENT_KNOBS->MAX_VERSION_CACHE_LAG - grvDelay) - (curTime - lastTime))));
wait(
delay(std::max(0.001,
std::min(CLIENT_KNOBS->MAX_PROXY_CONTACT_LAG - (curTime - lastProxyTime),
(CLIENT_KNOBS->MAX_VERSION_CACHE_LAG - grvDelay) - (curTime - lastTime)))));
}
}
} catch (Error& e) {

View File

@ -44,8 +44,6 @@ struct ReadYourWritesTransactionOptions {
int maxRetries;
int snapshotRywEnabled;
bool bypassUnreadable : 1;
bool useGrvCache : 1;
bool skipGrvCache : 1;
ReadYourWritesTransactionOptions() {}
explicit ReadYourWritesTransactionOptions(Transaction const& tr);

View File

@ -295,7 +295,8 @@ description is not currently required but encouraged.
<Option name="use_grv_cache" code="1101"
description="Allows this transaction to use cached GRV from the database context. Defaults to off. Upon first usage, starts a background updater to periodically update the cache to avoid stale read versions." />
<Option name="skip_grv_cache" code="1102"
description="Specifically instruct this transaction to NOT use cached GRV. Primarily used for the read version cache's background updater to avoid attempting to read a cached entry in specific situations." />
description="Specifically instruct this transaction to NOT use cached GRV. Primarily used for the read version cache's background updater to avoid attempting to read a cached entry in specific situations."
hidden="true"/>
</Scope>
<!-- The enumeration values matter - do not change them without

View File

@ -217,6 +217,14 @@ struct SidebandSingleWorkload : TestWorkload {
tr.getReadVersion().get()); // will assert that ReadVersion is set
++self->consistencyErrors;
} else if (val.get() != LiteralStringRef("deadbeef")) {
// If we read something NOT "deadbeef" and there was no commit_unknown_result,
// the cache somehow read a stale version of our key
if (message.commitVersion != invalidVersion) {
TraceEvent(SevError, "CausalConsistencyError2", self->interf.id())
.detail("MessageKey", messageKey.toString().c_str());
++self->consistencyErrors;
break;
}
// check again without cache, and if it's the same, that's expected
state Transaction tr2(cx);
state Optional<Value> val2;
@ -230,7 +238,7 @@ struct SidebandSingleWorkload : TestWorkload {
}
}
if (val != val2) {
TraceEvent(SevError, "CausalConsistencyError2", self->interf.id())
TraceEvent(SevError, "CausalConsistencyError3", self->interf.id())
.detail("MessageKey", messageKey.toString().c_str())
.detail("Val1", val)
.detail("Val2", val2)