Skip to content

Merging from master#1

Merged
dceravigupta merged 46 commits intodceravigupta:masterfrom
StackExchange:master
Apr 24, 2020
Merged

Merging from master#1
dceravigupta merged 46 commits intodceravigupta:masterfrom
StackExchange:master

Conversation

@dceravigupta
Copy link
Copy Markdown
Owner

No description provided.

Nick Craver and others added 30 commits March 12, 2020 20:21
Fixes a break from #1254
### NRediSearch
- Updated module so we're testing the right thing
- More output in the logging path for diagnosing issues
- Remove all the `FLUSHDB` calls masking issues
- Handle duplicate documents in latest module

Note: **locally** (on WSL 2), this fixes some issues and introduces stability but *does not* fix 2 scoring issues. Suggestions are coming back with scores > 1.0, which by the documentation is not supposed to happen...so I'm not sure what should be happening here. Here's the local WSL fail:
```
 NRediSearch.Test.ClientTests.ClientTest.TestGetSuggestionWithScore:
  Message: 
    StackExchange.Redis.RedisCommandException : Missing required fields:  score not within range: 2.11660146713257
  Stack Trace: 
    SuggestionBuilder.Build() line 99
    Client.GetSuggestionsWithPayloadAndScores(RedisResult[] results) line 1271
    Client.GetSuggestions(String prefix, SuggestionOptions options) line 796
    ClientTest.TestGetSuggestionWithScore() line 701
```
```
NRediSearch.Test.ClientTests.ClientTest.TestAddSuggestionGetSuggestionPayloadScores:
  Message: 
    StackExchange.Redis.RedisCommandException : Missing required fields:  score not within range: 2.71068739891052
  Stack Trace: 
    SuggestionBuilder.Build() line 99
    Client.GetSuggestionsWithPayloadAndScores(RedisResult[] results) line 1271
    Client.GetSuggestions(String prefix, SuggestionOptions options) line 796
    ClientTest.TestAddSuggestionGetSuggestionPayloadScores() line 642
```

However, the current fixes tidy up the CI pipeline so I recommend merging this as-is, then I'll keep following up with various local errors and I'm looking at some Docker test options as alternatives to WSL2 so that people can run either.
Since logic exists in the getter, we need to set the field across for actual quality.  The `Issue883_Exhaustive` test illustrates the difference in the comparison (currently failing).
Tidying this up so the diff/fix in #1374 is easier to analyze.
This is 2 of the 6 minutes of test CPU time locally....don't need to hang on this so much.
Clarifies how IPv6 needs to be expressed in example usage.
In going through issues, I see many users confused by `Unspecified/` and it really serves no purpose. Let's get a quick win (and add a test for it).

Also, WSL2 it appears has clock jitter - we can accommodate that while having a valid test, fixing that local case.
Adds support for running all Redis instances and configurations used in the tests inside of docker containers that work on all platforms. Just run `docker-compose up` inside of the `tests/RedisConfigs` folder.
Found while fixing up runs for #1067. Docker layer cache sucks :)
Asked for in #831 and low overhead so why not! I didn't do `RedisChannel` since it already has a constructor available. Tests added to ensure these are used in a way we don't break them.
Co-authored-by: Gunnar Liljas <gunnar.liljas@revide.se>
…on is thrown while trying to send the message or flush the connection. Leaving _activeMessage set causes WriteMessageInsideLock to always return a "NoConnectionAvailable" error indefinitely (#1374)

Co-authored-by: Nick Craver <craver@stackoverflow.com>
* re-implement GetEnumerator in EndPointCollection to be a bit more forgiving to concurrent changes during iteration

* poke release notes
This PR is derived from PR-692 and have been merged with the latest master commit.
Things that have been done:
1. review code for PR-692
2. fixed potential infinite loop in the code
3. Adapt code to success build with the latest master commit
4. Manual testing on 3 Sentinel nodes and 3 Redis nodes (connection and failover)

Usage:
```C#
                ConfigurationOptions sentinelConfig = new ConfigurationOptions();
                sentinelConfig.ServiceName = "mymaster";
                sentinelConfig.EndPoints.Add("192.168.99.102", 26379);
                sentinelConfig.EndPoints.Add("192.168.99.102", 26380);
                sentinelConfig.EndPoints.Add("192.168.99.102", 26381);
                sentinelConfig.TieBreaker = "";
                sentinelConfig.DefaultVersion = new Version(4, 0, 11);                 
                // its important to set the Sentinel commands supported
                sentinelConfig.CommandMap = CommandMap.Sentinel;

                // Get sentinel connection
                ConnectionMultiplexer sentinelConnection = ConnectionMultiplexer.Connect(sentinelConfig, Console.Out);
                // Create master service configuration
                ConfigurationOptions masterConfig = new ConfigurationOptions { ServiceName = "mymaster" };
                // Get master Redis connection
                var redisMasterConnection = sentinelConnection.GetSentinelMasterConnection(masterConfig);

                ...
               IDatabase db = redisMasterConnection.GetDatabase();                
               db.StringSet(key, value);
               ...
               string value1 = db.StringGet(key);

```
This bumps versioning for the 2.1.0 release, ready to GA!
Billing issues have cleared on the org - taking a stab at this.

First attempt: let's get a Linux CI going.
This changes does the following:
- Moves the broadcast to be only before the master reconfiguration to both before *and* after - a fix following 88dcf0c to fix the gap.
- Tweaks tests by overall lessening runtime (and thus build server time). Overall, fixes a few static timeouts to be conditional (so they short circuit faster if met), and brings some tests that were some variant of the above into RELEASE since they're safe now.
- Changes `UntilCondition` to take a `TimeSpan`, just because clearer. Even though I IntelliSense completed `.FromMinutes()` earlier and watched it like an idiot for a while...I stand by this decision!
- Locks the `ITestOutputHelper` writer because...that was jacked up in races:

![off to the races](https://user-images.githubusercontent.com/454813/77232395-2f799980-6b77-11ea-8fb4-0398de25e313.png)

------

Note: a lot of the test changes are just optimizations to delays which allow longer but short-circuit sooner. The important changes are in broadcast and locking around the test runner. I can think of downsides to neither, but want some @mgravell eyes. This should resolve a lot of flaky-ness with local (and build agent) tests. Not all of it, but a lot of it!
Old message:
> To create a disconnected multiplexer, disable AbortOnConnectFail.

New message proposal:
> Error connecting right now. To allow this multiplexer to continue retrying until it's able to connect, use abortConnect=false in your connection string or AbortOnConnectFail=false; in your code.
* ConnectionMultiplexer: track global counts for deebugging

* Build dammit

* Move to per-multiplexer/add tests

Note: I know mutiplexer isn't spelled right - will fix that in a follow-up to avoid noise.

* Fix test key names

Broken since the 2.1 bump, oops

* Simplify the NoConnectionAvailable static

Simpifies usage for all callers. Also shares code and adds diagnostics to the "no connection" case.

* Add tests for NoConnectionException

* Failover: fix tests and debug some

SubscriptionsSurviveMasterSwitchAsync is a thorn in our side - moving to DEBUG.

* Remove bad check

Inner is irrelevant here - can be not-null depending on the connection race.

* Update message and add more tests!

* Bump pipelines to 2.1.6
* fix naming of SentinelGetSentinelAddressesAsync

* tyop

* Sentinel: don't use async over sync with .Result

Co-authored-by: mgravell <marc.gravell@gmail.com>
NickCraver and others added 16 commits March 25, 2020 09:48
We were throwing in event handlers...that's not good. Also post-thread sleep in timer callbacks.
* Including SslProtocols in ConfigurationOptions.ToString()

* Removing an unnecessary method and making the code concise (based on code review comments)

Co-authored-by: Sampath Vuyyuru <sampath.vuyyuru@kincentric.com>
fix build to get green shields for Source Link and Deterministic
Explicitly didn't do `/toys` here because the versions aren't shared but if it makes sense: yeah let's add em. This structure is made for easier migration to https://github.com/NuGet/Home/wiki/Centrally-managing-NuGet-package-versions later (we should just need to remove the manual import in each project file (necessary for ordering):
```xml
<Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Packages.props', '$(MSBuildThisFileDirectory)../'))" />
```

If you're wondering why `Directory.Build.targets` doesn't work - I'm not sure. It's fine for `netcoreapp*` builds, but .NET Full framework throws a fit:
```
C:\git\StackExchange\StackExchange.Redis\tests\NRediSearch.Test\NRediSearch.Test.csproj : error NU1701: Package 'xunit 1.7.0.1540' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8' instead of the project target framework '.NETCoreApp,Version=v2.1'. This package may not be fully compatible with your project. [C:\git\StackExchange\StackExchange.Redis\Build.csproj]
C:\git\StackExchange\StackExchange.Redis\tests\BasicTestBaseline\BasicTestBaseline.csproj : error NU1604: Project dependency BenchmarkDotNet does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results. [C:\git\StackExchange\StackExchange.Redis\Build.csproj]
C:\git\StackExchange\StackExchange.Redis\tests\BasicTest\BasicTest.csproj : error NU1604: Project dependency BenchmarkDotNet does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results. [C:\git\StackExchange\StackExchange.Redis\Build.csproj]
C:\git\StackExchange\StackExchange.Redis\tests\BasicTest\BasicTest.csproj : error NU1701: Package 'BenchmarkDotNet 0.5.0' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8' instead of the project target
framework '.NETCoreApp,Version=v2.0'. This package may not be fully compatible with your project. [C:\git\StackExchange\StackExchange.Redis\Build.csproj]
C:\git\StackExchange\StackExchange.Redis\tests\BasicTestBaseline\BasicTestBaseline.csproj : error NU1701: Package 'BenchmarkDotNet 0.5.0' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8' instead of the project target framework '.NETCoreApp,Version=v2.0'. This package may not be fully compatible with your project. [C:\git\StackExchange\StackExchange.Redis\Build.csproj]
```
...but the approach in this PR is both works and is closer to future plans.
Sentinel servers can be set to require auth like regular servers.
@dceravigupta dceravigupta merged commit 6b51a99 into dceravigupta:master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants