Qovery / RedisLess

RedisLess is a fast, lightweight, embedded and scalable in-memory Key/Value store library compatible with the Redis API.
MIT License
150 stars 16 forks source link

Add commands #49

Closed tbmreza closed 3 years ago

tbmreza commented 3 years ago

4 #5 #28

barkanido commented 3 years ago

@tbmreza maybe take a look on #50 ? for implementing TYPE command? it might help you when/if it is merged.

tbmreza commented 3 years ago

maybe take a look on #50 ? for implementing TYPE command? it might help you when/if it is merged.

@barkanido Thanks for the heads-up. Instead of matching on self.data_mapper.get(key), after that PR is merged I could match on self.meta(key).

This (#49) shouldn't be in that PR's way though, unless I'm missing something.

tbmreza commented 3 years ago

This one case fails, haven't dug into it. server::tests::expire_and_ttl (https://github.com/Qovery/RedisLess/blob/main/redisless/src/server/tests/mod.rs#L102) @barkanido Any idea?

Other than that, I did rebuild and retest using the python client. Please let me know what you think. @clarity0 @evoxmusic

clarity0 commented 3 years ago

Can you add tests to src/server/tests/mod.rs ? DECR and DECRBY are supported by redis-rs

barkanido commented 3 years ago

This one case fails, haven't dug into it. server::tests::expire_and_ttl (https://github.com/Qovery/RedisLess/blob/main/redisless/src/server/tests/mod.rs#L102) @barkanido Any idea?

Other than that, I did rebuild and retest using the python client. Please let me know what you think. @clarity0 @evoxmusic

works on my machine in main, but the root cause should be that the test sleeps and then asserting on the remaining TTL. this approach although easy to implement produces somewhat unstable tests (sleep is non-deterministic). I suggest you fiddle with the duration var a bit to stabilize this.

tbmreza commented 3 years ago

works on my machine in main, but the root cause should be that the test sleeps and then asserting on the remaining TTL. this approach although easy to implement produces somewhat unstable tests (sleep is non-deterministic). I suggest you fiddle with the duration var a bit to stabilize this.

On my machine the returned ttl is very unstable. I ended up changing the delta to 10x. The test passes now, hopefully it doesn't defeat its purpose :D @barkanido

tbmreza commented 3 years ago

Anything else? @clarity0

evoxmusic commented 3 years ago

@tbmreza is it all good for you? (I'll review if yes - please use "draft" mode if you are still working on it). Thank you guys

tbmreza commented 3 years ago

@evoxmusic yes, all good to me.

evoxmusic commented 3 years ago

It looks like tests are failing @tbmreza - can you check?

tbmreza commented 3 years ago

That's weird. A different test case (server::tests::mget) is flaky on my machine, not the one that's failing in that CI.

What do you suggest me to do? I'm thinking about wrapping the Connection's get result in an if-let so it won't panic. But that's practically ignoring the test case. @evoxmusic

barkanido commented 3 years ago

That's weird. A different test case (server::tests::mget) is flaky on my machine, not the one that's failing in that CI.

What do you suggest me to do? I'm thinking about wrapping the Connection's get result in an if-let so it won't panic. But that's practically ignoring the test case. @evoxmusic

Yeah, tests are designed to panic on failure in rust. Why is it flaky though? Maybe it discovered a bug?

tbmreza commented 3 years ago

That's weird. A different test case (server::tests::mget) is flaky on my machine, not the one that's failing in that CI.

What do you suggest me to do? I'm thinking about wrapping the Connection's get result in an if-let so it won't panic. But that's practically ignoring the test case. @evoxmusic

Yeah, tests are designed to panic on failure in rust. Why is it flaky though? Maybe it discovered a bug?

By flaky I mean I simply rerun the test (without any change) and the test passes.

tbmreza commented 3 years ago

Oops didn't read your comment correctly. I'll try to look why when I can.

tbmreza commented 3 years ago

So I made a mistake by reusing the same port number for different cases. That sometimes causes starting the server returns Timeout.