Added support for redis sentinel #553
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unfortunately I was not able to test this locally as the test keep failing with a stack overflow error I have no clue as to what causes this since the new code should not even be invoked, as I have not added any test case for this. For me it fails reliably with Stack trace show a deep callstack; but nothing obvious that raises alarms for me: @brocaar any idea what might cause this? My only suspicion is that this adds an extra dependency for deadpool-redis (tokio) that causes some behavioral change; but according to the docs of deadpool it should not |
|
Thanks @ederuiter :+: Lets see if running the test using GitHub Actions raises the same stack overflow. |
|
failed :-( Another issue is that currently the database selection fails if you use another database number than 0 as it tries to select the database on the sentinel; not on the actual redis connection |
…:test_sns_uplink test to run
|
@brocaar It seems that for some reason my changes increased the used stack space slightly .. just to push it over the limit for this test Debugging with gdb seems to show that the used stack space up until the overflow (see https://stackoverflow.com/a/37327019) is approximately 2MB: make test
...
gdb --args /home/eric/code/oss/chirpstack/target/debug/deps/chirpstack-f5322796d11f3a3f test::class_a_pr_test::test_sns_uplink
...
(gdb) r
Thread 2 "test::class_a_p" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 3333828]
0x00005555567f73e1 in chirpstack::downlink::data::Data::set_mac_commands::{{closure}} ()
at chirpstack/src/downlink/data.rs:629
629 async fn set_mac_commands(&mut self) -> Result<()> {
(gdb) frame 0
#0 0x00005555567f73e1 in chirpstack::downlink::data::Data::set_mac_commands::{{closure}} ()
at chirpstack/src/downlink/data.rs:629
629 async fn set_mac_commands(&mut self) -> Result<()> {
(gdb) set $topsp=$sp
(gdb) bt
...
#60 0x00007ffff7d59272 in start_thread () from /nix/store/kpy2cyd05vdr6j1h200av81fnlxl1jw0-glibc-2.39-52/lib/libc.so.6
#61 0x00007ffff7dd4dec in clone3 () from /nix/store/kpy2cyd05vdr6j1h200av81fnlxl1jw0-glibc-2.39-52/lib/libc.so.6
(gdb) frame 61
#61 0x00007ffff7dd4dec in clone3 () from /nix/store/kpy2cyd05vdr6j1h200av81fnlxl1jw0-glibc-2.39-52/lib/libc.so.6
(gdb) print $sp - $topsp
$1 = 2094680running the tests with This could be caused by the many (according to: https://www.reddit.com/r/rust/comments/10l99ai/comment/j5wm0ix/) |
|
I also tested this without my changes and setting a breakpoint on the exact same spot: gdb --args /home/eric/code/oss/chirpstack/target/debug/deps/chirpstack-5554738068c9e0c3 test::class_a_pr_test::test_sns_uplink
...
(gdb) b chirpstack::downlink::data::Data::set_mac_commands::{{closure}}
Breakpoint 1 at 0xd0622d: file chirpstack/src/downlink/data.rs, line 629.
(gdb) r
...
Switching to LWP 3410467]
Thread 2 "test::class_a_p" hit Breakpoint 1, chirpstack::downlink::data::Data::set_mac_commands::{{closure}} ()
at chirpstack/src/downlink/data.rs:629
629 async fn set_mac_commands(&mut self) -> Result<()> {
(gdb) bt
#0 chirpstack::downlink::data::Data::set_mac_commands::{{closure}} () at chirpstack/src/downlink/data.rs:629
...
#61 0x00007ffff7dd4dec in clone3 () from /nix/store/kpy2cyd05vdr6j1h200av81fnlxl1jw0-glibc-2.39-52/lib/libc.so.6
(gdb) frame 0
#0 chirpstack::downlink::data::Data::set_mac_commands::{{closure}} () at chirpstack/src/downlink/data.rs:629
629 async fn set_mac_commands(&mut self) -> Result<()> {
(gdb) set $topsp=$sp
(gdb) frame 61
#61 0x00007ffff7dd4dec in clone3 () from /nix/store/kpy2cyd05vdr6j1h200av81fnlxl1jw0-glibc-2.39-52/lib/libc.so.6
(gdb) print $sp - $topsp
$1 = 1806416So my changes only increased the used stack space by ~20KB (2094680 vs 1806416) .. but this was enough to push it over the edge |
|
I haven't had the time to look into this more deeply, but I found somewhere that there is a difference with regards to stack size when running tests (2MB) vs running the application (8MB). The fact that the tests are compile with debug profile doesn't help with regards to the stack size. |
|
Once the new version of deadpool-redis is released (deadpool-rs/deadpool#369) I will update this MR However I did found out that |
|
Hi @ederuiter, would you mind rebasing the pull-request? Note that I just update redis-sentinel and redis crates in the master branch. |
|
Hi @ederuiter in case you missed my previous msg, I did update update the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support for redis sentinal was introduced in deadpool-redis 0.17