Skip to content

Linearizable reads/writes #1344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
srh opened this issue Aug 20, 2017 · 7 comments
Closed

Linearizable reads/writes #1344

srh opened this issue Aug 20, 2017 · 7 comments
Assignees

Comments

@srh
Copy link

srh commented Aug 20, 2017

(We didn't have an issue for this.) The feature we want is for read behavior to be linearizable, instead of happily reading outdated data.

The short of it is, we're going to use etcd/raft's ReadIndex functionality. To avoid spamming Raft groups' leaders with MsgReadIndex messages, each follower node will only make one in-flight request at a time. We'll do this from whatever node serves our Raft group.

That's the most basic version -- we could get more clever about how we do ReadIndex in the future to reduce latency. We could allow multiple in-flight requests (and take care not to let them bunch up).
We could send our requests from other machines to avoid extra network hops. Some descriptions of alternatives are here: https://discuss.dgraph.io/t/linearizability/1683/3

We will also need to change the watermark implementation to not use sleep.

@srh srh self-assigned this Aug 28, 2017
@srh
Copy link
Author

srh commented Aug 28, 2017

So, linearizability is implemented (albeit under-tested, and the simplest possible implementation as described). In the branch sam/new1344.

One thing that isn't implemented, though is the question, what should the UI be? How should the user specify a linearizable read vs. a non-linearizable one?

Option 1: an out-of-band flag.
Option 2: the flag gets parsed out of the query/schema read query somehow.
Option 3: it's not a mere flag -- the user could supply a {groupId => readIndex} bundle.

@manishrjain
Copy link
Contributor

All reads should be linearizable for now. If performance becomes an issue, we'll solve for it when it does.

@srh
Copy link
Author

srh commented Aug 29, 2017

I'm getting 14 second reads with a simple curl query while running dgraphloader. Running the Raft ReadIndex operation is quick, but then it takes about 12 seconds between getting a linearizable read index and for waitForAppliedMark to return. It looks like the time duration between a change reaching a majority of nodes and actually being "applied" can be large.

@manishrjain
Copy link
Contributor

manishrjain commented Aug 29, 2017

It looks like the time duration between a change reaching a majority of nodes and actually being "applied" can be large.

This is because we're using goroutines for each mutation which get scheduled whenever they do. @janardhan1993 is working on a smart scheduler which can prioritize mutations in a roughly FIFO order. That should reduce the applied latency.

But, great to hear read linearizability is working! PR?

@janardhan1993
Copy link
Contributor

To minimise the latency, i guess we need to schedule all the mtuations which came after read request only when the applied watermark catches up.

@srh
Copy link
Author

srh commented Aug 30, 2017

Linearizability won't be fully working until #1345 (for which a basic fix has just been implemented) because it does in fact trigger that scenario sometimes.

@srh
Copy link
Author

srh commented Sep 15, 2017

#1345 was fixed, so this can be closed.

@srh srh closed this as completed Sep 15, 2017
@srh srh removed the in progress label Sep 15, 2017
jarifibrahim pushed a commit that referenced this issue Jun 16, 2020
This commit brings the following changes from badger

c45d966 Fix assert in background compression and encryption. (#1366)
14386ac GC: Consider size of value while rewriting (#1357)
b2267c2 Restore: Account for value size as well (#1358)
b762832 Tests: Do not leave behind state goroutines (#1349)
056d859 Support disabling conflict detection (#1344)
fd89894 Compaction: Expired keys and delete markers are never purged (#1354)
543f353 Fix build on golang tip (#1355)
a7e239e StreamWriter: Close head writer (#1347)
da80eb9 Iterator: Always add key to txn.reads (#1328)
7e19cac Add immudb to the project list (#1341)
079f5ae DefaultOptions: Set KeepL0InMemory to false (#1345)
jarifibrahim pushed a commit that referenced this issue Jun 17, 2020
This commit brings the following changes from badger
```
c45d966 Fix assert in background compression and encryption. (#1366)
14386ac GC: Consider size of value while rewriting (#1357)
b2267c2 Restore: Account for value size as well (#1358)
b762832 Tests: Do not leave behind state goroutines (#1349)
056d859 Support disabling conflict detection (#1344)
fd89894 Compaction: Expired keys and delete markers are never purged (#1354)
543f353 Fix build on golang tip (#1355)
a7e239e StreamWriter: Close head writer (#1347)
da80eb9 Iterator: Always add key to txn.reads (#1328)
7e19cac Add immudb to the project list (#1341)
079f5ae DefaultOptions: Set KeepL0InMemory to false (#1345)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants