Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis Connector failing when initialized as in sample #1404

Closed
HSven1611 opened this issue Jun 10, 2023 · 1 comment · Fixed by #1408
Closed

Redis Connector failing when initialized as in sample #1404

HSven1611 opened this issue Jun 10, 2023 · 1 comment · Fixed by #1408

Comments

@HSven1611
Copy link

Describe the bug
When using the newly introduces redis connector as described in the quickstart it crashes when accessing the memory.

This is just a documentation issue as far as I see it.

To Reproduce
Steps to reproduce the behavior:

  1. initialize redis connector as in sample
  2. try to access memory

Exception:
Microsoft.Bot.Builder.Integration.AspNet.Core.IBotFrameworkHttpAdapter[0]
[OnTurnError] unhandled error : Cannot access a disposed object.
Object name: 'DELL5B70FK3(SE.Redis-v2.6.96.30123)'.
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'DELL5B70FK3(SE.Redis-v2.6.96.30123)'.
at StackExchange.Redis.ConnectionMultiplexer.ExecuteAsyncImpl[T](Message message, ResultProcessor`1 processor, Object
[...]

I think it eventually disposes something the redis memory connector needs to work. In my application there is some time between initialization and usage as I'm waiting for some user input.

Fix
remove using before Connection multiplexer.

old:

using ConnectionMultiplexer connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync("localhost:6379");

new:

ConnectionMultiplexer connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync("localhost:6379");

Expected behavior
Should not crash and access the memory.

Desktop (please complete the following information):

  • OS: Windows
  • IDE: VS2022
  • NuGet Package: Microsoft.SemanticKernel.Connectors.Memory.Redis
  • NuGet Package version: 0.15.230609.2-preview

Additional context
Sample code in quick Start 2. seems broken
Redis Connector quickstart

@JadynWong
Copy link
Contributor

JadynWong commented Jun 11, 2023

The document simply demonstrates how to use it quickly, similar to Example47_Redis.cs. In the application, ConnectionMultiplexer should typically be a singleton, and as a best practice, it should be disposed once done using. Additionally, when writing this README.md, I instinctively believe that users should first understand how to use ConnectionMultiplexer (StackExchange.Redis).

public static class Example47_Redis
{
private const string MemoryCollectionName = "redis-test";
public static async Task RunAsync()
{
string configuration = Env.Var("REDIS_CONFIGURATION");
using ConnectionMultiplexer connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync(configuration);
IDatabase database = connectionMultiplexer.GetDatabase();
RedisMemoryStore memoryStore = new(database, vectorSize: 1536);
IKernel kernel = Kernel.Builder
.WithLogger(ConsoleLogger.Log)
.WithOpenAITextCompletionService("text-davinci-003", Env.Var("OPENAI_API_KEY"))
.WithOpenAITextEmbeddingGenerationService("text-embedding-ada-002", Env.Var("OPENAI_API_KEY"))
.WithMemoryStorage(memoryStore)
.Build();
Console.WriteLine("== Printing Collections in DB ==");
var collections = memoryStore.GetCollectionsAsync();
await foreach (var collection in collections)
{
Console.WriteLine(collection);
}
Console.WriteLine("== Adding Memories ==");
var key1 = await kernel.Memory.SaveInformationAsync(MemoryCollectionName, id: "cat1", text: "british short hair");
var key2 = await kernel.Memory.SaveInformationAsync(MemoryCollectionName, id: "cat2", text: "orange tabby");
var key3 = await kernel.Memory.SaveInformationAsync(MemoryCollectionName, id: "cat3", text: "norwegian forest cat");
Console.WriteLine("== Printing Collections in DB ==");
collections = memoryStore.GetCollectionsAsync();
await foreach (var collection in collections)
{
Console.WriteLine(collection);
}
Console.WriteLine("== Retrieving Memories Through the Kernel ==");
MemoryQueryResult? lookup = await kernel.Memory.GetAsync(MemoryCollectionName, "cat1");
Console.WriteLine(lookup != null ? lookup.Metadata.Text : "ERROR: memory not found");
Console.WriteLine("== Retrieving Memories Directly From the Store ==");
var memory1 = await memoryStore.GetAsync(MemoryCollectionName, key1);
var memory2 = await memoryStore.GetAsync(MemoryCollectionName, key2);
var memory3 = await memoryStore.GetAsync(MemoryCollectionName, key3);
Console.WriteLine(memory1 != null ? memory1.Metadata.Text : "ERROR: memory not found");
Console.WriteLine(memory2 != null ? memory2.Metadata.Text : "ERROR: memory not found");
Console.WriteLine(memory3 != null ? memory3.Metadata.Text : "ERROR: memory not found");
Console.WriteLine("== Similarity Searching Memories: My favorite color is orange ==");
var searchResults = kernel.Memory.SearchAsync(MemoryCollectionName, "My favorite color is orange", limit: 3, minRelevanceScore: 0.8);
await foreach (var item in searchResults)
{
Console.WriteLine(item.Metadata.Text + " : " + item.Relevance);
}
Console.WriteLine("== Removing Collection {0} ==", MemoryCollectionName);
await memoryStore.DeleteCollectionAsync(MemoryCollectionName);
Console.WriteLine("== Printing Collections in DB ==");
await foreach (var collection in collections)
{
Console.WriteLine(collection);
}
}

lemillermicrosoft pushed a commit that referenced this issue Jun 12, 2023
### Motivation and Context
Fix #1404

### Description
Remove `using` from the code example and add some comments to
`ConnectionMultiplexer` to prevent exceptions due to some accidental
usage.
shawncal pushed a commit to shawncal/semantic-kernel that referenced this issue Jul 6, 2023
### Motivation and Context
Fix microsoft#1404

### Description
Remove `using` from the code example and add some comments to
`ConnectionMultiplexer` to prevent exceptions due to some accidental
usage.
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 a pull request may close this issue.

2 participants