Permalink
Browse files

Merge pull request #10 from garru/ReadRepair

Need to pull in entire timeline from redis in order to do read repair.
  • Loading branch information...
2 parents 8ba63ed + c8cba34 commit 26ca115be4232d5332265628494de8474a57e95c @garru garru committed Nov 8, 2011
@@ -189,20 +189,15 @@ class RedisShard(val shardInfo: ShardInfo, val weight: Int, val children: Seq[Ha
def get(timeline: String, offset: Int, length: Int, dedupeSecondary: Boolean): Option[TimelineSegment] = {
Stats.timeMicros("redisshard-get-usec") {
readPool.withClient(shardInfo) { client =>
- // heuristics for detecting a request for the entire timeline
- if (!(offset == 0 && (length == 800 || length == 3200))) {
- val size = client.size(timeline)
- if (size > 0) {
- // empty and miss look the same to redis, fix that
- val entries = getAndFilterSentinel(client, timeline, offset, length).get.toList
- Some(TimelineSegment(dedupe(entries, dedupeSecondary), size-1))
- } else {
- None
- }
+ // we've changed the size semantics to always return the size of theresult set.
+ val entries = client.get(timeline, 0, -1)
+
+ if (entries.isEmpty) {
+ None
} else {
- getAndFilterSentinel(client, timeline, offset, length) map { entries =>
- TimelineSegment(dedupe(entries, dedupeSecondary), entries.size)
- }
+ val filtered = entries filter isSentinel
+ val filteredEntries = dedupe(filtered, dedupeSecondary).slice(offset, length)
+ Some(TimelineSegment(filteredEntries, filteredEntries.size))
}
}
}
@@ -149,12 +149,10 @@ object IntegrationSpec extends ConfiguredSpecification with JMocker with ClassMo
service.nameServer.reload()
expect {
- one(jredisClient).llen(timeline1) willReturn future
- one(future).get(200L, TimeUnit.MILLISECONDS) willReturn 0L
+ one(jredisClient).lrange(timeline1, 0, -1) willReturn timelineFuture
+ one(timelineFuture).get(200L, TimeUnit.MILLISECONDS) willReturn Seq[Array[Byte]]().toJavaList
- one(jredisClient).llen(timeline1) willReturn future
- one(future).get(200L, TimeUnit.MILLISECONDS) willReturn 2L
- one(jredisClient).lrange(timeline1, -3, -1) willReturn timelineFuture
+ one(jredisClient).lrange(timeline1, 0, -1) willReturn timelineFuture
one(timelineFuture).get(200L, TimeUnit.MILLISECONDS) willReturn List("a", "b").map { _.getBytes }.toJavaList
one(jredisClient).expire(timeline1, 86400)
@@ -170,16 +168,14 @@ object IntegrationSpec extends ConfiguredSpecification with JMocker with ClassMo
}
val segment = service.haploService.get(timeline1, 0, 2, false)
- segment.size mustEqual 1
+ segment.size mustEqual 2
segment.entries.get(0).array.toList mustEqual "a".getBytes.toList
segment.entries.get(1).array.toList mustEqual "b".getBytes.toList
}
"get_multi hit" in {
expect {
- one(jredisClient).llen(timeline1) willReturn future
- one(future).get(200L, TimeUnit.MILLISECONDS) willReturn 2L
- one(jredisClient).lrange(timeline1, -3, -1) willReturn timelineFuture
+ one(jredisClient).lrange(timeline1, 0, -1) willReturn timelineFuture
one(timelineFuture).get(200L, TimeUnit.MILLISECONDS) willReturn List("a", "b").map { _.getBytes }.toJavaList
one(jredisClient).expire(timeline1, 86400)
}
@@ -190,17 +186,17 @@ object IntegrationSpec extends ConfiguredSpecification with JMocker with ClassMo
results.size mustEqual 1
val segment = results.toSeq(0)
segment.state mustEqual thrift.TimelineSegmentState.HIT
- segment.size mustEqual 1
+ segment.size mustEqual 2
segment.entries.get(0).array.toList mustEqual "a".getBytes.toList
segment.entries.get(1).array.toList mustEqual "b".getBytes.toList
}
"get_multi miss" in {
expect {
- one(jredisClient).llen(timeline1) willReturn future
- one(future).get(200L, TimeUnit.MILLISECONDS) willReturn 0L
- one(jredisClient).llen(timeline1) willReturn future
- one(future).get(200L, TimeUnit.MILLISECONDS) willReturn 0L
+ one(jredisClient).lrange(timeline1, 0, -1) willReturn future
+ one(future).get(200L, TimeUnit.MILLISECONDS) willReturn Seq[Array[Byte]]().toJavaList
+ one(jredisClient).lrange(timeline1, 0, -1) willReturn future
+ one(future).get(200L, TimeUnit.MILLISECONDS) willReturn Seq[Array[Byte]]().toJavaList
}
val cmd = new thrift.TimelineGet(timeline1, 0, 2)
@@ -214,8 +210,8 @@ object IntegrationSpec extends ConfiguredSpecification with JMocker with ClassMo
"get_multi exception" in {
expect {
- one(jredisClient).llen(timeline1) willReturn future
- one(future).get(200L, TimeUnit.MILLISECONDS) willReturn 0L
+ one(jredisClient).lrange(timeline1, 0, -1) willReturn future
+ one(future).get(200L, TimeUnit.MILLISECONDS) willReturn Seq[Array[Byte]]().toJavaList
}
val cmd = new thrift.TimelineGet(timeline1, 0, 2)
@@ -52,6 +52,12 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
one(future).get(1000, TimeUnit.MILLISECONDS) willReturn (Seq(TimelineEntry.EmptySentinel) ++ result).toJavaList
}
+ def lrangeWithoutEmptySentinel(timeline: String, start: Int, end: Int, result: Seq[Array[Byte]]) {
+ one(jredis).lrange(timeline, start, end) willReturn future
+ one(future).get(1000, TimeUnit.MILLISECONDS) willReturn result.toJavaList
+
+ }
+
def llen(timeline: String, result: Long) {
one(jredis).llen(timeline) willReturn longFuture
one(longFuture).get(1000, TimeUnit.MILLISECONDS) willReturn result
@@ -173,8 +179,7 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"unique entries" in {
expect {
one(shardInfo).hostname willReturn "host1"
- lrange(timeline, -11, -1, List(entry23, entry20, entry19).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23, entry20, entry19).reverse)
one(jredis).expire(timeline, 1)
}
@@ -186,20 +191,18 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"in range" in {
expect {
one(shardInfo).hostname willReturn "host1"
- lrange(timeline, -21, -11, List(entry23, entry20, entry19).reverse)
- llen(timeline, 23L)
+ lrange(timeline, 0, -1, List(entry23, entry20, entry19).reverse)
one(jredis).expire(timeline, 1)
}
- redisShard.get(timeline, 10, 10, false).get.entries.toList mustEqual List(entry23, entry20, entry19)
+ redisShard.get(timeline, 1, 10, false).get.entries.toList mustEqual List(entry20, entry19)
reads mustEqual 1
}
"out of range" in {
expect {
one(shardInfo).hostname willReturn "host1"
- lrange(timeline, -21, -11, List())
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23))
one(jredis).expire(timeline, 1)
}
@@ -210,19 +213,18 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"miss" in {
expect {
one(shardInfo).hostname willReturn "host1"
- llen(timeline, 0L)
+ lrangeWithoutEmptySentinel(timeline, 0, -1, List())
}
- redisShard.get(timeline, 10, 10, false) mustEqual None
+ redisShard.get(timeline, 0, 10, false) mustEqual None
reads mustEqual 1
}
}
"with duplicates in the sort key" in {
expect {
one(shardInfo).hostname willReturn "host1"
- lrange(timeline, -11, -1, List(entry23, entry23a, entry19).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23, entry23a, entry19).reverse)
one(jredis).expire(timeline, 1)
}
@@ -234,11 +236,9 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"to be deduped" in {
expect {
allowing(shardInfo).hostname willReturn "host1"
- lrange(timeline, -11, -1, List(entry23a, entry20, entry19a).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23a, entry20, entry19a).reverse)
one(jredis).expire(timeline, 1)
- lrange(timeline, -11, -1, List(entry23a, entry20, entry19a).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23a, entry20, entry19a).reverse)
one(jredis).expire(timeline, 1)
}
@@ -250,8 +250,7 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"not marked as having a secondary key" in {
expect {
allowing(shardInfo).hostname willReturn "host1"
- lrange(timeline, -11, -1, List(entry23a, entry20, entry19uniq).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23a, entry20, entry19uniq).reverse)
one(jredis).expire(timeline, 1)
}
@@ -263,8 +262,7 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"with duplicates between the secondary and primary keys" in {
expect {
one(shardInfo).hostname willReturn "host1"
- lrange(timeline, -11, -1, List(entry23share, entry23, entry20).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry23share, entry23, entry20).reverse)
one(jredis).expire(timeline, 1)
}
@@ -279,8 +277,7 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
expect {
one(shardInfo).hostname willReturn "host1"
- lrange(timeline, -11, -1, List(entry1, entry2, entry3).reverse)
- llen(timeline, 3L)
+ lrange(timeline, 0, -1, List(entry1, entry2, entry3).reverse)
one(jredis).expire(timeline, 1)
}
@@ -593,7 +590,7 @@ object RedisShardSpec extends ConfiguredSpecification with JMocker with ClassMoc
"exceptions are wrapped" in {
expect {
one(shardInfo).hostname willReturn "host1"
- one(jredis).llen(timeline) willThrow new IllegalStateException("aiee")
+ one(jredis).lrange(timeline, 0, -1) willThrow new IllegalStateException("aiee")
}
redisShard.get(timeline, 0, 10, true) must throwA[ShardException]

0 comments on commit 26ca115

Please sign in to comment.