From b1baa001a2cc2a2e908540baa1cb3189bdc692d3 Mon Sep 17 00:00:00 2001
From: Irene Y Zhang <iyzhang@cs.washington.edu>
Date: Mon, 22 Feb 2016 20:19:35 -0800
Subject: [PATCH] Revert "adding locks to occstore"
This reverts commit 4791cfd35cae41d78d928f9c2442ec03df55d95a.
---
store/benchmark/retwisClient.cc | 1 -
store/common/backend/versionstore.cc | 76 ++++++++++++++++------------
store/common/backend/versionstore.h | 2 +-
store/tapirstore/store.cc | 22 +++-----
store/tapirstore/store.h | 6 +--
5 files changed, 53 insertions(+), 54 deletions(-)
diff --git a/store/benchmark/retwisClient.cc b/store/benchmark/retwisClient.cc
index 8a8037a..631a5ec 100644
--- a/store/benchmark/retwisClient.cc
+++ b/store/benchmark/retwisClient.cc
@@ -11,7 +11,6 @@
#include "store/strongstore/client.h"
#include "store/weakstore/client.h"
#include "store/tapirstore/client.h"
-#include <algorithm>
using namespace std;
diff --git a/store/common/backend/versionstore.cc b/store/common/backend/versionstore.cc
index 6447427..ba2db9e 100644
--- a/store/common/backend/versionstore.cc
+++ b/store/common/backend/versionstore.cc
@@ -42,19 +42,18 @@ VersionedKVStore::inStore(const string &key)
return store.find(key) != store.end() && store[key].size() > 0;
}
-bool
+void
VersionedKVStore::getValue(const string &key, const Timestamp &t, set<VersionedKVStore::VersionedValue>::iterator &it)
{
- if (!inStore(key)) return false;
-
VersionedValue v(t);
it = store[key].upper_bound(v);
// if there is no valid version at this timestamp
- if (it == store[key].begin()) return false;
-
- it--;
- return true;
+ if (it == store[key].begin()) {
+ it = store[key].end();
+ } else {
+ it--;
+ }
}
@@ -77,10 +76,13 @@ VersionedKVStore::get(const string &key, pair<Timestamp, string> &value)
bool
VersionedKVStore::get(const string &key, const Timestamp &t, pair<Timestamp, string> &value)
{
- set<VersionedValue>::iterator it;
- if (getValue(key, t, it)) {
- value = make_pair((*it).write, (*it).value);
- return true;
+ if (inStore(key)) {
+ set<VersionedValue>::iterator it;
+ getValue(key, t, it);
+ if (it != store[key].end()) {
+ value = make_pair((*it).write, (*it).value);
+ return true;
+ }
}
return false;
}
@@ -89,14 +91,18 @@ bool
VersionedKVStore::getRange(const string &key, const Timestamp &t,
pair<Timestamp, Timestamp> &range)
{
- set<VersionedValue>::iterator it;
- if (getValue(key, t, it)) {
- range.first = (*it).write;
- it++;
+ if (inStore(key)) {
+ set<VersionedValue>::iterator it;
+ getValue(key, t, it);
+
if (it != store[key].end()) {
- range.second = (*it).write;
+ range.first = (*it).write;
+ it++;
+ if (it != store[key].end()) {
+ range.second = (*it).write;
+ }
+ return true;
}
- return true;
}
return false;
}
@@ -116,13 +122,17 @@ void
VersionedKVStore::commitGet(const string &key, const Timestamp &readTime, const Timestamp &commit)
{
// Hmm ... could read a key we don't have if we are behind ... do we commit this or wait for the log update?
- set<VersionedValue>::iterator it;
- if (getValue(key, readTime, it)) {
- // figure out if anyone has read this version before
- if (lastReads.find(key) != lastReads.end() &&
- lastReads[key].find((*it).write) != lastReads[key].end()) {
- if (lastReads[key][(*it).write] < commit) {
- lastReads[key][(*it).write] = commit;
+ if (inStore(key)) {
+ set<VersionedValue>::iterator it;
+ getValue(key, readTime, it);
+
+ if (it != store[key].end()) {
+ // figure out if anyone has read this version before
+ if (lastReads.find(key) != lastReads.end() &&
+ lastReads[key].find((*it).write) != lastReads[key].end()) {
+ if (lastReads[key][(*it).write] < commit) {
+ lastReads[key][(*it).write] = commit;
+ }
}
}
} // otherwise, ignore the read
@@ -148,15 +158,17 @@ VersionedKVStore::getLastRead(const string &key, Timestamp &lastRead)
bool
VersionedKVStore::getLastRead(const string &key, const Timestamp &t, Timestamp &lastRead)
{
- set<VersionedValue>::iterator it;
- bool ret = getValue(key, t, it);
- ASSERT(ret);
+ if (inStore(key)) {
+ set<VersionedValue>::iterator it;
+ getValue(key, t, it);
+ ASSERT(it != store[key].end());
- // figure out if anyone has read this version before
- if (lastReads.find(key) != lastReads.end() &&
- lastReads[key].find((*it).write) != lastReads[key].end()) {
- lastRead = lastReads[key][(*it).write];
- return true;
+ // figure out if anyone has read this version before
+ if (lastReads.find(key) != lastReads.end() &&
+ lastReads[key].find((*it).write) != lastReads[key].end()) {
+ lastRead = lastReads[key][(*it).write];
+ return true;
+ }
}
return false;
}
diff --git a/store/common/backend/versionstore.h b/store/common/backend/versionstore.h
index 93e94d1..3443687 100644
--- a/store/common/backend/versionstore.h
+++ b/store/common/backend/versionstore.h
@@ -76,7 +76,7 @@ private:
std::unordered_map< std::string, std::set<VersionedValue> > store;
std::unordered_map< std::string, std::map< Timestamp, Timestamp > > lastReads;
bool inStore(const std::string &key);
- bool getValue(const std::string &key, const Timestamp &t, std::set<VersionedValue>::iterator &it);
+ void getValue(const std::string &key, const Timestamp &t, std::set<VersionedValue>::iterator &it);
};
#endif /* _VERSIONED_KV_STORE_H_ */
diff --git a/store/tapirstore/store.cc b/store/tapirstore/store.cc
index c149d8f..5d1875e 100644
--- a/store/tapirstore/store.cc
+++ b/store/tapirstore/store.cc
@@ -43,9 +43,9 @@ int
Store::Get(uint64_t id, const string &key, pair<Timestamp,string> &value)
{
Debug("[%lu] GET %s", id, key.c_str());
- std::lock_guard<std::mutex> lck (mtx);
-
- if (store.get(key, value)) {
+
+ bool ret = store.get(key, value);
+ if (ret) {
Debug("Value: %s at <%lu, %lu>", value.second.c_str(), value.first.getTimestamp(), value.first.getID());
return REPLY_OK;
} else {
@@ -57,9 +57,9 @@ int
Store::Get(uint64_t id, const string &key, const Timestamp ×tamp, pair<Timestamp,string> &value)
{
Debug("[%lu] GET %s at <%lu, %lu>", id, key.c_str(), timestamp.getTimestamp(), timestamp.getID());
- std::lock_guard<std::mutex> lck (mtx);
- if (store.get(key, timestamp, value)) {
+ bool ret = store.get(key, timestamp, value);
+ if (ret) {
return REPLY_OK;
} else {
return REPLY_FAIL;
@@ -71,8 +71,6 @@ Store::Prepare(uint64_t id, const Transaction &txn, const Timestamp ×tamp,
{
Debug("[%lu] START PREPARE", id);
- std::lock_guard<std::mutex> lck (mtx);
-
if (prepared.find(id) != prepared.end()) {
if (prepared[id].first == timestamp) {
Warning("[%lu] Already Prepared!", id);
@@ -214,9 +212,7 @@ Store::Commit(uint64_t id, uint64_t timestamp)
{
Debug("[%lu] COMMIT", id);
-
- std::lock_guard<std::mutex> lck (mtx);
-
+
// Nope. might not find it
//ASSERT(prepared.find(id) != prepared.end());
@@ -249,8 +245,7 @@ void
Store::Abort(uint64_t id, const Transaction &txn)
{
Debug("[%lu] ABORT", id);
- std::lock_guard<std::mutex> lck (mtx);
-
+
if (prepared.find(id) != prepared.end()) {
prepared.erase(id);
}
@@ -259,11 +254,9 @@ Store::Abort(uint64_t id, const Transaction &txn)
void
Store::Load(const string &key, const string &value, const Timestamp ×tamp)
{
- std::lock_guard<std::mutex> lck (mtx);
store.put(key, value, timestamp);
}
-// hold lock when using this function
void
Store::GetPreparedWrites(unordered_map<string, set<Timestamp>> &writes)
{
@@ -275,7 +268,6 @@ Store::GetPreparedWrites(unordered_map<string, set<Timestamp>> &writes)
}
}
-// hold lock when using this function
void
Store::GetPreparedReads(unordered_map<string, set<Timestamp>> &reads)
{
diff --git a/store/tapirstore/store.h b/store/tapirstore/store.h
index 0924f77..b722b0a 100644
--- a/store/tapirstore/store.h
+++ b/store/tapirstore/store.h
@@ -42,7 +42,6 @@
#include <set>
#include <unordered_map>
#include <vector>
-#include <mutex>
namespace tapirstore {
@@ -69,11 +68,8 @@ private:
// Data store
VersionedKVStore store;
- // list of prepared transactions
+ // TODO: comment this.
std::unordered_map<uint64_t, std::pair<Timestamp, Transaction>> prepared;
-
- // protect both store and prepared
- std::mutex mtx;
void GetPreparedWrites(std::unordered_map< std::string, std::set<Timestamp> > &writes);
void GetPreparedReads(std::unordered_map< std::string, std::set<Timestamp> > &reads);
--
GitLab