Tests: Add retries to test cluster creation #994
Merged
Conversation
This file contains 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
jaymell
force-pushed
the
cluster-creation-retries
branch
from
3b6edef
to
0449541
Compare
jaymell
force-pushed
the
cluster-creation-retries
branch
3 times, most recently
from
eb7ccee
to
b6d9651
Compare
jaymell
commented
Nov 26, 2023
nihohit
reviewed
Nov 27, 2023
jaymell
force-pushed
the
cluster-creation-retries
branch
from
b6d9651
to
d3d7b20
Compare
altanozlu
added a commit
to altanozlu/redis-rs
that referenced
this pull request
Nov 30, 2023
barshaul
pushed a commit
to amazon-contributing/redis-rs
that referenced
this pull request
Dec 7, 2023
* Disable JSON module tests for redis 6.2.4. (redis-rs#980) This happens due to a consistent crash - RedisJSON/RedisJSON#1124 * Implement `FromRedisValue` for `Box<[T]>` and `Arc<[T]>` (redis-rs#799) * Update minimal rust version to 1.6.5 (redis-rs#982) It's required by regex v1.10.2. https://github.com/redis-rs/redis-rs/actions/runs/6760188489/job/18373652560?pr=981 * Support Mutual TLS (redis-rs#858) Add support for mutual TLS. Enable specifying client certificate and key and (optional) root certificate. * changing timeouts from usize and isize to f64 (redis-rs#988) Co-authored-by: Eythor Helgason <eytorhelgason@gmail.com> * Accept iterator at `ClusterClient` initialization (redis-rs#987) * Prefer routing to primary in a transaction. (redis-rs#986) * Rename route_pipeline to route_for_pipeline. There are 2 `route_pipeline` functions in the file, so we change one to avoid confusion. * Prefer routing to primary in a transaction. * CrossSlot error on pipeline with many routes. * Sync Pub/Sub - cache received pub/sub messages. (redis-rs#910) When sending subscibre/unsubscribe messages to the server, the pub/sub client might receive messages that it can mistake to be the response, and not return to the next `get_message` call. This fix ensures that pub/sub messages will be queued until the next `get_message` call, so that no message will be missed. * Fix sync cluster behavior with transactions. (redis-rs#983) The leading MULTI prevents the transaction from being properly routed. In order to route the request correctly, we drop the leading MULTI from the routable. * Release redis 0.23.4 / redis-test 0.2.4 (redis-rs#992) * Tests: Add retries to test cluster creation (redis-rs#994) * Revert redis-test versioning changes (redis-rs#993) Publishing to crates.io without the version specification is currently not possible. Reverting these changes for now until we have a better solution/workaround. * Fix features for `load_native_certs`. (redis-rs#996) The directive now matches the directives wrapping `load_native_certs`'s `use` statements. This would fail compilation for anyone accidentally using the "tls" feature with rustls. * Bump aHash to v0.8.6 (redis-rs#966) * Update Command expiration values to be an appropriate type (redis-rs#589) This change updates expiration values provided to some commands in the `Command` trait to be of type `i64` or `u64` where appropriate, replacing the previously used `usize` type. More specifically, the commands that are updated are - SETEX, PSETEX - Expiration value changed from `usize` to `u64` - EXPIRE, PEXPIRE, EXPIREAT, PEXPIREAT - Expiration value changed from `usize` to `i64` For the `*SETEX` commands, there is no mention of supporting negative values. And indeed, providing a negative value through redis-cli returns an error message. For the `*EXPIRE*` commands, I couldn't find the paragraph in the docs referenced in redis-rs#575, nor any other mention that the commands accept a negative expiration argument. However, in testing with redis-cli it seems a negative expiration value is valid and in checking the code I found it to confirm these observations. See [here][code1] and [here][code2]. Note that this is a breaking change. [code1]: https://github.com/redis/redis/blob/e3ef73dc2a557232c60c732705e8e6ff2050eba9/src/expire.c#L570-L571 [code2]: https://github.com/redis/redis/blob/e3ef73dc2a557232c60c732705e8e6ff2050eba9/src/expire.c#L483-L491 * Fix StreamId::contains_key signature (redis-rs#783) * Order in usage of ClusterParams. (redis-rs#997) * Move `tls_params` into `ClusterParams`. The two objects are used together, so it makes sense to join them. * Cluster: Contain `ClusterParams` internally. This matches the async cluster's structure, and ensures that fields that are added to `ClusterParams` are automatically added to the sync cluster * Release redis 0.24.0 / redis-test 0.3.0 (redis-rs#998) --------- Co-authored-by: Jacob O'Toole <jacob@jotpot.co.uk> Co-authored-by: spaceangel <129979014+sp-angel@users.noreply.github.com> Co-authored-by: eythorhel19 <70693645+eythorhel19@users.noreply.github.com> Co-authored-by: Eythor Helgason <eytorhelgason@gmail.com> Co-authored-by: Ruan Petterson <ruan@petterson.eng.br> Co-authored-by: James Lucas <jaymell@users.noreply.github.com> Co-authored-by: aumetra <aumetra@cryptolab.net> Co-authored-by: Josh Leeb-du Toit <mail@joshleeb.com> Co-authored-by: Ayush <ayushsingh1325@gmail.com>
nihohit
pushed a commit
that referenced
this pull request
Dec 24, 2023
* squash! * oops! * test invalidation in cluster && introduce client tracking options * introduce basic PubSub functionality to MultiplexedConnection and make tokio sender unbounded channel * fix tests & linter, make PushSender::Tokio as aio feature only * add resp3 to ci branches and fix cluster client tracking option * test dropping and update ci yml * remove unsubscribe fn and introduce unsubscribing by dropping receiver. * fix tests because RedisJson returns responses in an array when it's resp3 * override redisjson cache (it's a temp solution) * add -skip test_module to RESP3 testing and upgrade redis 6.2.4 to 6.2.13 * test json modules with RESP3 and get json fix from main * in redis v7 RedisJson is different with Resp3 🤔 * Implement Pub/Sub in Async Cluster & fmt & remove usage of is_err_and(stable only after v1.70) * don't use sharded pub/sub with redis v6 * use REDIS_VERSION in env instead of using HELLO command to fetch redis version * oops * fix linter * fix fmt and remove benchmark from CI * simplify PushManager and add tokio as non-optional dependency. * get fixes from 220c6a9 * use --test-threads=1 * override redisjson cache (it's a temp solution) * remove get_push_manager from traits & remove push manager from aio::Connection * remove client_tracking_options * remove 0.21.x from rust.yml * add tests for pushmanager * format & move push_info into a variable * change tests according to comments. * apply 6.2.4 changes && fmt * try to fix * remove con_addr & remove pub/sub support in cluster connections * add disconnection handling to sync, mpx, cm && test it * remove push_manager argument from connection creation * better docs * add has_reply function to PushKind * remove no response command support in mpx since it's not used in mpx pub/sub * apply changes from #994 * fix tests
shachlanAmazon
pushed a commit
to amazon-contributing/redis-rs
that referenced
this pull request
Feb 26, 2024
* Initial implementation of RESP3 (redis-rs#757) These changes implement all RESP3 types (excluding streamed types). RESP3 can be enabled per connection by adding `?resp3=true` to connection uri. It currently supports PubSub as RESP2 PubSub support in library, but in future PRs it'll support handling normal commands and PubSub messages in one connection. Only `num-bigint` is added as dependency to support `BigNumber` type. Changes made in support of redis-rs#329 and redis-rs#749 * Add RESP3 support to cluster connections. (redis-rs#1001) * Resp3 Push Management (redis-rs#898) * squash! * oops! * test invalidation in cluster && introduce client tracking options * introduce basic PubSub functionality to MultiplexedConnection and make tokio sender unbounded channel * fix tests & linter, make PushSender::Tokio as aio feature only * add resp3 to ci branches and fix cluster client tracking option * test dropping and update ci yml * remove unsubscribe fn and introduce unsubscribing by dropping receiver. * fix tests because RedisJson returns responses in an array when it's resp3 * override redisjson cache (it's a temp solution) * add -skip test_module to RESP3 testing and upgrade redis 6.2.4 to 6.2.13 * test json modules with RESP3 and get json fix from main * in redis v7 RedisJson is different with Resp3 🤔 * Implement Pub/Sub in Async Cluster & fmt & remove usage of is_err_and(stable only after v1.70) * don't use sharded pub/sub with redis v6 * use REDIS_VERSION in env instead of using HELLO command to fetch redis version * oops * fix linter * fix fmt and remove benchmark from CI * simplify PushManager and add tokio as non-optional dependency. * get fixes from 220c6a9 * use --test-threads=1 * override redisjson cache (it's a temp solution) * remove get_push_manager from traits & remove push manager from aio::Connection * remove client_tracking_options * remove 0.21.x from rust.yml * add tests for pushmanager * format & move push_info into a variable * change tests according to comments. * apply 6.2.4 changes && fmt * try to fix * remove con_addr & remove pub/sub support in cluster connections * add disconnection handling to sync, mpx, cm && test it * remove push_manager argument from connection creation * better docs * add has_reply function to PushKind * remove no response command support in mpx since it's not used in mpx pub/sub * apply changes from redis-rs#994 * fix tests * Use enum instead of boolean for RESP version. (redis-rs#1012) Since there's a discussion starting about what might become RESP4, this PR will make it easier to add more RESP versions in the future. * Add lock file to keep MSRV constant. (redis-rs#1039) * [opt] preallocate buffer for evalsha in `Script` (redis-rs#1044) * Cluster: fix read from replica & missing slots. (redis-rs#965) * Cluster SlotMap: Allow missing slots. The cluster can still operate when some slots don't have coverage, so there's no reason that the connection won't work. Requests that should be routed to missing slots will be forwarded to a random node. * Fix SlotAddrs not holding all replicas. This change fixes 2 issues - 1. Since SlotAddrs saves at most 1 replica (or none, if `read_from_replica` is false), it means that when there are 2 or more replicas in the cluster, `AllNodes` commands aren't actually sent to all nodes. 2. `SlotAddr::Replica` carries 2 separate semantic meanings - it could mean that the command is a readonly command, or that the user asked to route the command to a replica. The first case should be affected by the `read_from_replica` client flag, but the second case shouldn't - if the user requested that this specific command be routed to the replica, the specific request should override the prior configuration. * Fix multi-slot routes/addresses mismatch. If the `SlotMap` is partial, and some slots don't have addresses, then we need to communicate this back when getting addresses for multi-slot commands. Otherwise we the key indices won't match the addresses, and commands with the wrong keys will be sent to the wrong nodes. * fix typo * fix redis-rs#1045: impl `Clone`/`Copy` for `SetOptions` (redis-rs#1046) * docs: add "connection-manager" cfg attr (redis-rs#1048) * Fix new clippy lints. (redis-rs#1052) Replace Vec reference with a slice, and remove iterator trait when using ExactSizeIterator, since it encompasses it. * Rename Value enum types in order to match Redis RESP names. (redis-rs#779) * Rename Value::Bulk to Value::Array. * Rename Value::Status to Value::SimpleString. * Rename Value::Data to Value::BulkString. * Fix debug names of values. * Fix nightly lints. --------- Co-authored-by: Altan Özlü <5479094+altanozlu@users.noreply.github.com> Co-authored-by: Huxley Hu <framlog@users.noreply.github.com> Co-authored-by: Ahmad <39441506+ahmadbky@users.noreply.github.com> Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com>
nihohit
added a commit
that referenced
this pull request
Mar 12, 2024
* Initial implementation of RESP3 (#757) These changes implement all RESP3 types (excluding streamed types). RESP3 can be enabled per connection by adding `?resp3=true` to connection uri. It currently supports PubSub as RESP2 PubSub support in library, but in future PRs it'll support handling normal commands and PubSub messages in one connection. Only `num-bigint` is added as dependency to support `BigNumber` type. Changes made in support of #329 and #749 * Add RESP3 support to cluster connections. (#1001) * Resp3 Push Management (#898) * squash! * oops! * test invalidation in cluster && introduce client tracking options * introduce basic PubSub functionality to MultiplexedConnection and make tokio sender unbounded channel * fix tests & linter, make PushSender::Tokio as aio feature only * add resp3 to ci branches and fix cluster client tracking option * test dropping and update ci yml * remove unsubscribe fn and introduce unsubscribing by dropping receiver. * fix tests because RedisJson returns responses in an array when it's resp3 * override redisjson cache (it's a temp solution) * add -skip test_module to RESP3 testing and upgrade redis 6.2.4 to 6.2.13 * test json modules with RESP3 and get json fix from main * in redis v7 RedisJson is different with Resp3 🤔 * Implement Pub/Sub in Async Cluster & fmt & remove usage of is_err_and(stable only after v1.70) * don't use sharded pub/sub with redis v6 * use REDIS_VERSION in env instead of using HELLO command to fetch redis version * oops * fix linter * fix fmt and remove benchmark from CI * simplify PushManager and add tokio as non-optional dependency. * get fixes from 220c6a9 * use --test-threads=1 * override redisjson cache (it's a temp solution) * remove get_push_manager from traits & remove push manager from aio::Connection * remove client_tracking_options * remove 0.21.x from rust.yml * add tests for pushmanager * format & move push_info into a variable * change tests according to comments. * apply 6.2.4 changes && fmt * try to fix * remove con_addr & remove pub/sub support in cluster connections * add disconnection handling to sync, mpx, cm && test it * remove push_manager argument from connection creation * better docs * add has_reply function to PushKind * remove no response command support in mpx since it's not used in mpx pub/sub * apply changes from #994 * fix tests * Use enum instead of boolean for RESP version. (#1012) Since there's a discussion starting about what might become RESP4, this PR will make it easier to add more RESP versions in the future. * Rename Value enum types in order to match Redis RESP names. (#779) * Rename Value::Bulk to Value::Array. * Rename Value::Status to Value::SimpleString. * Rename Value::Data to Value::BulkString. * Fix debug names of values. * fix nightly comments. * reintroduce client tracking to tests. * fix merge errors. --------- Co-authored-by: Altan Özlü <5479094+altanozlu@users.noreply.github.com>
shachlanAmazon
pushed a commit
to amazon-contributing/redis-rs
that referenced
this pull request
Mar 13, 2024
* Add lock file to keep MSRV constant. (redis-rs#1039) * [opt] preallocate buffer for evalsha in `Script` (redis-rs#1044) * Cluster: fix read from replica & missing slots. (redis-rs#965) * Cluster SlotMap: Allow missing slots. The cluster can still operate when some slots don't have coverage, so there's no reason that the connection won't work. Requests that should be routed to missing slots will be forwarded to a random node. * Fix SlotAddrs not holding all replicas. This change fixes 2 issues - 1. Since SlotAddrs saves at most 1 replica (or none, if `read_from_replica` is false), it means that when there are 2 or more replicas in the cluster, `AllNodes` commands aren't actually sent to all nodes. 2. `SlotAddr::Replica` carries 2 separate semantic meanings - it could mean that the command is a readonly command, or that the user asked to route the command to a replica. The first case should be affected by the `read_from_replica` client flag, but the second case shouldn't - if the user requested that this specific command be routed to the replica, the specific request should override the prior configuration. * Fix multi-slot routes/addresses mismatch. If the `SlotMap` is partial, and some slots don't have addresses, then we need to communicate this back when getting addresses for multi-slot commands. Otherwise we the key indices won't match the addresses, and commands with the wrong keys will be sent to the wrong nodes. * fix typo * fix redis-rs#1045: impl `Clone`/`Copy` for `SetOptions` (redis-rs#1046) * docs: add "connection-manager" cfg attr (redis-rs#1048) * Fix new clippy lints. (redis-rs#1052) Replace Vec reference with a slice, and remove iterator trait when using ExactSizeIterator, since it encompasses it. * Async cluster connection: Improve handling of missing connections (redis-rs#968) * aio::ClusterConnection: Report missing connections. This change should ensure that if a connection wasn't found, after redirecting to node, or when no random connection is available, then the cluster connection will refresh slots. * Add sleep to refresh slots action. * Cancel redirects after disconnects. If a redirected request reaches a disconnected node, the redirection will be cancelled, and the routing will revert to the original routing. * Move OperationTarget to Err side of result, and reduce generics. OperationTarget is used only on errors, so it should be in the `Err` case only. The added generics were used to hide a single type. * Handle disconnect from all nodes. If the async cluster connection completely disconnects from all nodes in the server, it will try again to connect to the inital nodes that were provided upon creation. This prevents a situation where the client removes connections incrementally, until the connection map is completely empty, and there are no connections to refresh slots on. * Appease Clippy (redis-rs#1061) * Reconnect on parsing errors. Parsing errors mean that the connection received a response it doesn't know how to handle. This means that it cannot make sense of the next values sent over the connection, and the connection must be replaced. related: redis-rs#984 (comment) * Save reconnected connections during retries. This change ensures that reconnect attempts that happen during retries, or new connections that happen after MOVED/ASKING errors, will be saved instead of constantly reconnecting until slots are refreshed. * [fix] make `Pipeline` handle returned bulks correctly * Update mio dependency due to vulnerability. (redis-rs#1064) GHSA-r8w9-5wcg-vfj7 * Simplify Sink polling logic. This removes unnecessary `match`es and `map` from the code, and moves the usage of `poll_recover` into `poll_flush`, so as not to block new requests while trying to recover a connection. * Remove the usage of aio::Connection in tests. `aio::Connection` is deprecated, we should test `aio::MultiplexedConnection` instead. * Fail CI if lock file isn't updated. * Handle server errors in array response. Currently server errors stop the parser and return a RedisError. This caused errors that are returned inside an array, such as transaction errors, to cause the rest of the array to not be parsed. This is solved by adding an internal value type that includes the server errors, so when parsing to the internal value type, the array will finish parsing, and then extracting the error. * Create a server error type. * Separate parsing errors from general response errors. Parsing errors are client-side errors, that are caused by bad output from the server. Response errors are server-side errors, caused by bad input from the user / client. Parse errors cause the client to be in an unrecoverable state. response errors are OK. * Release redis 0.25.0 / redis-test 0.4.0 * Update test version (redis-rs#1071) * Fix ambiguity in examples * Upgrade to socket2 0.5 * Avoid library dependency on futures-time * Merge the `resp3` branch. (redis-rs#1058) * Initial implementation of RESP3 (redis-rs#757) These changes implement all RESP3 types (excluding streamed types). RESP3 can be enabled per connection by adding `?resp3=true` to connection uri. It currently supports PubSub as RESP2 PubSub support in library, but in future PRs it'll support handling normal commands and PubSub messages in one connection. Only `num-bigint` is added as dependency to support `BigNumber` type. Changes made in support of redis-rs#329 and redis-rs#749 * Add RESP3 support to cluster connections. (redis-rs#1001) * Resp3 Push Management (redis-rs#898) * squash! * oops! * test invalidation in cluster && introduce client tracking options * introduce basic PubSub functionality to MultiplexedConnection and make tokio sender unbounded channel * fix tests & linter, make PushSender::Tokio as aio feature only * add resp3 to ci branches and fix cluster client tracking option * test dropping and update ci yml * remove unsubscribe fn and introduce unsubscribing by dropping receiver. * fix tests because RedisJson returns responses in an array when it's resp3 * override redisjson cache (it's a temp solution) * add -skip test_module to RESP3 testing and upgrade redis 6.2.4 to 6.2.13 * test json modules with RESP3 and get json fix from main * in redis v7 RedisJson is different with Resp3 🤔 * Implement Pub/Sub in Async Cluster & fmt & remove usage of is_err_and(stable only after v1.70) * don't use sharded pub/sub with redis v6 * use REDIS_VERSION in env instead of using HELLO command to fetch redis version * oops * fix linter * fix fmt and remove benchmark from CI * simplify PushManager and add tokio as non-optional dependency. * get fixes from 220c6a9 * use --test-threads=1 * override redisjson cache (it's a temp solution) * remove get_push_manager from traits & remove push manager from aio::Connection * remove client_tracking_options * remove 0.21.x from rust.yml * add tests for pushmanager * format & move push_info into a variable * change tests according to comments. * apply 6.2.4 changes && fmt * try to fix * remove con_addr & remove pub/sub support in cluster connections * add disconnection handling to sync, mpx, cm && test it * remove push_manager argument from connection creation * better docs * add has_reply function to PushKind * remove no response command support in mpx since it's not used in mpx pub/sub * apply changes from redis-rs#994 * fix tests * Use enum instead of boolean for RESP version. (redis-rs#1012) Since there's a discussion starting about what might become RESP4, this PR will make it easier to add more RESP versions in the future. * Rename Value enum types in order to match Redis RESP names. (redis-rs#779) * Rename Value::Bulk to Value::Array. * Rename Value::Status to Value::SimpleString. * Rename Value::Data to Value::BulkString. * Fix debug names of values. * fix nightly comments. * reintroduce client tracking to tests. * fix merge errors. --------- Co-authored-by: Altan Özlü <5479094+altanozlu@users.noreply.github.com> * fix flakey test * Fix linter. * fix cargo lock. --------- Co-authored-by: Huxley Hu <framlog@users.noreply.github.com> Co-authored-by: Ahmad <39441506+ahmadbky@users.noreply.github.com> Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com> Co-authored-by: James Lucas <jaymell@users.noreply.github.com> Co-authored-by: framlog <framlog@gmail.com> Co-authored-by: Neo Sun <huachuang20@gmail.com> Co-authored-by: Dirkjan Ochtman <dirkjan@ochtman.nl> Co-authored-by: Altan Özlü <5479094+altanozlu@users.noreply.github.com>
barshaul
pushed a commit
to barshaul/redis-rs
that referenced
this pull request
Jul 11, 2024
* ++redis-rs. * Fix transactions not being routed. Transactions without user routing were routed to a random node, and so almost by default caused a MOVED error and slot refresh. `req_packed_commands` finds the right routing for the transaction. * Fix python exception tests. Longer error messages are shortened by pytest, so we need to use match instead of asserting over the string.
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.
Add retries to both the server and cluster creation process. I believe the core
fix here is just adding a sleep after the server child process is spawned, however
I'm leaving the retries in as I don't think they hurt anything.