Pull from upstream#1
Open
deepakverma wants to merge 467 commits into
Open
Conversation
…onnected, even when no completion is needed, to be able to dequeue and complete other timed out messages. (#2397) When the client is not connected timed out fire and forget messages currently are not removed from the backlog that also results in subsequent timed out messages not being marked as timed out, as described in #2392.
* add passing (modified) test for #2418 * - clarify the meaning of RedisValue.IsInteger, and reduce the visibility - fix a typo * fix PR number
…fixes for outstanding scenarios (#2408) This combination PR is both fixing a GC issue (see below, and #2413 for details) and improves timeout exception. Basically if a timeout happens for a message that was in the backlog but was never sent, the user now gets a much more informative message like this: > Exception: The message timed out in the backlog attempting to send because no connection became available - Last Connection Exception: InternalFailure on 127.0.0.1:6379/Interactive, Initializing/NotStarted, last: GET, origin: ConnectedAsync, outstanding: 0, last-read: 0s ago, last-write: 0s ago, keep-alive: 500s, state: Connecting, mgr: 10 of 10 available, last-heartbeat: never, last-mbeat: 0s ago, global: 0s ago, v: 2.6.99.22667, command=PING, inst: 0, qu: 0, qs: 0, aw: False, bw: CheckingForTimeout, last-in: 0, cur-in: 0, sync-ops: 1, async-ops: 1, serverEndpoint: 127.0.0.1:6379, conn-sec: n/a, aoc: 0, mc: 1/1/0, mgr: 10 of 10 available, clientName: NAMISTOU-3(SE.Redis-v2.6.99.22667), IOCP: (Busy=0,Free=1000,Min=32,Max=1000), WORKER: (Busy=2,Free=32765,Min=32,Max=32767), POOL: (Threads=18,QueuedItems=0,CompletedItems=65), v: 2.6.99.22667 (Please take a look at this article for some common client-side issues that can cause timeouts: https://stackexchange.github.io/StackExchange.Redis/Timeouts) Today, this isn't intuitive especially for connections with `AbortOnConnectFail` set to `false`. What happens is a multiplexer _never_ connects successfully, but the user just gets generic timeouts. This makes the error more specific and includes the inner exception (also as `.InnerException`) for more details, informing the user of a config/auth/whatever error underneath as to why things are never successfully sending. Also adds `aoc: (0|1)` to the exception message for easier advice in issues (reflecting what `AbortOnConnectFail` is set to). Co-authored-by: Nick Craver <nrcraver@gmail.com> Co-authored-by: Marc Gravell <marc.gravell@gmail.com>
For wrappers which intend to provide this (e.g. managed service accounts and such), the intent was for them to override these and return their "current" values (e.g. as a token rotates), but I missed them in the initial pass...even though this was the original extensibility reason, because I suck! Fixing.
- DefaultOptionsProvider.TryGetAzureRoleInstanceIdNoThrow use Type.GetType to check for Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment, so the trimmer knows to preserve this type, if it is in the app - ChannelMessage use the added public API for ChannelReader CanCount and Count, but fallback to reflection on netcoreapp3.1, since those APIs are not available on that runtime. Contributes to #2449 This at least removes the warnings from using `Microsoft.Extensions.Caching.StackExchangeRedis`. We could also add a trimming test app as outlined in https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming#show-all-warnings-with-sample-application, along with a unit test that publishes the app and ensures there are no new warnings. LMK if you think this is valuable. There are still these warnings left in this library: ``` /_/src/StackExchange.Redis/ScriptParameterMapper.cs(173): Trim analysis warning IL2070: StackExchange.Redis.ScriptParameterMapper.IsValidParameterHash(Type,LuaScript,String&,String&): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicNestedTypes', 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.PublicEvents' in call to 'System.Type.GetMember(String)'. The parameter 't' of method 'StackExchange.Redis.ScriptParameterMapper.IsValidParameterHash(Type,LuaScript,String&,String&)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj] /_/src/StackExchange.Redis/ScriptParameterMapper.cs(227): Trim analysis warning IL2070: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicNestedTypes', 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.PublicEvents' in call to 'System.Type.GetMember(String)'. The parameter 't' of method 'StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj] /_/src/StackExchange.Redis/ScriptParameterMapper.cs(260): Trim analysis warning IL2026: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): Using member 'System.Linq.Expressions.Expression.Property(Expression,String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj] /_/src/StackExchange.Redis/ScriptParameterMapper.cs(261): Trim analysis warning IL2026: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): Using member 'System.Linq.Expressions.Expression.Call(Expression,String,Type[],Expression[])' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj] ``` Fixing those will require a bit more work, as the whole LuaScript functionality looks like it needs to be marked `RequiresUnreferencedCode`. cc @mgravell @NickCraver
According to https://datatracker.ietf.org/doc/html/draft-luotonen-web-proxy-tunneling-01#section-3.2, the expected response to a `CONNECT` should be `HTTP/1.1 200 Connection established` instead of `HTTP/1.1 200 OK`. Allow both responses for now (as still a lot of proxy implementations fail to follow the standard). A HTTP(S), SOCKS, etc. proxy is traditionally specified by an URI of form `scheme://[user:pass@]host[:port]`. Accept both formats for backwards compatibility.
One regression I just noticed in the exception message tweaks we did was we no longer readily include the timeout in the main message...but we should. Fixing messages and tests here. Before: > Exception: The message timed out in the backlog attempting to send because no connection became available - Last Connection Exception: InternalFailure on 127.0.0.1:6379/Interactive, Initializing/NotStarted, last: GET, origin: ConnectedAsync, outstanding: 0, last-read: 0s ago, last-write: 0s ago, keep-alive: 500s, state: Connecting, mgr: 10 of 10 available, last-heartbeat: never, last-mbeat: 0s ago, global: 0s ago, v: 2.6.99.22667, command=PING, inst: 0, qu: 0, qs: 0, aw: False, bw: CheckingForTimeout, last-in: 0, cur-in: 0, sync-ops: 1, async-ops: 1, serverEndpoint: 127.0.0.1:6379, conn-sec: n/a, aoc: 0, mc: 1/1/0, mgr: 10 of 10 available, clientName: NAMISTOU-3(SE.Redis-v2.6.99.22667), IOCP: (Busy=0,Free=1000,Min=32,Max=1000), WORKER: (Busy=2,Free=32765,Min=32,Max=32767), POOL: (Threads=18,QueuedItems=0,CompletedItems=65), v: 2.6.99.22667 (Please take a look at this article for some common client-side issues that can cause timeouts: https://stackexchange.github.io/StackExchange.Redis/Timeouts) After: > Exception: The message timed out in the backlog attempting to send because no connection became available **(100ms)** - Last Connection Exception: InternalFailure on 127.0.0.1:6379/Interactive, Initializing/NotStarted, last: GET, origin: ConnectedAsync, outstanding: 0, last-read: 0s ago, last-write: 0s ago, keep-alive: 500s, state: Connecting, mgr: 10 of 10 available, last-heartbeat: never, last-mbeat: 0s ago, global: 0s ago, v: 2.6.99.22667, command=PING, inst: 0, qu: 0, qs: 0, aw: False, bw: CheckingForTimeout, last-in: 0, cur-in: 0, sync-ops: 1, async-ops: 1, serverEndpoint: 127.0.0.1:6379, conn-sec: n/a, aoc: 0, mc: 1/1/0, mgr: 10 of 10 available, clientName: NAMISTOU-3(SE.Redis-v2.6.99.22667), IOCP: (Busy=0,Free=1000,Min=32,Max=1000), WORKER: (Busy=2,Free=32765,Min=32,Max=32767), POOL: (Threads=18,QueuedItems=0,CompletedItems=65), v: 2.6.99.22667 (Please take a look at this article for some common client-side issues that can cause timeouts: https://stackexchange.github.io/StackExchange.Redis/Timeouts)
If a wrapper package is generally in use in a deployment, it may want to override what we set as the library name in `CLIENT SETINFO lib-name <name>`. This allows doing so via the `DefaultOptionsProvider` (intentionally not on `ConfigurationOptions` directly as version isn't either). Note that this does NOT upgrade the test suite to 7.2.0 RC1. I did test and this works, but there are other breaks we need to evaluate - I'll open another PR separately to demonstrate.
This version contains fixes for supporting NativeAOT. Contributes to #2449
* add RedisChannel UseImplicitAutoPattern and IsPatternBased * mark the implicit RedisChannel operators as [Obsolete] (#2481)
(also fix some remaining debug tests)
I'm seeing a few issues here that we need to resolve for the upcoming Redis 7.2 release: - [x] The internal encoding has changed 1 spot to listpack, which is correct from redis/redis#11303, but is missing in the docs at https://redis.io/commands/object-encoding/ (fixed in tests themselves) - [x] The `HackyGetPerf` reliably returns 0 now, regardless of how long has passed (e.g. upping iterations tremendously)...this may legit be bugged. - [x] `StreamAutoClaim_IncludesDeletedMessageId` expectations are broken, not sure what to make of this yet but it's an odd change to hit between 7.0 and 7.2 versions. Note: no release notes because these are all test tweaks. Co-authored-by: slorello89 <steve.lorello@redis.com>
…#2494) RegisterProfiler allows for a null ProfilingSession to be returned - it is even documented with "or returning null to not profile". Fixing the nullable annotation to indicate that null is an acceptable result of the profilingSessionProvider function.
This allows for using new APIs introduced in net6.0. In order to enable building for net6.0, we also need to revert the thread pool changes in #1939 and #1950. This was already effectively reverted in #1992 by not building for net6.0. Now that we are building for net6.0 again, these if-defs need to be removed.
We're seeing some instances of quite delayed timeouts and at least in two deeper investigations it was due to a huge number of timers firing and the backlog timeout assessment timer not triggering for an extended period of time. To help users diagnose this, adding the cheap counter to the .NET Core pool stats where `Timer.ActiveCount` is readily available. This is available in all the .NET (non-Framework) versions we support. Before: > ...POOL: (Threads=25,QueuedItems=0,CompletedItems=1066) After: > ...POOL: (Threads=25,QueuedItems=0,CompletedItems=1066,Timers=46) See #2477 for another possible instance of this.
* add LibraryName at ConfigurationOptions level * add PR number * add GetProvider * remove `libname` config-string; move docs to code-only section * remove s_DefaultProvider * Update docs/ReleaseNotes.md --------- Co-authored-by: Nick Craver <nrcraver@gmail.com>
* release notes * mention connection-id tracking
Fixing the current tests stalling PRs. Issue2507 fundamentally can't play nice either others, especially shared connections and simultaneous, so fixing that case, and adding `SetEmpty`) which has come up a few times as an example of this working.
This is a test-only change. I don't want to risk an upgrade and harvesting PII from anyone who works on our project, so I'm removing Moq immediately. See https://github.com/moq/moq/issues/1372 for details/discussion.
ILogger support - since we support `Action<ConfigurationOptions>` this can be added today e.g. with `o => o.LoggerFactory = myLoggerFactory`:
```cs
var muxer = ConnectionMultiplexer.Connect("localhost", o => o.LoggerFactory = myLoggerFactory);
```
...or we could add sibling overloads to the existing `TextWriter`. I'd love to not create a ^2 matrix every time we do this, though. Starting simpler for that reason. Note: this is on top of #2050 for simplicity, will roll against `main` when merged.
- satisfy infosec scanner that we're not risking pointer weirdness (although in reality, this is always UTF8; can't be hijacked) - prefer EncodingExtensions.GetString when it is available - switch payload to an explicit field so we can use ^^^ via pass-by-ref There is no actual infosec here; if the inbuilt UTF8 implementation is broken, we're already hosed; there is no extension point to replace UTF8
Setting up CodeQL scanning on the repo to see where it's at, and hopefully leave enabled as a safeguard.
* failing tests for #2783 - `HELLO 3` with `3` response: pass - `HELLO 3` with `-ERR` response: fail - `HELLO 3` with `2` response: fail * more test permutations * don't run unnecessary RESP2-client permutations * fix RESP3 handshakes * words * words
#3043) * Filter out 'temporary placeholder' clsuter nodes with handshake flag that redis creates internally during cluster handshake (during initial cluster MEET, cluster MEET, cluster PONG sequence) since they are not expected to be usable members of the cluster at that time. * - move the "ignore handshake nodes" logic deeper into the client, so external callers can still fetch the value - add new IgnoreFromClient internal member for ^^^, and use - support "handshake" etc in the in-proc test server - support "only initially connect to default node" in the in-proc test server - add a unit test that handshake nodes are ignored * prove that ClusterNodesAsync still reports the node * split out the two "handshake" test parts * in-proc test server; make DefaultEndPoint reliable * release notes * put IsHandshake onto public API --------- Co-authored-by: Marc Gravell <marc.gravell@gmail.com>
* - propose new API to resolve product variant; only handles Redis, Valkey and Garnet at the moment * - change the test-server to support multi-DB operations - update the client to use the product-variant info to allow multi-DB operations on Valkey - unit test with the toy-server * naming is hard * release notes
* Implement GRCA (redis/redis#14826) * Flag [SER006]; remove [SER001] * release notes * Update ReleaseNotes.md
…2965) * Allow heartbeat to restart the pipe thread with only sync commands There is a thread looping in the method PhysicalConnection.ReadFromPipe to process response from Redis, match them with the sent command and signaling the completion of the message. If this thread has an exception, its catch block will call RecordConnectionFailed which will proceed to restart a new thread to continue reading Redis responses. However, if another exception occurred in the catch before the new thread can be started (in a case of high memory pressure, OOM exceptions can happen anywhere) we are in a state where no one is reading the pipe of Redis responses, and all commands sent end in timeout. If at least one async command is sent, the heartbeat thread will detect the timeout in the OnBridgeHeartbeat method, and if no read were perform for 4 heartbeat it will issue a connection failure. With this commit, this becomes true for sync commands as well. Therefore, it ensures we will not reach a state were all commands end in timeout. * Compare count of sync commands timeouted with sync timeout --------- Co-authored-by: Francois ROBION <francois.robion@esker.com>
* interpreting server_mode field (to support Valkey 8+ cluster) * add valkey server_mode unit test * release notes --------- Co-authored-by: mgravell <marc.gravell@gmail.com>
* run-tests composite action for ubuntu tests, allow custom image runs * Allow using custom image in CI workflow dispatch * Return checkout to be able to run the action * Updgrade dorny/test-reporter to v3
* GCRA: reflect name/protocol change from redis/redis#14950 (note: need a new CI build before we can validate this) * PR number * CI version * more CI version
* propose XNACK for 8.8 * PR number
* Propose ZUNION ... AGGREATE COUNT for 8.8 * remove "consumer" from XNACK, and fix FATAL overflow; CI didn't catch these because the server version was misleading (fixed in this PR)
* Recognize Azure Managed Redis in new clouds
* Support ZADD INCR; fix #3069 * PR number in release note * use high bits of enum for CH/INCR * LoggerTests: synchronize over the SB (CI race condition, rare) * CI stability: `VectorSetAdd_WithEverything` - disambiguate test key * VADD: clarify where CI is failing * CI: stabilize GC test * avoid m02 * more tools to inlvestigate weird VADD break on CI * unused directive * unused extern * more VADD investigation * move FP32 logic to the request level * giving up for now; logging separately * note ticket number
* Upgrade version of some packages needing system.memory, so they use the same vesion (4:0:1:2) than System.Hashing.IO System.Hashing.IO can not be downgraded for old .net because first version with XxHash3 use system.memory 4:0:1:2 * Fix implementation of interface that now match modern syntax
* inital stab at subkey notifications * clean up API * docs, simplify usage * subkeys API * avoid a fixed stackalloc * CI file * tyop * integration tests for subkey notifications (something channel-routing related still failing) * fix channel-prefix scenarios * fix unit test (bad payload) * fix default-span "fixed" scenario; we could pre-check the length, but let's just fix the underlying problem * fix cluster routing of subkey notifications; assert newline logic; use deterministic tests (estimate was causing flakiness) * ignore codex files * experimental efficiency API * cleanup
* reinstate failing test for investigation * fix CI
* limit keyspace events to AKE until 8.8 lands * improve sln layout * yank GCRA * prefer slnx and re-apply folder fixes * release notes
* AMR: use RESP3 and disable the default config channel by default * release notes * support TLS (self-signed) in the test server, use for the AMR test * don't do the AMR post-connection callback * fix test break in Issue883 due to lazy provider lookup * improved release notes * verify config override works as expected * spin up pub/sub on demand (or if pub/sub is active) * build warning cleanup * logging * breakfix * format * AMR test for RESP2
* update CI to 8.8 rc1 * and the dockerfile
* draft support for 8.8 arrays * - mark API as experimental - add keyspace notification tests - tidying - docs * clarify how last-items interacts with ring buffers * fix CI netfx compilation * use ValuePairInterleavedProcessorBase to ensure that jagged vs flat doesn't impact us (RESP2 vs RESP3) * make life even easier * add Array to signature prefix list * stabilize hotkeys CI * fix last-minute ARGREP result change
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
No description provided.