Commit b0ee9e0c authored by Georgios Bitzes's avatar Georgios Bitzes
Browse files

Improve error messages, differentiate between "renewed" and "acquired" cases

parent 8403f856
Pipeline #430142 passed with stages
in 50 minutes and 41 seconds
......@@ -286,7 +286,7 @@ RedisEncodedResponse RedisDispatcher::dispatchWrite(StagingArea &stagingArea, Re
return Formatter::ok();
}
case RedisCommand::TIMESTAMPED_LEASE_ACQUIRE: {
qdb_assert(request.size() == 5); // Internal command, safe to assume this size
if(request.size() != 5) return Formatter::errArgs("lease_acquire");
int64_t duration = 0;
if(!my_strtoll(request[3], duration) || duration < 1) {
......@@ -295,17 +295,22 @@ RedisEncodedResponse RedisDispatcher::dispatchWrite(StagingArea &stagingArea, Re
qdb_assert(request[4].size() == 8u);
bool acquired;
ClockValue timestamp = binaryStringToUnsignedInt(request[4].c_str());
LeaseInfo leaseInfo;
rocksdb::Status st = store.lease_acquire(stagingArea, request[1], request[2], binaryStringToUnsignedInt(request[4].c_str()), duration, leaseInfo, acquired);
LeaseAcquisitionStatus status = store.lease_acquire(stagingArea, request[1], request[2], timestamp, duration, leaseInfo);
if(!st.ok()) return Formatter::fromStatus(st);
if(acquired) {
return Formatter::ok();
if(status == LeaseAcquisitionStatus::kKeyTypeMismatch) {
return Formatter::err("Invalid Argument: WRONGTYPE Operation against a key holding the wrong kind of value");
}
else if(status == LeaseAcquisitionStatus::kAcquired) {
return Formatter::status("ACQUIRED");
}
else if(status == LeaseAcquisitionStatus::kRenewed) {
return Formatter::status("RENEWED");
}
else {
return Formatter::err("lease already held");
qdb_assert(status == LeaseAcquisitionStatus::kFailedDueToOtherOwner);
return Formatter::err(SSTR("lease held by '" << leaseInfo.getValue() << "', time remaining " << leaseInfo.getDeadline() - timestamp << " ms"));
}
}
default: {
......
......@@ -1139,7 +1139,7 @@ void StateMachine::getClock(ClockValue &value) {
getClock(stagingArea, value);
}
rocksdb::Status StateMachine::lease_acquire(StagingArea &stagingArea, const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info, bool &acquired) {
LeaseAcquisitionStatus StateMachine::lease_acquire(StagingArea &stagingArea, const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info) {
qdb_assert(!value.empty());
// First, some timekeeping, update clock time if necessary.
......@@ -1147,7 +1147,7 @@ rocksdb::Status StateMachine::lease_acquire(StagingArea &stagingArea, const std:
// Ensure the key pointed to is either a lease, or non-existent.
WriteOperation operation(stagingArea, key, KeyType::kLease);
if(!operation.valid()) return wrong_type();
if(!operation.valid()) return LeaseAcquisitionStatus::kKeyTypeMismatch;
// Quick check that no-one else holds the lease right now.
// Could it be that the lease has actually expired? Not at this point.
......@@ -1161,10 +1161,9 @@ rocksdb::Status StateMachine::lease_acquire(StagingArea &stagingArea, const std:
if(st.ok()) {
if(oldLeaseHolder != value) {
KeyDescriptor &descriptor = operation.descriptor();
acquired = false;
info = LeaseInfo(oldLeaseHolder, descriptor.getStartIndex(), descriptor.getEndIndex());
operation.cancel();
return rocksdb::Status::InvalidArgument(SSTR("lease being currently held by " << oldLeaseHolder << ", remaining time: " << descriptor.getEndIndex() - clockUpdate << " ms" ));
return LeaseAcquisitionStatus::kFailedDueToOtherOwner;
}
}
......@@ -1172,6 +1171,7 @@ rocksdb::Status StateMachine::lease_acquire(StagingArea &stagingArea, const std:
// simply an extension request, or this is a new lease altogether.
KeyDescriptor &descriptor = operation.descriptor();
bool extended = operation.keyExists();
if(operation.keyExists()) {
// Lease extension.. need to wipe out old pending expiration event
ExpirationEventLocator oldEvent(descriptor.getEndIndex(), key);
......@@ -1190,9 +1190,11 @@ rocksdb::Status StateMachine::lease_acquire(StagingArea &stagingArea, const std:
// Update lease value.
operation.write(value);
acquired = true;
info = LeaseInfo(value, descriptor.getStartIndex(), descriptor.getEndIndex());
return operation.finalize(value.size(), true);
operation.finalize(value.size(), true);
if(extended) return LeaseAcquisitionStatus::kRenewed;
return LeaseAcquisitionStatus::kAcquired;
}
rocksdb::Status StateMachine::lease_release(StagingArea &stagingArea, const std::string &key) {
......@@ -1553,7 +1555,7 @@ void StateMachine::commitTransaction(rocksdb::WriteBatchWithIndex &wb, LogIndex
#define CHAIN(index, func, ...) { \
StagingArea stagingArea(*this); \
rocksdb::Status st = this->func(stagingArea, ## __VA_ARGS__); \
auto st = this->func(stagingArea, ## __VA_ARGS__); \
stagingArea.commit(index); \
return st; \
}
......@@ -1720,8 +1722,8 @@ rocksdb::Status StateMachine::lhset(const std::string &key, const std::string &f
CHAIN(index, lhset, key, field, hint, value, fieldcreated);
}
rocksdb::Status StateMachine::lease_acquire(const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info, bool &acquired, LogIndex index) {
CHAIN(index, lease_acquire, key, value, clockUpdate, duration, info, acquired);
LeaseAcquisitionStatus StateMachine::lease_acquire(const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info, LogIndex index) {
CHAIN(index, lease_acquire, key, value, clockUpdate, duration, info);
}
rocksdb::Status StateMachine::lease_get(const std::string &key, ClockValue clockUpdate, LeaseInfo &info, LogIndex index) {
......
......@@ -40,6 +40,13 @@
namespace quarkdb {
enum class LeaseAcquisitionStatus {
kKeyTypeMismatch,
kAcquired,
kRenewed,
kFailedDueToOtherOwner
};
class StagingArea;
class StateMachine {
......@@ -81,7 +88,7 @@ public:
rocksdb::Status rpop(StagingArea &stagingArea, const std::string &key, std::string &item);
void advanceClock(StagingArea &stagingArea, ClockValue newValue);
rocksdb::Status lease_acquire(StagingArea &stagingArea, const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info, bool &acquired);
LeaseAcquisitionStatus lease_acquire(StagingArea &stagingArea, const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info);
rocksdb::Status lease_release(StagingArea &stagingArea, const std::string &key);
rocksdb::Status lease_get(StagingArea &stagingArea, const std::string &key, ClockValue clockUpdate, LeaseInfo &info);
......@@ -149,7 +156,7 @@ public:
void advanceClock(ClockValue newValue, LogIndex index = 0);
void getClock(ClockValue &value);
rocksdb::Status rawGetAllVersions(const std::string &key, std::vector<rocksdb::KeyVersion> &versions);
rocksdb::Status lease_acquire(const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info, bool &acquired, LogIndex index = 0);
LeaseAcquisitionStatus lease_acquire(const std::string &key, const std::string &value, ClockValue clockUpdate, uint64_t duration, LeaseInfo &info, LogIndex index = 0);
rocksdb::Status lease_release(const std::string &key, LogIndex index = 0);
rocksdb::Status lease_get(const std::string &key, ClockValue clockUpdate, LeaseInfo &info, LogIndex index = 0);
......
......@@ -707,10 +707,10 @@ TEST_F(State_Machine, Leases) {
}
bool acquired;
LeaseInfo info;
ASSERT_OK(stateMachine()->lease_acquire("my-lease", "some-string", ClockValue(1), 10, info, acquired));
ASSERT_TRUE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease", "some-string", ClockValue(1), 10, info),
LeaseAcquisitionStatus::kAcquired);
ASSERT_EQ(info.getDeadline(), 11u);
ASSERT_EQ(info.getLastRenewal(), 1u);
ASSERT_EQ(info.getValue(), "some-string");
......@@ -728,8 +728,10 @@ TEST_F(State_Machine, Leases) {
ASSERT_FALSE(iterator.valid());
}
ASSERT_OK(stateMachine()->lease_acquire("my-lease", "some-string", ClockValue(9), 10, info, acquired));
ASSERT_TRUE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease", "some-string", ClockValue(9), 10, info),
LeaseAcquisitionStatus::kRenewed
);
ASSERT_EQ(info.getDeadline(), 19u);
ASSERT_EQ(info.getLastRenewal(), 9u);
ASSERT_EQ(info.getValue(), "some-string");
......@@ -747,8 +749,9 @@ TEST_F(State_Machine, Leases) {
ASSERT_FALSE(iterator.valid());
}
stateMachine()->lease_acquire("my-lease", "some-other-string", ClockValue(12), 10, info, acquired);
ASSERT_FALSE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease", "some-other-string", ClockValue(12), 10, info),
LeaseAcquisitionStatus::kFailedDueToOtherOwner);
ASSERT_EQ(info.getDeadline(), 19u);
ASSERT_EQ(info.getLastRenewal(), 9u);
ASSERT_EQ(info.getValue(), "some-string");
......@@ -756,8 +759,9 @@ TEST_F(State_Machine, Leases) {
stateMachine()->getClock(clk);
ASSERT_EQ(clk, 12u);
stateMachine()->lease_acquire("my-lease-2", "some-other-string", ClockValue(13), 10, info, acquired);
ASSERT_TRUE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease-2", "some-other-string", ClockValue(13), 10, info),
LeaseAcquisitionStatus::kAcquired);
ASSERT_EQ(info.getDeadline(), 23u);
ASSERT_EQ(info.getLastRenewal(), 13u);
ASSERT_EQ(info.getValue(), "some-other-string");
......@@ -805,14 +809,16 @@ TEST_F(State_Machine, Leases) {
ASSERT_FALSE(iterator.valid());
}
stateMachine()->lease_acquire("my-lease-3", "some-other-string", ClockValue(18), 10, info, acquired);
ASSERT_TRUE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease-3", "some-other-string", ClockValue(18), 10, info),
LeaseAcquisitionStatus::kAcquired);
ASSERT_EQ(info.getDeadline(), 28u);
ASSERT_EQ(info.getLastRenewal(), 18u);
ASSERT_EQ(info.getValue(), "some-other-string");
stateMachine()->lease_acquire("my-lease-4", "some-other-string", ClockValue(18), 10, info, acquired);
ASSERT_TRUE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease-4", "some-other-string", ClockValue(18), 10, info),
LeaseAcquisitionStatus::kAcquired);
ASSERT_EQ(info.getDeadline(), 28u);
ASSERT_EQ(info.getLastRenewal(), 18u);
ASSERT_EQ(info.getValue(), "some-other-string");
......@@ -838,8 +844,8 @@ TEST_F(State_Machine, Leases) {
ASSERT_FALSE(iterator.valid());
}
stateMachine()->lease_acquire("my-lease-4", "some-other-string", ClockValue(25), 10, info, acquired);
ASSERT_TRUE(acquired);
ASSERT_EQ(stateMachine()->lease_acquire("my-lease-4", "some-other-string", ClockValue(25), 10, info),
LeaseAcquisitionStatus::kRenewed);
ASSERT_EQ(info.getDeadline(), 35u);
ASSERT_EQ(info.getLastRenewal(), 25u);
ASSERT_EQ(info.getValue(), "some-other-string");
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment