Fix transaction option consistency in TagThrottleInfo getter (#9513)

* Fix transaction option consistency in TagThrottleInfo getter

Subroutine of getter actor function for throttled and recommended tags
was, upon retry, resetting the transaction object which the caller also uses,
resetting the transaction option and causing a key_outside_legal_range by caller

Also, allowing a subroutine to conditionally, non-trivially modify the passed object
(i.e. transaction reset) is a risky pattern.

Fix: confine subroutine's responsibility to "attempting to" fetch and parse
"autoThrottlingEnabled" key. Let the calling function reset the object if needed.

* Apply Clang format
This commit is contained in:
Junhyun Shim 2023-02-28 23:47:26 +01:00 committed by GitHub
parent 739144da6d
commit 6b26f5a6da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -254,38 +254,34 @@ namespace ThrottleApi {
// or using IClientAPI like IDatabase, ITransaction
ACTOR template <class Tr>
Future<bool> getValidAutoEnabled(Reference<Tr> tr) {
state bool result;
loop {
// hold the returned standalone object's memory
state typename Tr::template FutureT<Optional<Value>> valueF = tr->get(tagThrottleAutoEnabledKey);
Optional<Value> value = wait(safeThreadFutureToFuture(valueF));
if (!value.present()) {
tr->reset();
wait(delay(CLIENT_KNOBS->DEFAULT_BACKOFF));
continue;
} else if (value.get() == "1"_sr) {
result = true;
} else if (value.get() == "0"_sr) {
result = false;
} else {
TraceEvent(SevWarnAlways, "InvalidAutoTagThrottlingValue").detail("Value", value.get());
tr->reset();
wait(delay(CLIENT_KNOBS->DEFAULT_BACKOFF));
continue;
}
return result;
};
Future<Optional<bool>> getValidAutoEnabled(Reference<Tr> tr) {
// hold the returned standalone object's memory
state typename Tr::template FutureT<Optional<Value>> valueF = tr->get(tagThrottleAutoEnabledKey);
Optional<Value> value = wait(safeThreadFutureToFuture(valueF));
if (!value.present()) {
return {};
} else if (value.get() == "1"_sr) {
return true;
} else if (value.get() == "0"_sr) {
return false;
} else {
TraceEvent(SevWarnAlways, "InvalidAutoTagThrottlingValue").detail("Value", value.get());
return {};
}
}
ACTOR template <class DB>
Future<std::vector<TagThrottleInfo>> getRecommendedTags(Reference<DB> db, int limit) {
state Reference<typename DB::TransactionT> tr = db->createTransaction();
loop {
tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
try {
bool enableAuto = wait(getValidAutoEnabled(tr));
if (enableAuto) {
tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
Optional<bool> enableAuto = wait(getValidAutoEnabled(tr));
if (!enableAuto.present()) {
tr->reset();
wait(delay(CLIENT_KNOBS->DEFAULT_BACKOFF));
continue;
} else if (enableAuto.get()) {
return std::vector<TagThrottleInfo>();
}
state typename DB::TransactionT::template FutureT<RangeResult> f =
@ -307,15 +303,19 @@ ACTOR template <class DB>
Future<std::vector<TagThrottleInfo>>
getThrottledTags(Reference<DB> db, int limit, ContainsRecommended containsRecommended = ContainsRecommended::False) {
state Reference<typename DB::TransactionT> tr = db->createTransaction();
state bool reportAuto = containsRecommended;
state Optional<bool> reportAuto;
loop {
tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
try {
if (!containsRecommended) {
wait(store(reportAuto, getValidAutoEnabled(tr)));
tr->setOption(FDBTransactionOptions::READ_SYSTEM_KEYS);
wait(store(reportAuto, getValidAutoEnabled(tr)));
if (!reportAuto.present()) {
tr->reset();
wait(delay(CLIENT_KNOBS->DEFAULT_BACKOFF));
continue;
}
state typename DB::TransactionT::template FutureT<RangeResult> f = tr->getRange(
reportAuto ? tagThrottleKeys : KeyRangeRef(tagThrottleKeysPrefix, tagThrottleAutoKeysPrefix), limit);
reportAuto.get() ? tagThrottleKeys : KeyRangeRef(tagThrottleKeysPrefix, tagThrottleAutoKeysPrefix),
limit);
RangeResult throttles = wait(safeThreadFutureToFuture(f));
std::vector<TagThrottleInfo> results;
for (auto throttle : throttles) {