Add read from replica round robin to client CMD #315

shachlanAmazon · 2023-07-17T10:24:44Z

based over https://github.com/barshaul/babushka/pull/306

babushka-core/src/client/client_cmd.rs

barshaul · 2023-07-19T13:45:41Z

babushka-core/tests/test_client_cmd.rs

+            (0..4).map(|_| get_listener_on_available_port()).collect();
+        let mut primary_responses = std::collections::HashMap::new();
+        primary_responses.insert(
+            "*2\r\n$4\r\nINFO\r\n$11\r\nREPLICATION\r\n".to_string(),


can we use a parser function to encode the commands into resp protocol, instead of hardcoding it?

what format would you prefer?

You can use the same format (resp), but why dont you use some parser.to_resp(["INFO", "REPLICATION"])?
Dont we have this functionallity in redis-rs parser?

no, only with Value, which is even more verbose. But if you prefer, I can use Value.

I think it would be better (dont they have some easy fuction for vec=>Value and then Value=>resp?? eef).
I think it would make it harder to write tests if each test writer will need to manually encode the expected result to a resp string

maybe use https://docs.rs/redis-protocol/latest/redis_protocol/ ?

doesn't resolve to Value, and doesn't seem significantly less verbose.

barshaul · 2023-07-19T13:51:26Z

babushka-core/tests/test_client_cmd.rs

+
+        for mock in mocks.iter() {
+            for _ in 0..3 {
+                mock.add_response(&cmd, "$3\r\nbar\r\n".to_string());


same - can we pass simple response string and let it be parsed to resp inside add_resopnse?

how will we handle complex responses such as arrays?

babushka-core/tests/test_client_cmd.rs

babushka-core/src/client/client_cmd.rs

shachlanAmazon · 2023-07-31T10:11:25Z

@barshaul round

barshaul

Approved, some comments

babushka-core/src/client/client_cmd.rs

barshaul · 2023-08-02T08:49:12Z

babushka-core/src/client/reconnecting_connection.rs

-    pub(super) fn reconnect(&self) {
-        {
+    pub(super) fn reconnect(&self, force_reconnect: bool) {
+        if !force_reconnect {


Why do we need to force reconnect?

Because if the client was created in reconnecting state, without force reconnection it will never start the initial reconnect attempt.

This is done by adding an initial state for `ReconnectingConnection`, which doesn't block reconnect. Additionally, as per request, moved the call to `reconnect` when creating a disconnected connection into `ReconnectingConnection`.

This is done in order to handle heartbeats.

shachlanAmazon · 2023-08-02T16:52:29Z

@barshaul pushed some more commits with various fixes needed in order to reduce flakeyness. please review.

Java: Update implementation for `FUNCTION LOAD`. (#315) * Update implementation for `FUNCTION LOAD`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com>

shachlanAmazon requested a review from barshaul July 17, 2023 10:24

barshaul reviewed Jul 19, 2023

View reviewed changes

shachlanAmazon force-pushed the read-from-replica-cmd branch 3 times, most recently from b479ffb to feaa8c6 Compare July 31, 2023 09:49

barshaul approved these changes Aug 2, 2023

View reviewed changes

shachlanAmazon force-pushed the read-from-replica-cmd branch from feaa8c6 to 664da0e Compare August 2, 2023 09:30

shachlanAmazon added 2 commits August 2, 2023 11:47

Create list of replicas.

8792645

Add CMD read from replica.

ceb2e5f

shachlanAmazon force-pushed the read-from-replica-cmd branch 2 times, most recently from 5635b36 to 944b2d6 Compare August 2, 2023 13:23

shachlanAmazon added 3 commits August 2, 2023 16:15

Remove the need to force reconnect.

17c146f

This is done by adding an initial state for `ReconnectingConnection`, which doesn't block reconnect. Additionally, as per request, moved the call to `reconnect` when creating a disconnected connection into `ReconnectingConnection`.

Added handling PING to the mocks.

f6c5de7

This is done in order to handle heartbeats.

Await for mocks to properly close before testing.

0696e2b

shachlanAmazon force-pushed the read-from-replica-cmd branch from 1b4e066 to 0696e2b Compare August 2, 2023 16:16

Verify connection liveness on reconnect.

321d7b4

shachlanAmazon force-pushed the read-from-replica-cmd branch 2 times, most recently from 830cfed to 321d7b4 Compare August 2, 2023 16:51

barshaul approved these changes Aug 3, 2023

View reviewed changes

shachlanAmazon merged commit 27c2fb2 into valkey-io:main Aug 3, 2023
7 of 8 checks passed

shachlanAmazon deleted the read-from-replica-cmd branch August 3, 2023 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add read from replica round robin to client CMD #315

Add read from replica round robin to client CMD #315

shachlanAmazon commented Jul 17, 2023

barshaul Jul 19, 2023

shachlanAmazon Jul 24, 2023 •

edited

Loading

barshaul Jul 25, 2023

shachlanAmazon Jul 25, 2023

barshaul Jul 25, 2023 •

edited

Loading

barshaul Jul 25, 2023

shachlanAmazon Jul 25, 2023

barshaul Jul 19, 2023

shachlanAmazon Jul 24, 2023

shachlanAmazon commented Jul 31, 2023

barshaul left a comment

barshaul Aug 2, 2023

shachlanAmazon Aug 2, 2023

shachlanAmazon commented Aug 2, 2023

Add read from replica round robin to client CMD #315

Add read from replica round robin to client CMD #315

Conversation

shachlanAmazon commented Jul 17, 2023

Choose a reason for hiding this comment

shachlanAmazon Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barshaul Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shachlanAmazon commented Jul 31, 2023

barshaul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shachlanAmazon commented Aug 2, 2023

shachlanAmazon Jul 24, 2023 •

edited

Loading

barshaul Jul 25, 2023 •

edited

Loading