From 60893b621321a6e6c456c3bd82d8d23b3b2ba38b Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Fri, 3 Feb 2023 12:38:44 -0600 Subject: [PATCH 01/15] parsing add comparison with stdlib --- Cargo.lock | 5 ++ src/storage/types/Cargo.toml | 11 ++- src/storage/types/benches/benchmark.rs | 47 +++++++++++ src/storage/types/src/lib.rs | 3 + src/storage/types/src/parse.rs | 111 +++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 src/storage/types/benches/benchmark.rs create mode 100644 src/storage/types/src/parse.rs diff --git a/Cargo.lock b/Cargo.lock index b36a6d55..10605d6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2480,6 +2480,11 @@ checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" [[package]] name = "storage-types" version = "0.3.1" +dependencies = [ + "criterion 0.3.6", + "rand", + "rand_xoshiro", +] [[package]] name = "strsim" diff --git a/src/storage/types/Cargo.toml b/src/storage/types/Cargo.toml index 5f006c7e..3378e7f7 100644 --- a/src/storage/types/Cargo.toml +++ b/src/storage/types/Cargo.toml @@ -7,5 +7,14 @@ edition = { workspace = true } homepage = { workspace = true } repository = { workspace = true } license = { workspace = true } - +[[bench]] +name = "parsing" +path = "benches/benchmark.rs" +harness = false [dependencies] + + +[dev-dependencies] +criterion = "0.3.4" +rand_xoshiro = { workspace = true } +rand = { workspace = true , features = ["small_rng", "getrandom"] } diff --git a/src/storage/types/benches/benchmark.rs b/src/storage/types/benches/benchmark.rs new file mode 100644 index 00000000..55c03a01 --- /dev/null +++ b/src/storage/types/benches/benchmark.rs @@ -0,0 +1,47 @@ +// Copyright 2021 Twitter, Inc. +// Licensed under the Apache License, Version 2.0 +// http://www.apache.org/licenses/LICENSE-2.0 + +use std::num::ParseIntError; +use criterion::{criterion_group, criterion_main, Criterion, Throughput}; +use rand::RngCore; +use rand::SeedableRng; +use storage_types::*; + +use std::time::Duration; + +// A very fast PRNG which is appropriate for testing +pub fn rng() -> impl RngCore { + rand_xoshiro::Xoshiro256PlusPlus::seed_from_u64(0) +} + +fn parse_redis_benchmark(c: &mut Criterion) { + let mut group = c.benchmark_group("parse_signed_redis"); + group.measurement_time(Duration::from_secs(30)); + + + let bytes = b"9223372036854775807"; + group.throughput(Throughput::Elements(1)); + group.bench_with_input("parse_numbers", &bytes, |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + if (result.is_none()) { + println!("it's empty!") + } + }) + }); + + let string = "9223372036854775807"; + group.bench_with_input("parse_numbers_std_lib", &string, |b, &bytes| { + b.iter(|| { + let result: Result = bytes.parse(); + if (result.is_err()) { + println!("it's empty!") + } + }) + }); +} + + +criterion_group!(benches, parse_redis_benchmark); +criterion_main!(benches); diff --git a/src/storage/types/src/lib.rs b/src/storage/types/src/lib.rs index 87aaeea0..7691240a 100644 --- a/src/storage/types/src/lib.rs +++ b/src/storage/types/src/lib.rs @@ -2,6 +2,9 @@ // Licensed under the Apache License, Version 2.0 // http://www.apache.org/licenses/LICENSE-2.0 +mod parse; +pub use parse::*; + #[derive(PartialEq, Eq)] pub enum Value<'a> { Bytes(&'a [u8]), diff --git a/src/storage/types/src/parse.rs b/src/storage/types/src/parse.rs new file mode 100644 index 00000000..059036f2 --- /dev/null +++ b/src/storage/types/src/parse.rs @@ -0,0 +1,111 @@ +// Copyright 2023 Pelikan Foundation LLC. +// Licensed under the Apache License, Version 2.0 +// http://www.apache.org/licenses/LICENSE-2.0 + +//! Common, performance-oriented mechanisms of parsing byte strings into various types + + +/// maximum length of a string that could be stored as a u64 in Redis +/// comment from redis/util.h +/// > Bytes needed for long -> str + '\0' +const REDIS_LONG_STR_SIZE: usize = 22; + +/// optionally parse a bytestring into a signed integer +/// implementation inspired by Redis +pub fn parse_signed_redis(bytes: &[u8]) -> Option { + if bytes.is_empty() || bytes.len() >= REDIS_LONG_STR_SIZE { + return None + } + + // Special case: first and only digit is 0. + if bytes.len() == 1 && bytes[0] == b'0' { + return Some(0) + } + ///parses the remainder of the byte string as a number, returning None if at any point + /// it is determined it isn't a canonical integer + #[inline] + fn parse_rest(start: i64, rest: &[u8], sign: i64) -> Option { + let mut number: i64 = start; + for byte in rest { + let multiplied = number.checked_mul(10)?; + let digit = convert_digit(*byte)?; + let signed_digit = (digit as i64) * sign; + let added = multiplied.checked_add(signed_digit)?; + number = added; + } + Some(number) + } + #[inline] + fn convert_digit(u: u8) -> Option { + (u as char).to_digit(10) + } + + match &bytes[0] { + b'-' if bytes.len() >= 2 && bytes[1] != b'0' => { + let first_digit = convert_digit(bytes[1])?; + parse_rest((first_digit as i64) * -1, &bytes[2..], -1) + } + other if (b'1'..=b'9').contains(other) => { + let digit = convert_digit(*other)?; + parse_rest( digit as i64, &bytes[1..], 1) + } + _ => { + None + } + } +} + +#[cfg(test)] +mod tests { + use crate::parse::parse_signed_redis; + use rand::prelude::*; + #[test] + fn it_should_parse_numbers() { + assert_eq!(parse_signed_redis(b"199"), Some(199)); + assert_eq!(parse_signed_redis(b"0"), Some(0)); + assert_eq!(parse_signed_redis(b"-1"), Some(-1)); + assert_eq!(parse_signed_redis(b"9223372036854775807"), Some(i64::MAX)); + assert_eq!(parse_signed_redis(b"-9223372036854775808"), Some(i64::MIN)); + for x in 0..100_000 { + let number = rand::random::(); + let string = number.to_string(); + assert_eq!(parse_signed_redis(string.as_bytes()), Some(number)); + } + } + + #[test] + fn it_should_not_parse_non_canonical_signed_ints() { + //leading zeroes + assert_eq!(parse_signed_redis(b"042"), None); + assert_eq!(parse_signed_redis(b"007"), None); + assert_eq!(parse_signed_redis(b"000"), None); + + //negative numbers with leading zeroes + assert_eq!(parse_signed_redis(b"-042"), None); + assert_eq!(parse_signed_redis(b"-007"), None); + assert_eq!(parse_signed_redis(b"-0007"), None); + + //negative zero + assert_eq!(parse_signed_redis(b"-0"), None); + assert_eq!(parse_signed_redis(b"-00"), None); + assert_eq!(parse_signed_redis(b"-0000"), None); + + //won't parse overflowed values + assert_eq!(parse_signed_redis(b"9223372036854775808"), None); + assert_eq!(parse_signed_redis(b"-9223372036854775809"), None); + + //text strings + assert_eq!(parse_signed_redis(b"foobar"), None); + assert_eq!(parse_signed_redis(b"42foobar"), None); + assert_eq!(parse_signed_redis(b"foobar42"), None); + assert_eq!(parse_signed_redis(b"0f"), None); + assert_eq!(parse_signed_redis(b"8f"), None); + + //symbols + assert_eq!(parse_signed_redis(b"0&"), None); + assert_eq!(parse_signed_redis(b"8&"), None); + assert_eq!(parse_signed_redis(b"&$@!@#@0"), None); + assert_eq!(parse_signed_redis(b"42-42"), None); + + } +} \ No newline at end of file From 2d3801489f9aadff97a4c44fb8b8f42027ad7462 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Fri, 3 Feb 2023 13:54:00 -0600 Subject: [PATCH 02/15] better benchmarking --- src/storage/types/benches/benchmark.rs | 70 ++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/src/storage/types/benches/benchmark.rs b/src/storage/types/benches/benchmark.rs index 55c03a01..704da799 100644 --- a/src/storage/types/benches/benchmark.rs +++ b/src/storage/types/benches/benchmark.rs @@ -17,12 +17,12 @@ pub fn rng() -> impl RngCore { fn parse_redis_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("parse_signed_redis"); - group.measurement_time(Duration::from_secs(30)); + group.measurement_time(Duration::from_secs(2)); let bytes = b"9223372036854775807"; group.throughput(Throughput::Elements(1)); - group.bench_with_input("parse_numbers", &bytes, |b, &bytes| { + group.bench_with_input("parse_max_value", &bytes, |b, &bytes| { b.iter(|| { let result = parse_signed_redis(bytes); if (result.is_none()) { @@ -32,7 +32,71 @@ fn parse_redis_benchmark(c: &mut Criterion) { }); let string = "9223372036854775807"; - group.bench_with_input("parse_numbers_std_lib", &string, |b, &bytes| { + group.bench_with_input("parse_max_value_stdlib", &string, |b, &bytes| { + b.iter(|| { + let result: Result = bytes.parse(); + if (result.is_err()) { + println!("it's empty!") + } + }) + }); + + let bytes = b"-9223372036854775807"; + group.throughput(Throughput::Elements(1)); + group.bench_with_input("parse_min_value", &bytes, |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + if (result.is_none()) { + println!("it's empty!") + } + }) + }); + + let string = "-9223372036854775807"; + group.bench_with_input("parse_min_value_std_lib", &string, |b, &bytes| { + b.iter(|| { + let result: Result = bytes.parse(); + if (result.is_err()) { + println!("it's empty!") + } + }) + }); + + let bytes = b"123456"; + group.throughput(Throughput::Elements(1)); + group.bench_with_input("parse_reg_value", &bytes, |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + if (result.is_none()) { + println!("it's empty!") + } + }) + }); + + let string = "123456"; + group.bench_with_input("parse_reg_value_std_lib", &string, |b, &bytes| { + b.iter(|| { + let result: Result = bytes.parse(); + if (result.is_err()) { + println!("it's empty!") + } + }) + }); + + + let bytes = b"0"; + group.throughput(Throughput::Elements(1)); + group.bench_with_input("parse_zero", &bytes, |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + if (result.is_none()) { + println!("it's empty!") + } + }) + }); + + let string = "0"; + group.bench_with_input("parse_zero_std_lib", &string, |b, &bytes| { b.iter(|| { let result: Result = bytes.parse(); if (result.is_err()) { From 6194f1f8377daadc95a37727a9bcbe56b034d1d7 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Fri, 3 Feb 2023 14:09:09 -0600 Subject: [PATCH 03/15] add overflow case to benchmarks --- src/storage/types/benches/benchmark.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/storage/types/benches/benchmark.rs b/src/storage/types/benches/benchmark.rs index 704da799..189acb3e 100644 --- a/src/storage/types/benches/benchmark.rs +++ b/src/storage/types/benches/benchmark.rs @@ -41,6 +41,27 @@ fn parse_redis_benchmark(c: &mut Criterion) { }) }); + let bytes = b"92233720368547758079223372036854775807"; + group.throughput(Throughput::Elements(1)); + group.bench_with_input("parse_overflowed_value", &bytes, |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + if (result.is_some()) { + println!("failed parse") + } + }) + }); + + let string = "92233720368547758079223372036854775807"; + group.bench_with_input("parse_overflowed_value_std_lib", &string, |b, &bytes| { + b.iter(|| { + let result: Result = bytes.parse(); + if (result.is_ok()) { + println!("failed parse") + } + }) + }); + let bytes = b"-9223372036854775807"; group.throughput(Throughput::Elements(1)); group.bench_with_input("parse_min_value", &bytes, |b, &bytes| { From 38436cf5a03b105eb4bea15444ecc17992b39225 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Mon, 6 Feb 2023 09:02:01 -0600 Subject: [PATCH 04/15] test changes --- src/storage/types/src/parse.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/storage/types/src/parse.rs b/src/storage/types/src/parse.rs index 059036f2..9c921e76 100644 --- a/src/storage/types/src/parse.rs +++ b/src/storage/types/src/parse.rs @@ -58,19 +58,14 @@ pub fn parse_signed_redis(bytes: &[u8]) -> Option { #[cfg(test)] mod tests { use crate::parse::parse_signed_redis; - use rand::prelude::*; #[test] - fn it_should_parse_numbers() { - assert_eq!(parse_signed_redis(b"199"), Some(199)); - assert_eq!(parse_signed_redis(b"0"), Some(0)); - assert_eq!(parse_signed_redis(b"-1"), Some(-1)); + fn it_should_parse_obvious_numbers() { + for x in 0..=10_000 { + assert_eq!(parse_signed_redis(x.to_string().as_bytes()), Some(x as i64)); + assert_eq!(parse_signed_redis((-x).to_string().as_bytes()), Some(-x as i64)); + } assert_eq!(parse_signed_redis(b"9223372036854775807"), Some(i64::MAX)); assert_eq!(parse_signed_redis(b"-9223372036854775808"), Some(i64::MIN)); - for x in 0..100_000 { - let number = rand::random::(); - let string = number.to_string(); - assert_eq!(parse_signed_redis(string.as_bytes()), Some(number)); - } } #[test] From c002a6042ab96212dcfacdef873b5e3724d8ffab Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Tue, 7 Feb 2023 12:44:43 -0600 Subject: [PATCH 05/15] cargo fmt --- src/storage/types/benches/benchmark.rs | 5 +---- src/storage/types/src/parse.rs | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/storage/types/benches/benchmark.rs b/src/storage/types/benches/benchmark.rs index 189acb3e..674c10f2 100644 --- a/src/storage/types/benches/benchmark.rs +++ b/src/storage/types/benches/benchmark.rs @@ -2,10 +2,10 @@ // Licensed under the Apache License, Version 2.0 // http://www.apache.org/licenses/LICENSE-2.0 -use std::num::ParseIntError; use criterion::{criterion_group, criterion_main, Criterion, Throughput}; use rand::RngCore; use rand::SeedableRng; +use std::num::ParseIntError; use storage_types::*; use std::time::Duration; @@ -19,7 +19,6 @@ fn parse_redis_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("parse_signed_redis"); group.measurement_time(Duration::from_secs(2)); - let bytes = b"9223372036854775807"; group.throughput(Throughput::Elements(1)); group.bench_with_input("parse_max_value", &bytes, |b, &bytes| { @@ -104,7 +103,6 @@ fn parse_redis_benchmark(c: &mut Criterion) { }) }); - let bytes = b"0"; group.throughput(Throughput::Elements(1)); group.bench_with_input("parse_zero", &bytes, |b, &bytes| { @@ -127,6 +125,5 @@ fn parse_redis_benchmark(c: &mut Criterion) { }); } - criterion_group!(benches, parse_redis_benchmark); criterion_main!(benches); diff --git a/src/storage/types/src/parse.rs b/src/storage/types/src/parse.rs index 9c921e76..e9ebaf62 100644 --- a/src/storage/types/src/parse.rs +++ b/src/storage/types/src/parse.rs @@ -4,7 +4,6 @@ //! Common, performance-oriented mechanisms of parsing byte strings into various types - /// maximum length of a string that could be stored as a u64 in Redis /// comment from redis/util.h /// > Bytes needed for long -> str + '\0' @@ -14,12 +13,12 @@ const REDIS_LONG_STR_SIZE: usize = 22; /// implementation inspired by Redis pub fn parse_signed_redis(bytes: &[u8]) -> Option { if bytes.is_empty() || bytes.len() >= REDIS_LONG_STR_SIZE { - return None + return None; } // Special case: first and only digit is 0. - if bytes.len() == 1 && bytes[0] == b'0' { - return Some(0) + if bytes.len() == 1 && bytes[0] == b'0' { + return Some(0); } ///parses the remainder of the byte string as a number, returning None if at any point /// it is determined it isn't a canonical integer @@ -45,13 +44,11 @@ pub fn parse_signed_redis(bytes: &[u8]) -> Option { let first_digit = convert_digit(bytes[1])?; parse_rest((first_digit as i64) * -1, &bytes[2..], -1) } - other if (b'1'..=b'9').contains(other) => { + other if (b'1'..=b'9').contains(other) => { let digit = convert_digit(*other)?; - parse_rest( digit as i64, &bytes[1..], 1) - } - _ => { - None + parse_rest(digit as i64, &bytes[1..], 1) } + _ => None, } } @@ -62,7 +59,10 @@ mod tests { fn it_should_parse_obvious_numbers() { for x in 0..=10_000 { assert_eq!(parse_signed_redis(x.to_string().as_bytes()), Some(x as i64)); - assert_eq!(parse_signed_redis((-x).to_string().as_bytes()), Some(-x as i64)); + assert_eq!( + parse_signed_redis((-x).to_string().as_bytes()), + Some(-x as i64) + ); } assert_eq!(parse_signed_redis(b"9223372036854775807"), Some(i64::MAX)); assert_eq!(parse_signed_redis(b"-9223372036854775808"), Some(i64::MIN)); @@ -101,6 +101,5 @@ mod tests { assert_eq!(parse_signed_redis(b"8&"), None); assert_eq!(parse_signed_redis(b"&$@!@#@0"), None); assert_eq!(parse_signed_redis(b"42-42"), None); - } -} \ No newline at end of file +} From c53dc8e1a51da776e9f8582a730585063aac94d6 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Tue, 7 Feb 2023 13:01:53 -0600 Subject: [PATCH 06/15] clean up benchmarks --- Cargo.lock | 2 - src/storage/types/Cargo.toml | 3 +- src/storage/types/benches/benchmark.rs | 144 ++++++------------------- 3 files changed, 36 insertions(+), 113 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 04e2f0ae..6ae2d2cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2500,8 +2500,6 @@ name = "storage-types" version = "0.3.1" dependencies = [ "criterion 0.3.6", - "rand", - "rand_xoshiro", ] [[package]] diff --git a/src/storage/types/Cargo.toml b/src/storage/types/Cargo.toml index 3378e7f7..5c00f9bc 100644 --- a/src/storage/types/Cargo.toml +++ b/src/storage/types/Cargo.toml @@ -16,5 +16,4 @@ harness = false [dev-dependencies] criterion = "0.3.4" -rand_xoshiro = { workspace = true } -rand = { workspace = true , features = ["small_rng", "getrandom"] } + diff --git a/src/storage/types/benches/benchmark.rs b/src/storage/types/benches/benchmark.rs index 674c10f2..ec989db8 100644 --- a/src/storage/types/benches/benchmark.rs +++ b/src/storage/types/benches/benchmark.rs @@ -1,128 +1,54 @@ -// Copyright 2021 Twitter, Inc. +// Copyright 2023 Pelikan Foundation LLC. // Licensed under the Apache License, Version 2.0 // http://www.apache.org/licenses/LICENSE-2.0 use criterion::{criterion_group, criterion_main, Criterion, Throughput}; -use rand::RngCore; -use rand::SeedableRng; use std::num::ParseIntError; use storage_types::*; use std::time::Duration; -// A very fast PRNG which is appropriate for testing -pub fn rng() -> impl RngCore { - rand_xoshiro::Xoshiro256PlusPlus::seed_from_u64(0) -} - fn parse_redis_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("parse_signed_redis"); group.measurement_time(Duration::from_secs(2)); - let bytes = b"9223372036854775807"; - group.throughput(Throughput::Elements(1)); - group.bench_with_input("parse_max_value", &bytes, |b, &bytes| { - b.iter(|| { - let result = parse_signed_redis(bytes); - if (result.is_none()) { - println!("it's empty!") - } - }) - }); - - let string = "9223372036854775807"; - group.bench_with_input("parse_max_value_stdlib", &string, |b, &bytes| { - b.iter(|| { - let result: Result = bytes.parse(); - if (result.is_err()) { - println!("it's empty!") - } - }) - }); + let examples = vec![ + ("max_value", "9223372036854775807"), + ("min_value", "-9223372036854775807"), + ("average_value 6 bytes", "123456"), + ("average_value 7 bytes", "1234567"), + ("average_value 8 bytes", "12345678"), + ("zero_value", "0"), + ]; - let bytes = b"92233720368547758079223372036854775807"; + //passing cases group.throughput(Throughput::Elements(1)); - group.bench_with_input("parse_overflowed_value", &bytes, |b, &bytes| { - b.iter(|| { - let result = parse_signed_redis(bytes); - if (result.is_some()) { - println!("failed parse") - } - }) - }); - - let string = "92233720368547758079223372036854775807"; - group.bench_with_input("parse_overflowed_value_std_lib", &string, |b, &bytes| { - b.iter(|| { - let result: Result = bytes.parse(); - if (result.is_ok()) { - println!("failed parse") - } - }) - }); - - let bytes = b"-9223372036854775807"; + for (label, value) in examples { + let bytes = value.as_bytes(); + group.bench_with_input(format!("parse i64: {}", label), &bytes, |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + assert!(result.is_some()); + }) + }); + } + + let examples = vec![("overflowed_value", "92233720368547758079223372036854775807")]; + //non-passing cases group.throughput(Throughput::Elements(1)); - group.bench_with_input("parse_min_value", &bytes, |b, &bytes| { - b.iter(|| { - let result = parse_signed_redis(bytes); - if (result.is_none()) { - println!("it's empty!") - } - }) - }); - - let string = "-9223372036854775807"; - group.bench_with_input("parse_min_value_std_lib", &string, |b, &bytes| { - b.iter(|| { - let result: Result = bytes.parse(); - if (result.is_err()) { - println!("it's empty!") - } - }) - }); - - let bytes = b"123456"; - group.throughput(Throughput::Elements(1)); - group.bench_with_input("parse_reg_value", &bytes, |b, &bytes| { - b.iter(|| { - let result = parse_signed_redis(bytes); - if (result.is_none()) { - println!("it's empty!") - } - }) - }); - - let string = "123456"; - group.bench_with_input("parse_reg_value_std_lib", &string, |b, &bytes| { - b.iter(|| { - let result: Result = bytes.parse(); - if (result.is_err()) { - println!("it's empty!") - } - }) - }); - - let bytes = b"0"; - group.throughput(Throughput::Elements(1)); - group.bench_with_input("parse_zero", &bytes, |b, &bytes| { - b.iter(|| { - let result = parse_signed_redis(bytes); - if (result.is_none()) { - println!("it's empty!") - } - }) - }); - - let string = "0"; - group.bench_with_input("parse_zero_std_lib", &string, |b, &bytes| { - b.iter(|| { - let result: Result = bytes.parse(); - if (result.is_err()) { - println!("it's empty!") - } - }) - }); + for (label, value) in examples { + let bytes = value.as_bytes(); + group.bench_with_input( + format!("parse (failed) i64: {}", label), + &bytes, + |b, &bytes| { + b.iter(|| { + let result = parse_signed_redis(bytes); + assert!(result.is_none()); + }) + }, + ); + } } criterion_group!(benches, parse_redis_benchmark); From dc8b4549380479ec9f9e3c4f12f556e244990b2e Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Tue, 7 Feb 2023 13:51:56 -0600 Subject: [PATCH 07/15] add incr instructions to RESP --- Cargo.lock | 1 + src/entrystore/Cargo.toml | 3 +- src/protocol/resp/src/request/incr.rs | 127 ++++++++++++++++++++++++++ src/protocol/resp/src/request/mod.rs | 12 +++ src/protocol/resp/src/storage/mod.rs | 1 + 5 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 src/protocol/resp/src/request/incr.rs diff --git a/Cargo.lock b/Cargo.lock index 6ae2d2cd..d2113622 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -774,6 +774,7 @@ dependencies = [ "protocol-ping", "protocol-resp", "seg", + "storage-types", ] [[package]] diff --git a/src/entrystore/Cargo.toml b/src/entrystore/Cargo.toml index a0223b42..15dea843 100644 --- a/src/entrystore/Cargo.toml +++ b/src/entrystore/Cargo.toml @@ -19,4 +19,5 @@ protocol-common = { path = "../protocol/common" } protocol-memcache = { path = "../protocol/memcache" } protocol-ping = { path = "../protocol/ping" } protocol-resp = { path = "../protocol/resp" } -seg = { path = "../storage/seg" } \ No newline at end of file +seg = { path = "../storage/seg" } +storage-types = { path = "../storage/types" } diff --git a/src/protocol/resp/src/request/incr.rs b/src/protocol/resp/src/request/incr.rs new file mode 100644 index 00000000..2349a103 --- /dev/null +++ b/src/protocol/resp/src/request/incr.rs @@ -0,0 +1,127 @@ +// Copyright 2022 Twitter, Inc. +// Licensed under the Apache License, Version 2.0 +// http://www.apache.org/licenses/LICENSE-2.0 + +use super::*; +use logger::klog; +use std::io::{Error, ErrorKind}; +use std::sync::Arc; + +#[derive(Debug, PartialEq, Eq)] +pub struct Incr { + key: Arc<[u8]>, +} + +impl TryFrom for Incr { + type Error = Error; + + fn try_from(other: Message) -> Result { + if let Message::Array(array) = other { + if array.inner.is_none() { + return Err(Error::new(ErrorKind::Other, "malformed command")); + } + + let mut array = array.inner.unwrap(); + + if array.len() != 2 { + return Err(Error::new(ErrorKind::Other, "malformed command")); + } + + let key = if let Message::BulkString(key) = array.remove(1) { + if key.inner.is_none() { + return Err(Error::new(ErrorKind::Other, "malformed command")); + } + + let key = key.inner.unwrap(); + + if key.len() == 0 { + return Err(Error::new(ErrorKind::Other, "malformed command")); + } + + key + } else { + return Err(Error::new(ErrorKind::Other, "malformed command")); + }; + + Ok(Self { key }) + } else { + Err(Error::new(ErrorKind::Other, "malformed command")) + } + } +} + +impl Incr { + pub fn new(key: &[u8]) -> Self { + Self { key: key.into() } + } + + pub fn key(&self) -> &[u8] { + &self.key + } +} + +impl From<&Incr> for Message { + fn from(other: &Incr) -> Message { + Message::Array(Array { + inner: Some(vec![ + Message::BulkString(BulkString::new(b"INCR")), + Message::BulkString(BulkString::from(other.key.clone())), + ]), + }) + } +} + +impl Compose for Incr { + fn compose(&self, buf: &mut dyn BufMut) -> usize { + let message = Message::from(self); + message.compose(buf) + } +} + +impl Klog for Incr { + type Response = Response; + + fn klog(&self, response: &Self::Response) { + let (code, len) = match response { + Message::BulkString(_) if *response == Response::null() => (ResponseCode::Miss, 0), + Message::BulkString(s) => (ResponseCode::Hit, s.len()), + _ => (ResponseCode::Miss, 0), + }; + + klog!( + "\"incr {}\" {} {}", + string_key(self.key()), + code as u32, + len + ); + } +} +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parser() { + let parser = RequestParser::new(); + assert_eq!( + parser.parse(b"incr 0\r\n").unwrap().into_inner(), + Request::Incr(Incr::new(b"0")) + ); + + assert_eq!( + parser + .parse(b"incr \"\0\r\n key\"\r\n") + .unwrap() + .into_inner(), + Request::Incr(Incr::new(b"\0\r\n key")) + ); + + assert_eq!( + parser + .parse(b"*2\r\n$3\r\nincr\r\n$1\r\n0\r\n") + .unwrap() + .into_inner(), + Request::Incr(Incr::new(b"0")) + ); + } +} diff --git a/src/protocol/resp/src/request/mod.rs b/src/protocol/resp/src/request/mod.rs index e2baa0bb..9dfae770 100644 --- a/src/protocol/resp/src/request/mod.rs +++ b/src/protocol/resp/src/request/mod.rs @@ -25,6 +25,7 @@ mod hlen; mod hmget; mod hset; mod hvals; +mod incr; mod lindex; mod llen; mod lpop; @@ -68,6 +69,7 @@ pub use hlen::*; pub use hmget::*; pub use hset::*; pub use hvals::*; +pub use incr::*; pub use sadd::*; pub use set::*; @@ -191,6 +193,7 @@ impl Parse for RequestParser { Some(b"hincrby") | Some(b"HINCRBY") => { HashIncrBy::try_from(message).map(Request::from) } + Some(b"incr") | Some(b"INCR") => Incr::try_from(message).map(Request::from), Some(b"lindex") | Some(b"LINDEX") => { ListIndex::try_from(message).map(Request::from) } @@ -263,6 +266,7 @@ impl Compose for Request { Self::HashSet(r) => r.compose(buf), Self::HashValues(r) => r.compose(buf), Self::HashIncrBy(r) => r.compose(buf), + Self::Incr(r) => r.compose(buf), Self::ListIndex(r) => r.compose(buf), Self::ListLen(r) => r.compose(buf), Self::ListPop(r) => r.compose(buf), @@ -297,6 +301,7 @@ pub enum Request { HashSet(HashSet), HashValues(HashValues), HashIncrBy(HashIncrBy), + Incr(Incr), ListIndex(ListIndex), ListLen(ListLen), ListPop(ListPop), @@ -396,6 +401,7 @@ impl Request { Self::HashSet(_) => "hset", Self::HashValues(_) => "hvals", Self::HashIncrBy(_) => "hincrby", + Self::Incr(_) => "incr", Self::ListIndex(_) => "lindex", Self::ListLen(_) => "llen", Self::ListPop(_) => "lpop", @@ -488,6 +494,12 @@ impl From for Request { } } +impl From for Request { + fn from(value: Incr) -> Self { + Self::Incr(value) + } +} + impl From for Request { fn from(value: ListIndex) -> Self { Self::ListIndex(value) diff --git a/src/protocol/resp/src/storage/mod.rs b/src/protocol/resp/src/storage/mod.rs index 98c59292..e5e2dbda 100644 --- a/src/protocol/resp/src/storage/mod.rs +++ b/src/protocol/resp/src/storage/mod.rs @@ -7,4 +7,5 @@ use crate::*; pub trait Storage { fn get(&mut self, request: &Get) -> Response; fn set(&mut self, request: &Set) -> Response; + fn incr(&mut self, request: &Incr) -> Response; } From b3f9e8388d4dc1bad870f47dd59a1491387a7656 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Tue, 7 Feb 2023 13:53:00 -0600 Subject: [PATCH 08/15] implement incr instruction for RESP via segment store --- src/entrystore/src/seg/resp.rs | 35 +++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/entrystore/src/seg/resp.rs b/src/entrystore/src/seg/resp.rs index 84d42b0d..b958b205 100644 --- a/src/entrystore/src/seg/resp.rs +++ b/src/entrystore/src/seg/resp.rs @@ -10,13 +10,16 @@ use protocol_common::*; use protocol_resp::*; +use seg::Value::{Bytes, U64}; use std::time::Duration; +use storage_types::parse_signed_redis; impl Execute for Seg { fn execute(&mut self, request: &Request) -> Response { match request { Request::Get(get) => self.get(get), Request::Set(set) => self.set(set), + Request::Incr(incr) => self.incr(incr), _ => Response::error("not supported"), } } @@ -39,10 +42,14 @@ impl Storage for Seg { ExpireTime::Seconds(n) => n, _ => 0, }; + let value = match parse_signed_redis(set.value()) { + Some(integer) => U64(integer as u64), + None => Bytes(set.value()), + }; if self .data - .insert(set.key(), set.value(), None, Duration::from_secs(ttl)) + .insert(set.key(), value, None, Duration::from_secs(ttl)) .is_ok() { Response::simple_string("OK") @@ -50,4 +57,30 @@ impl Storage for Seg { Response::error("not stored") } } + + fn incr(&mut self, incr: &Incr) -> Response { + if let Some(mut item) = self.data.get(incr.key()) { + match item.value() { + seg::Value::Bytes(b) => Response::error("wrong data type"), + seg::Value::U64(uint) => { + if let Some(incremented) = (uint as i64).checked_add(1) { + item.wrapping_add(1); + Response::integer(incremented) + } else { + Response::error("increment or decrement would overflow") + } + } + } + } else { + if self + .data + .insert(incr.key(), 1 as u64, None, Duration::from_secs(0)) + .is_ok() + { + Response::integer(1) + } else { + Response::error("not stored") + } + } + } } From 7c283e2bb0900262beaf09902ec2954a6b8f6ed4 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Tue, 7 Feb 2023 14:04:17 -0600 Subject: [PATCH 09/15] fix: copyright --- src/protocol/resp/src/request/incr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocol/resp/src/request/incr.rs b/src/protocol/resp/src/request/incr.rs index 2349a103..68094556 100644 --- a/src/protocol/resp/src/request/incr.rs +++ b/src/protocol/resp/src/request/incr.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Twitter, Inc. +// Copyright 2023 Pelikan Foundation LLC. // Licensed under the Apache License, Version 2.0 // http://www.apache.org/licenses/LICENSE-2.0 From edd04e81a09579741ceea2a15178165fb84f1ec1 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Wed, 8 Feb 2023 10:54:21 -0600 Subject: [PATCH 10/15] add missing "command" logic for incr --- src/protocol/resp/src/request/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/protocol/resp/src/request/mod.rs b/src/protocol/resp/src/request/mod.rs index 9dfae770..3099c493 100644 --- a/src/protocol/resp/src/request/mod.rs +++ b/src/protocol/resp/src/request/mod.rs @@ -609,6 +609,7 @@ pub enum Command { HashMultiGet, HashSet, HashValues, + Incr, Set, } @@ -628,6 +629,7 @@ impl TryFrom<&[u8]> for Command { b"hmget" | b"HMGET" => Ok(Command::HashMultiGet), b"hset" | b"HSET" => Ok(Command::HashSet), b"hvals" | b"HVALS" => Ok(Command::HashValues), + b"incr" | b"INCR" => Ok(Command::Incr), b"set" | b"SET" => Ok(Command::Set), _ => Err(()), } From f2e6e2fd4629d33b806735cddfea3f063ce527f1 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Wed, 8 Feb 2023 10:54:44 -0600 Subject: [PATCH 11/15] fix test with wrong encoding for string (missing correct number of bytes) --- src/protocol/resp/src/request/incr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocol/resp/src/request/incr.rs b/src/protocol/resp/src/request/incr.rs index 68094556..0430311a 100644 --- a/src/protocol/resp/src/request/incr.rs +++ b/src/protocol/resp/src/request/incr.rs @@ -118,7 +118,7 @@ mod tests { assert_eq!( parser - .parse(b"*2\r\n$3\r\nincr\r\n$1\r\n0\r\n") + .parse(b"*2\r\n$4\r\nincr\r\n$1\r\n0\r\n") .unwrap() .into_inner(), Request::Incr(Incr::new(b"0")) From ccb30f21dc7bc2b2e4369f5cc9cc08bbb856d722 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Wed, 8 Feb 2023 11:06:12 -0600 Subject: [PATCH 12/15] use new mechanism of declaring commands using `decl_request!` macro --- src/protocol/resp/src/request/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/protocol/resp/src/request/mod.rs b/src/protocol/resp/src/request/mod.rs index 257bfa9e..b4f529ac 100644 --- a/src/protocol/resp/src/request/mod.rs +++ b/src/protocol/resp/src/request/mod.rs @@ -207,6 +207,7 @@ decl_request! { HashSet(HashSet) => "hset", HashValues(HashValues) => "hvals", HashIncrBy(HashIncrBy) => "hincrby", + Incr(Incr) => "incr", ListIndex(ListIndex) => "lindex", ListLen(ListLen) => "llen", ListPop(ListPop) => "lpop", From 1bd4ff8cb98197834890bda0df340f0b07f94c37 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Thu, 9 Feb 2023 10:26:49 -0600 Subject: [PATCH 13/15] move parse.rs to protocol-common --- Cargo.lock | 3 --- src/protocol/common/Cargo.toml | 5 +++++ .../types => protocol/common}/benches/benchmark.rs | 9 ++++----- src/protocol/common/src/lib.rs | 2 ++ .../src/parse.rs => protocol/common/src/parsing.rs} | 3 ++- src/storage/types/Cargo.toml | 10 ---------- src/storage/types/src/lib.rs | 3 --- 7 files changed, 13 insertions(+), 22 deletions(-) rename src/{storage/types => protocol/common}/benches/benchmark.rs (89%) rename src/{storage/types/src/parse.rs => protocol/common/src/parsing.rs} (98%) diff --git a/Cargo.lock b/Cargo.lock index d2113622..5555d090 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2499,9 +2499,6 @@ checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" [[package]] name = "storage-types" version = "0.3.1" -dependencies = [ - "criterion 0.3.6", -] [[package]] name = "strsim" diff --git a/src/protocol/common/Cargo.toml b/src/protocol/common/Cargo.toml index 0618eccf..6f129693 100644 --- a/src/protocol/common/Cargo.toml +++ b/src/protocol/common/Cargo.toml @@ -9,6 +9,11 @@ homepage = { workspace = true } repository = { workspace = true } license = { workspace = true } +[[bench]] +name = "parsing" +path = "benches/benchmark.rs" +harness = false + [dependencies] bytes = { workspace = true } common = { path = "../../common" } diff --git a/src/storage/types/benches/benchmark.rs b/src/protocol/common/benches/benchmark.rs similarity index 89% rename from src/storage/types/benches/benchmark.rs rename to src/protocol/common/benches/benchmark.rs index ec989db8..cc6f5688 100644 --- a/src/storage/types/benches/benchmark.rs +++ b/src/protocol/common/benches/benchmark.rs @@ -3,14 +3,13 @@ // http://www.apache.org/licenses/LICENSE-2.0 use criterion::{criterion_group, criterion_main, Criterion, Throughput}; -use std::num::ParseIntError; -use storage_types::*; +use protocol_common::parsing::*; use std::time::Duration; fn parse_redis_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("parse_signed_redis"); - group.measurement_time(Duration::from_secs(2)); + group.measurement_time(Duration::from_secs(4)); let examples = vec![ ("max_value", "9223372036854775807"), @@ -23,7 +22,7 @@ fn parse_redis_benchmark(c: &mut Criterion) { //passing cases group.throughput(Throughput::Elements(1)); - for (label, value) in examples { + for (label, value) in examples.iter() { let bytes = value.as_bytes(); group.bench_with_input(format!("parse i64: {}", label), &bytes, |b, &bytes| { b.iter(|| { @@ -36,7 +35,7 @@ fn parse_redis_benchmark(c: &mut Criterion) { let examples = vec![("overflowed_value", "92233720368547758079223372036854775807")]; //non-passing cases group.throughput(Throughput::Elements(1)); - for (label, value) in examples { + for (label, value) in examples.iter() { let bytes = value.as_bytes(); group.bench_with_input( format!("parse (failed) i64: {}", label), diff --git a/src/protocol/common/src/lib.rs b/src/protocol/common/src/lib.rs index bb0ca185..de12d89e 100644 --- a/src/protocol/common/src/lib.rs +++ b/src/protocol/common/src/lib.rs @@ -8,6 +8,8 @@ pub use bytes::BufMut; +pub mod parsing; + pub const CRLF: &str = "\r\n"; pub trait Compose { diff --git a/src/storage/types/src/parse.rs b/src/protocol/common/src/parsing.rs similarity index 98% rename from src/storage/types/src/parse.rs rename to src/protocol/common/src/parsing.rs index e9ebaf62..d09b33e4 100644 --- a/src/storage/types/src/parse.rs +++ b/src/protocol/common/src/parsing.rs @@ -54,7 +54,8 @@ pub fn parse_signed_redis(bytes: &[u8]) -> Option { #[cfg(test)] mod tests { - use crate::parse::parse_signed_redis; + use crate::parsing::parse_signed_redis; + #[test] fn it_should_parse_obvious_numbers() { for x in 0..=10_000 { diff --git a/src/storage/types/Cargo.toml b/src/storage/types/Cargo.toml index 5c00f9bc..bdcef582 100644 --- a/src/storage/types/Cargo.toml +++ b/src/storage/types/Cargo.toml @@ -7,13 +7,3 @@ edition = { workspace = true } homepage = { workspace = true } repository = { workspace = true } license = { workspace = true } -[[bench]] -name = "parsing" -path = "benches/benchmark.rs" -harness = false -[dependencies] - - -[dev-dependencies] -criterion = "0.3.4" - diff --git a/src/storage/types/src/lib.rs b/src/storage/types/src/lib.rs index 7691240a..87aaeea0 100644 --- a/src/storage/types/src/lib.rs +++ b/src/storage/types/src/lib.rs @@ -2,9 +2,6 @@ // Licensed under the Apache License, Version 2.0 // http://www.apache.org/licenses/LICENSE-2.0 -mod parse; -pub use parse::*; - #[derive(PartialEq, Eq)] pub enum Value<'a> { Bytes(&'a [u8]), From b3a9281a4144d992c41db0ed5d65e335a70bf9b7 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Thu, 9 Feb 2023 10:42:34 -0600 Subject: [PATCH 14/15] cleanup --- src/entrystore/src/seg/resp.rs | 23 +++++++++++------------ src/protocol/common/src/parsing.rs | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/entrystore/src/seg/resp.rs b/src/entrystore/src/seg/resp.rs index b4addbe0..3d3b0473 100644 --- a/src/entrystore/src/seg/resp.rs +++ b/src/entrystore/src/seg/resp.rs @@ -10,9 +10,9 @@ use protocol_common::*; use protocol_resp::*; +use protocol_common::parsing::parse_signed_redis; use seg::Value::{Bytes, U64}; use std::time::Duration; -use storage_types::parse_signed_redis; impl Execute for Seg { fn execute(&mut self, request: &Request) -> Response { @@ -61,26 +61,25 @@ impl Storage for Seg { fn incr(&mut self, incr: &Incr) -> Response { if let Some(mut item) = self.data.get(incr.key()) { match item.value() { - seg::Value::Bytes(b) => Response::error("wrong data type"), + seg::Value::Bytes(_) => Response::error("value is not an integer or out of range"), seg::Value::U64(uint) => { if let Some(incremented) = (uint as i64).checked_add(1) { - item.wrapping_add(1); + item.wrapping_add(1) + .expect("we already checked that type is numeric, how can this fail?"); Response::integer(incremented) } else { Response::error("increment or decrement would overflow") } } } + } else if self + .data + .insert(incr.key(), 1_u64, None, Duration::ZERO) + .is_ok() + { + Response::integer(1) } else { - if self - .data - .insert(incr.key(), 1 as u64, None, Duration::from_secs(0)) - .is_ok() - { - Response::integer(1) - } else { - Response::error("not stored") - } + Response::error("not stored") } } } diff --git a/src/protocol/common/src/parsing.rs b/src/protocol/common/src/parsing.rs index d09b33e4..40322d08 100644 --- a/src/protocol/common/src/parsing.rs +++ b/src/protocol/common/src/parsing.rs @@ -42,7 +42,7 @@ pub fn parse_signed_redis(bytes: &[u8]) -> Option { match &bytes[0] { b'-' if bytes.len() >= 2 && bytes[1] != b'0' => { let first_digit = convert_digit(bytes[1])?; - parse_rest((first_digit as i64) * -1, &bytes[2..], -1) + parse_rest(-(first_digit as i64), &bytes[2..], -1) } other if (b'1'..=b'9').contains(other) => { let digit = convert_digit(*other)?; From bfeaf7b47468be2d499d1f4db620f5c87a437001 Mon Sep 17 00:00:00 2001 From: Dermot Haughey Date: Thu, 9 Feb 2023 11:05:17 -0600 Subject: [PATCH 15/15] add entrystore module-level integration tests to demonstrate storage behavior matches expected behavior --- src/entrystore/src/seg/resp.rs | 92 ++++++++++++++++++++++++++++ src/protocol/resp/src/message/mod.rs | 3 + 2 files changed, 95 insertions(+) diff --git a/src/entrystore/src/seg/resp.rs b/src/entrystore/src/seg/resp.rs index 3d3b0473..1dd5b269 100644 --- a/src/entrystore/src/seg/resp.rs +++ b/src/entrystore/src/seg/resp.rs @@ -83,3 +83,95 @@ impl Storage for Seg { } } } + +#[cfg(test)] +mod tests { + use crate::Seg; + use config::RdsConfig; + use protocol_resp::*; + + #[test] + fn it_should_set() { + let config = RdsConfig::default(); + let mut seg = Seg::new(&config).expect("could not initialize seg"); + let set_missing = Set::new(b"missing", b"value", None, SetMode::Set, false); + let response = seg.set(&set_missing); + assert_eq!(response, Response::ok()); + } + + #[test] + fn it_should_get() { + //setup + let config = RdsConfig::default(); + let mut seg = Seg::new(&config).expect("could not initialize seg"); + + //missing + let get_missing = Get::new(b"missing"); + let response = seg.get(&get_missing); + assert_eq!(response, Response::null()); + + //get something set + let key = b"foo"; + let value = b"bar"; + let set = Set::new(key, value, None, SetMode::Set, false); + let response = seg.set(&set); + assert_eq!(response, Response::ok()); + + let get = Get::new(key); + let response = seg.get(&get); + assert_eq!(response, Response::bulk_string(value)); + } + + #[test] + fn it_should_incr() { + //setup + let config = RdsConfig::default(); + let mut seg = Seg::new(&config).expect("could not initialize seg"); + + // incr missing + let incr_missing = Incr::new(b"missing"); + let response = seg.incr(&incr_missing); + assert_eq!(response, Response::integer(1)); + + // incr numeric set + let key = b"number"; + let number = 123456_i64; + let set = Set::new( + key, + number.to_string().as_bytes(), + None, + SetMode::Set, + false, + ); + seg.set(&set); + + let incr_numeric = Incr::new(key); + let response = seg.incr(&incr_numeric); + assert_eq!(response, Response::integer(number + 1)); + + // incr string set + let key = b"string"; + let set = Set::new(key, b"value", None, SetMode::Set, false); + seg.set(&set); + + let incr_missing = Incr::new(key); + let response = seg.incr(&incr_missing); + assert_eq!( + response, + Response::error("value is not an integer or out of range") + ); + + // incr overflow + let key = b"string"; + let value = b"9223372036854775807"; + let set = Set::new(key, value, None, SetMode::Set, false); + seg.set(&set); + + let incr_overflow = Incr::new(key); + let response = seg.incr(&incr_overflow); + assert_eq!( + response, + Response::error("increment or decrement would overflow") + ); + } +} diff --git a/src/protocol/resp/src/message/mod.rs b/src/protocol/resp/src/message/mod.rs index ae0c36c6..1f97006a 100644 --- a/src/protocol/resp/src/message/mod.rs +++ b/src/protocol/resp/src/message/mod.rs @@ -50,6 +50,9 @@ impl Message { pub fn bulk_string(value: &[u8]) -> Self { Self::BulkString(BulkString::new(value)) } + pub fn ok() -> Self { + Self::simple_string("OK") + } } impl Compose for Message {