From 518f83aaa396b8e445b155e3652661d52f8cbcd2 Mon Sep 17 00:00:00 2001
From: Dominik Gleich <dominik.gleich@memgraph.io>
Date: Tue, 24 Apr 2018 16:16:20 +0200
Subject: [PATCH] Optimize gc clog rpc calls
Summary:
GC called clog info on transactions which were not completed even though it
could deduce that they are not finished. By avoiding calling clog info on them
we can lower the number of rpc calls and remove a strange behaviour of gc
appearing to be hanging while recovering a worker.
Reviewers: mculinovic, mferencevic, buda
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1377
---
src/mvcc/record.hpp | 36 +++++++++++++++++++++---------------
src/mvcc/version_list.hpp | 7 ++++++-
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/src/mvcc/record.hpp b/src/mvcc/record.hpp
index 15df71f5a..e305cd50f 100644
--- a/src/mvcc/record.hpp
+++ b/src/mvcc/record.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <atomic>
+#include <experimental/optional>
#include <iostream>
#include "transactions/commit_log.hpp"
@@ -85,8 +86,8 @@ class Record : public Version<T> {
auto exp_id = tx_.exp.load();
// a record is NOT visible if:
- // 1. it creating transaction aborted (last check)
- // OR
+ // 1. it creating transaction aborted (last check), and is also older than
+ // the current oldest active transaction (optimization) OR
// 2. a) it's expiration is not 0 (some transaction expired it)
// AND
// b) the expiring transaction is older than latest active
@@ -98,7 +99,7 @@ class Record : public Version<T> {
// newer transactions)
return (exp_id != 0 && exp_id < snapshot.back() &&
committed(Hints::kExp, engine) && !snapshot.contains(exp_id)) ||
- cre_aborted(engine);
+ (tx_.cre.load() < snapshot.back() && cre_aborted(engine));
}
// TODO: Test this
@@ -142,11 +143,14 @@ class Record : public Version<T> {
/**
* Makes sure that create and expiry are in sync with hints if they are
- * committed or aborted.
+ * committed or aborted and are before the `tx_cutoff`.
+ * `tx_cutoff` exists as a performance optimization to avoid setting hint bits
+ * on records for which we don't need to have a guarantee that they are set as
+ * part of GC hints setting procedure
*/
- void populate_hints(const tx::Engine &engine) {
- populate_hint_if_possible(engine, Hints::kCre);
- if (!populate_hint_if_possible(engine, Hints::kExp)) {
+ void populate_hints(const tx::Engine &engine, tx::TransactionId tx_cutoff) {
+ populate_hint_if_possible(engine, Hints::kCre, tx_cutoff);
+ if (!populate_hint_if_possible(engine, Hints::kExp, tx_cutoff)) {
// Exp is aborted and we can't set the hint, this way we don't have to set
// the hint because an aborted transaction which expires a record is the
// same thing as a non-expired record
@@ -232,22 +236,24 @@ class Record : public Version<T> {
}
/**
- * Populates hint if it is not set for the given create/expiry mask.
- * Note that it doesn't set hint bits for expiry transactions which
- * abort because it's too expensive to maintain correctness of those hints
- * with regards to race conditions
+ * Populates hint if it is not set for the given create/expiry mask and is
+ * before the `tx_cutoff` if specified. Note that it doesn't set hint bits for
+ * expiry transactions which abort because it's too expensive to maintain
+ * correctness of those hints with regards to race conditions
* @returns - true if hints are now equal to transaction status
* (committed/aborted), will only be false if we are trying to set hint for
* aborted transaction which is this records expiry
*/
- bool populate_hint_if_possible(const tx::Engine &engine,
- const uint8_t mask) const {
+ bool populate_hint_if_possible(
+ const tx::Engine &engine, const uint8_t mask,
+ const std::experimental::optional<tx::TransactionId> tx_cutoff =
+ std::experimental::nullopt) const {
DCHECK(mask == Hints::kCre || mask == Hints::kExp)
<< "Mask should be either for creation or expiration";
if (hints_.Get(mask)) return true;
auto id = mask == Hints::kCre ? tx_.cre.load() : tx_.exp.load();
- // Nothing to do here
- if (!id) return true;
+ // Nothing to do here if there is no id or id is larger than tx_cutoff
+ if (!id || (tx_cutoff && id >= *tx_cutoff)) return true;
auto info = engine.Info(id);
if (info.is_committed()) {
hints_.Set(mask & Hints::kCmt);
diff --git a/src/mvcc/version_list.hpp b/src/mvcc/version_list.hpp
index 02234b0cd..96cfa7333 100644
--- a/src/mvcc/version_list.hpp
+++ b/src/mvcc/version_list.hpp
@@ -99,7 +99,12 @@ class VersionList {
T *current = head;
T *oldest_visible_record = nullptr;
while (current) {
- current->populate_hints(engine);
+ // Populate hints only when needed to avoid excessive rpc calls on
+ // workers.
+ // snapshot.back() corresponds to the oldest active transaction,
+ // and this makes it set only hint bits when the creating or expiring
+ // transaction of a record is older than that)
+ current->populate_hints(engine, snapshot.back());
if (!current->is_not_visible_from(snapshot, engine))
oldest_visible_record = current;
current = current->next();