search

uncleDecart / nkv

Share your state between services using persisted key value storage with notification option
1 stars 0 forks source link

Add keyspace and make notifiers async #10

Closed uncleDecart closed 1 month ago

uncleDecart commented 2 months ago

Still need proper documentation with design decisions and nkv limitations in place, once I'll add those I'll describe also the context of this PR and why notifications are now asynchronous. Will ping you once I'm done, Avi :)

uncleDecart commented 2 months ago

Okay, I want to add benchmarks and load testing to see how this new version is going to behave, plus I want to do a small screen recording with terminals showing nkv client and server and work. You're right, it's about API, not language. So I moved stuff around, removing almost everyting related to rust under different docs and feel free to ask me any questions, this change was kind of not intuitive and took me a while to figure it out

uncleDecart commented 2 months ago

benchmark results are just awful, regression by 1000% is not okay, I guess there's something wrong with my trie implementation and problems with multithreading programming I did, investigating. Such a good tool though, this criterion.rs

uncleDecart commented 2 months ago

Edit: I was comparing apples to oranges, meaning results from my laptop vs github runner, on same machine it shows significant perfomance degradation

So apparently it's faster everywhere, except server, because in server benchmarks we wait to receive updated value from notification, and now we're doing it lazy way, meaning we have a separate thread to send notifications to all clients and we kick a notification channel to do so, that's why delays, it's really huge, like could be 10x slower, but I'm not sure how we can improve that. Let's talk tomorrow and see about that. Meanwhile I'll just touch up some other things...

uncleDecart commented 1 month ago

Fixed some document typos and added handler for client.

So this is server build with debug symbols and two clients connecting to it, one is putting value, subscribing to updates, and another getting value and updating it. Server is just 14% of samples... Which is weird tbh. I'll add time measurement on client side, I want to see the response time on client side which would be sort of real one with unix sockets flamegraph

uncleDecart commented 1 month ago

So now this PR also fixes #7

uncleDecart commented 1 month ago

Looking at the client times with TCP stack it makes me think that the problem could be in Benchmarking suite, real perfomance is 1ms per 1K of data compared to 72 ms in tests. I think we should merge this @deitch and then see to the benchmarking problem separately, I think these changes are enough to demo nkv to the team and gather feedback