From d75624a771e55fa7ebc857930b9c4815a2e9d979 Mon Sep 17 00:00:00 2001 From: atakavci Date: Thu, 3 Apr 2025 17:48:44 +0300 Subject: [PATCH 1/7] Bump Redis 8.0 version(8.0-RC1-pre) in pipeline --- .github/workflows/integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 12fe2741..6c107919 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -37,7 +37,7 @@ jobs: max-parallel: 15 fail-fast: false matrix: - redis-version: [ '8.0-M05-pre', '${{ needs.redis_version.outputs.CURRENT }}', '7.2.6', '6.2.16'] + redis-version: [ '8.0-RC1-pre', '${{ needs.redis_version.outputs.CURRENT }}', '7.2.6', '6.2.16'] dotnet-version: ['6.0', '7.0', '8.0'] env: ACTIONS_ALLOW_UNSECURE_COMMANDS: true From d5f290d5d79b80c8789ed32f2a10a9da1512ce6e Mon Sep 17 00:00:00 2001 From: atakavci Date: Thu, 3 Apr 2025 18:28:18 +0300 Subject: [PATCH 2/7] drop/fix failing tests with 8.0 --- tests/NRedisStack.Tests/Search/SearchTests.cs | 2 +- .../TimeSeries/TestDataTypes/TestTimeSeriesInformation.cs | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index fc77a3e4..96e2c97f 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -1417,7 +1417,7 @@ public async Task TestDropIndexAsync(string endpointId) } catch (RedisServerException ex) { - Assert.Contains("no such index", ex.Message); + Assert.Contains("no such index", ex.Message, StringComparison.OrdinalIgnoreCase); } Assert.Equal("100", db.Execute("DBSIZE").ToString()); } diff --git a/tests/NRedisStack.Tests/TimeSeries/TestDataTypes/TestTimeSeriesInformation.cs b/tests/NRedisStack.Tests/TimeSeries/TestDataTypes/TestTimeSeriesInformation.cs index 6a8dd707..4f974123 100644 --- a/tests/NRedisStack.Tests/TimeSeries/TestDataTypes/TestTimeSeriesInformation.cs +++ b/tests/NRedisStack.Tests/TimeSeries/TestDataTypes/TestTimeSeriesInformation.cs @@ -27,14 +27,12 @@ public void TestInformationSync() TimeSeriesInformation info = ts.Info(key); TimeSeriesInformation infoDebug = ts.Info(key, debug: true); - Assert.Equal(4184, info.MemoryUsage); Assert.Equal(0, info.RetentionTime); Assert.Equal(1, info.ChunkCount); Assert.Null(info.DuplicatePolicy); Assert.Null(info.KeySelfName); Assert.Null(info.Chunks); - Assert.Equal(4184, infoDebug.MemoryUsage); Assert.Equal(0, infoDebug.RetentionTime); Assert.Equal(1, infoDebug.ChunkCount); Assert.Null(infoDebug.DuplicatePolicy); @@ -55,14 +53,12 @@ public async Task TestInformationAsync() TimeSeriesInformation info = await ts.InfoAsync(key); TimeSeriesInformation infoDebug = await ts.InfoAsync(key, debug: true); - Assert.Equal(4184, info.MemoryUsage); Assert.Equal(0, info.RetentionTime); Assert.Equal(1, info.ChunkCount); Assert.Null(info.DuplicatePolicy); Assert.Null(info.KeySelfName); Assert.Null(info.Chunks); - Assert.Equal(4184, infoDebug.MemoryUsage); Assert.Equal(0, infoDebug.RetentionTime); Assert.Equal(1, infoDebug.ChunkCount); Assert.Null(infoDebug.DuplicatePolicy); From 4a9b95f0bd54a870b81088df84e6a69158ab392a Mon Sep 17 00:00:00 2001 From: atakavci Date: Thu, 3 Apr 2025 18:40:57 +0300 Subject: [PATCH 3/7] fix failing test --- tests/NRedisStack.Tests/Search/SearchTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index 96e2c97f..68a973c2 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -1384,7 +1384,7 @@ public void TestDropIndex(string endpointId) } catch (RedisServerException ex) { - Assert.Contains("no such index", ex.Message); + Assert.Contains("no such index", ex.Message, StringComparison.OrdinalIgnoreCase); } Assert.Equal("100", db.Execute("DBSIZE").ToString()); } From dbd0db03a8896281881dbd95f07fa96de01e899e Mon Sep 17 00:00:00 2001 From: atakavci Date: Tue, 8 Apr 2025 17:12:43 +0300 Subject: [PATCH 4/7] fix flaky test --- tests/NRedisStack.Tests/Search/SearchTests.cs | 63 +++++++++++++++---- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index 68a973c2..86d31bc2 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -8,6 +8,8 @@ using System.Runtime.InteropServices; using NetTopologySuite.IO; using NetTopologySuite.Geometries; +using System.Linq.Expressions; +using System.Collections; namespace NRedisStack.Tests.Search; @@ -3416,6 +3418,13 @@ public void TestNumericLogicalOperatorsInDialect4(string endpointId) Assert.Equal(1, ft.Search(index, new Query("@version==123 @id==456").Dialect(4)).TotalResults); } + /// + /// this test is to check if the issue 352 is fixed + /// Load operation was failing because the document was not being dropped in search result due to this behaviour; + /// "If a relevant key expires while a query is running, an attempt to load the key's value will return a null array. + /// However, the key is still counted in the total number of results." + /// https://redis.io/docs/latest/commands/ft.search/#:~:text=If%20a%20relevant%20key%20expires,the%20total%20number%20of%20results. + /// [Fact] public void TestDocumentLoad_Issue352() { @@ -3423,6 +3432,13 @@ public void TestDocumentLoad_Issue352() Assert.Empty(d.GetProperties().ToList()); } + /// + /// this test is to check if the issue 352 is fixed + /// Load operation was failing because the document was not being dropped in search result due to this behaviour; + /// "If a relevant key expires while a query is running, an attempt to load the key's value will return a null array. + /// However, the key is still counted in the total number of results." + /// https://redis.io/docs/latest/commands/ft.search/#:~:text=If%20a%20relevant%20key%20expires,the%20total%20number%20of%20results. + /// [SkippableTheory] [MemberData(nameof(EndpointsFixture.Env.StandaloneOnly), MemberType = typeof(EndpointsFixture.Env))] public void TestDocumentLoadWithDB_Issue352(string endpointId) @@ -3435,33 +3451,56 @@ public void TestDocumentLoadWithDB_Issue352(string endpointId) Document droppedDocument = null; int numberOfAttempts = 0; + do { - db.HashSet("student:1111", new HashEntry[] { new("first", "Joe"), new("last", "Dod"), new("age", 18) }); - - Assert.True(db.KeyExpire("student:1111", TimeSpan.FromMilliseconds(500))); + // try until succesfully create the key and set the TTL + bool ttlRefreshed = false; + do + { + db.HashSet("student:1111", new HashEntry[] { new("first", "Joe"), new("last", "Dod"), new("age", 18) }); + ttlRefreshed = db.KeyExpire("student:1111", TimeSpan.FromMilliseconds(500)); + } while (!ttlRefreshed); Boolean cancelled = false; - Task searchTask = Task.Run(() => + Action checker = () => { - for (int i = 0; i < 100000; i++) + for (int i = 0; i < 100000 && !cancelled; i++) { SearchResult result = ft.Search(index, new Query()); List docs = result.Documents; - if (docs.Count == 0 || cancelled) + // check if doc is already dropped before search and load; + // if yes then its already late and we missed the window that + // doc would show up in search result with no fields + if (docs.Count == 0) { break; } - else if (docs[0].GetProperties().ToList().Count == 0) + // if we get a document with no fields then we know that the key + // expired while the query is running, and we are able to catch the state + // so we can break the loop + else if (docs[0].GetProperties().Count() == 0) { droppedDocument = docs[0]; + break; } } - }); - Task.WhenAny(searchTask, Task.Delay(1000)).GetAwaiter().GetResult(); - Assert.True(searchTask.IsCompleted); - Assert.Null(searchTask.Exception); + }; + + List tasks = new List(); + // try with 3 different tasks simultaneously to increase the chance of hitting it + for (int i = 0; i < 3; i++) + { + tasks.Add(Task.Run(checker)); + } + Task checkTask = Task.WhenAll(tasks); + Task.WhenAny(checkTask, Task.Delay(1500)).GetAwaiter().GetResult(); + Assert.True(checkTask.IsCompleted); + Assert.Null(checkTask.Exception); cancelled = true; - } while (droppedDocument == null && numberOfAttempts++ < 3); + } while (droppedDocument == null && numberOfAttempts++ < 5); + // we wont do an actual assert here since + // it is not guaranteed that window stays open wide enough to catch it. + // instead we attempt 5 times } } From 98a635ce35ef235ed93ace2ede9ac8ef40ad6728 Mon Sep 17 00:00:00 2001 From: atakavci Date: Tue, 8 Apr 2025 18:01:09 +0300 Subject: [PATCH 5/7] attempt with await --- tests/NRedisStack.Tests/Search/SearchTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index 86d31bc2..bd9bffca 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -3441,7 +3441,7 @@ public void TestDocumentLoad_Issue352() /// [SkippableTheory] [MemberData(nameof(EndpointsFixture.Env.StandaloneOnly), MemberType = typeof(EndpointsFixture.Env))] - public void TestDocumentLoadWithDB_Issue352(string endpointId) + public async void TestDocumentLoadWithDB_Issue352(string endpointId) { IDatabase db = GetCleanDatabase(endpointId); var ft = db.FT(); @@ -3494,7 +3494,7 @@ public void TestDocumentLoadWithDB_Issue352(string endpointId) tasks.Add(Task.Run(checker)); } Task checkTask = Task.WhenAll(tasks); - Task.WhenAny(checkTask, Task.Delay(1500)).GetAwaiter().GetResult(); + await Task.WhenAny(checkTask, Task.Delay(1500)); Assert.True(checkTask.IsCompleted); Assert.Null(checkTask.Exception); cancelled = true; From 0186b02cd8985a97181b5f15e1913debf27bb6cb Mon Sep 17 00:00:00 2001 From: atakavci Date: Tue, 8 Apr 2025 18:18:54 +0300 Subject: [PATCH 6/7] check with completed count --- tests/NRedisStack.Tests/Search/SearchTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index bd9bffca..94cb1ebd 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -8,9 +8,6 @@ using System.Runtime.InteropServices; using NetTopologySuite.IO; using NetTopologySuite.Geometries; -using System.Linq.Expressions; -using System.Collections; - namespace NRedisStack.Tests.Search; @@ -3456,6 +3453,8 @@ public async void TestDocumentLoadWithDB_Issue352(string endpointId) { // try until succesfully create the key and set the TTL bool ttlRefreshed = false; + Int32 completed = 0; + do { db.HashSet("student:1111", new HashEntry[] { new("first", "Joe"), new("last", "Dod"), new("age", 18) }); @@ -3474,6 +3473,7 @@ public async void TestDocumentLoadWithDB_Issue352(string endpointId) // doc would show up in search result with no fields if (docs.Count == 0) { + Interlocked.Increment(ref completed); break; } // if we get a document with no fields then we know that the key @@ -3482,6 +3482,7 @@ public async void TestDocumentLoadWithDB_Issue352(string endpointId) else if (docs[0].GetProperties().Count() == 0) { droppedDocument = docs[0]; + Interlocked.Increment(ref completed); break; } } @@ -3495,8 +3496,7 @@ public async void TestDocumentLoadWithDB_Issue352(string endpointId) } Task checkTask = Task.WhenAll(tasks); await Task.WhenAny(checkTask, Task.Delay(1500)); - Assert.True(checkTask.IsCompleted); - Assert.Null(checkTask.Exception); + Assert.Equal(3, completed); cancelled = true; } while (droppedDocument == null && numberOfAttempts++ < 5); // we wont do an actual assert here since From 4819d4e267ac0975543a52365929c9d986ccbb1d Mon Sep 17 00:00:00 2001 From: atakavci Date: Wed, 9 Apr 2025 09:24:56 +0300 Subject: [PATCH 7/7] - remove cancelled - assert ttl - run till key disappears from search result --- tests/NRedisStack.Tests/Search/SearchTests.cs | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index 94cb1ebd..9c059e2f 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -3443,31 +3443,30 @@ public async void TestDocumentLoadWithDB_Issue352(string endpointId) IDatabase db = GetCleanDatabase(endpointId); var ft = db.FT(); - Schema sc = new Schema().AddTextField("first", 1.0).AddTextField("last", 1.0).AddNumericField("age"); + Schema sc = new Schema().AddTextField("firstText", 1.0).AddTextField("lastText", 1.0).AddNumericField("ageNumeric"); Assert.True(ft.Create(index, FTCreateParams.CreateParams(), sc)); Document droppedDocument = null; int numberOfAttempts = 0; - do { // try until succesfully create the key and set the TTL bool ttlRefreshed = false; - Int32 completed = 0; - do { - db.HashSet("student:1111", new HashEntry[] { new("first", "Joe"), new("last", "Dod"), new("age", 18) }); - ttlRefreshed = db.KeyExpire("student:1111", TimeSpan.FromMilliseconds(500)); + db.HashSet("student:22222", new HashEntry[] { new("firstText", "Joe"), new("lastText", "Dod"), new("ageNumeric", 18) }); + ttlRefreshed = db.KeyExpire("student:22222", TimeSpan.FromMilliseconds(500)); } while (!ttlRefreshed); - Boolean cancelled = false; + Int32 completed = 0; + Action checker = () => { - for (int i = 0; i < 100000 && !cancelled; i++) + for (int i = 0; i < 1000000; i++) { SearchResult result = ft.Search(index, new Query()); List docs = result.Documents; + // check if doc is already dropped before search and load; // if yes then its already late and we missed the window that // doc would show up in search result with no fields @@ -3477,30 +3476,28 @@ public async void TestDocumentLoadWithDB_Issue352(string endpointId) break; } // if we get a document with no fields then we know that the key - // expired while the query is running, and we are able to catch the state - // so we can break the loop + // is going to be expired while the query is running, and we are able to catch the state + // but key itself might not be expired yet else if (docs[0].GetProperties().Count() == 0) { droppedDocument = docs[0]; - Interlocked.Increment(ref completed); - break; } } }; List tasks = new List(); // try with 3 different tasks simultaneously to increase the chance of hitting it - for (int i = 0; i < 3; i++) - { - tasks.Add(Task.Run(checker)); - } + for (int i = 0; i < 3; i++) { tasks.Add(Task.Run(checker)); } Task checkTask = Task.WhenAll(tasks); - await Task.WhenAny(checkTask, Task.Delay(1500)); + await Task.WhenAny(checkTask, Task.Delay(1000)); + var keyTtl = db.KeyTimeToLive("student:22222"); + Assert.Equal(0, keyTtl.HasValue ? keyTtl.Value.Milliseconds : 0); Assert.Equal(3, completed); - cancelled = true; } while (droppedDocument == null && numberOfAttempts++ < 5); - // we wont do an actual assert here since + // we won't do an actual assert here since // it is not guaranteed that window stays open wide enough to catch it. - // instead we attempt 5 times + // instead we attempt 5 times. + // Without fix for Issue352, document load in this case fails %100 with my local test runs,, and %100 success with fixed version. + // The results in pipeline should be the same. } }