[Bug] getIndexConnection is not thread safe

[borowis]

Is this a new bug?
In other words: Is this an error, flaw, failure or fault? Please search Github issues and check our Community Forum to see if someone has already reported the bug you encountered.

If this is a request for help or troubleshooting code in your own Pinecone project, please join the Pinecone Community Forum.

• I believe this is a new bug
• I have searched the existing Github issues and Community Forum, and I could not find an existing post for this bug

Describe the bug
Right now the code in Java Pinecone SDK is not thread safe, for example getIndexConnection(String indexName) mutates config in place, that means that if we call getIndexConnection with 2 (or more) different indices at the same time we might end up in a situation where connection is cached for indexName A while in reality host points to index B. To make it a bit more clear:

public Index getIndexConnection(String indexName) throws PineconeValidationException {
        if(indexName == null || indexName.isEmpty()) {
            throw new PineconeValidationException("Index name cannot be null or empty");
        }

A:    config.setHost(getIndexHost(indexName));
        PineconeConnection connection = getConnection(indexName);


inside getConnection:
    PineconeConnection getConnection(String indexName) {
        return connectionsMap.computeIfAbsent(indexName, key -> new PineconeConnection(config));
    }

    public PineconeConnection(PineconeConfig config) {
B:      this.config = config;

So in A we mutate config and in B we use that config to establish a connection, if A and B run concurrently in 2 getIndexConnection there's no guarantee that in B config would be the same as it was in A 😅

Error information
We have seen on production a transient issue where records were not found in index A because it was actually using an index B to look them up! This resulted in our app returning some 500 errors to users, no errors in Pinecone client itself, that was just an index connection issue.

Steps to reproduce the issue locally
I didn't write a test to reproduce it, sorry, but I think it should be relatively straightforward, concurrently establish connections to indices A and B and verify underlying Pinecone connection netty channel endpoint is not what expected (might require some reflection as those fields are private in SDK)

Environment

• OS Version: Ubuntu Linux
• Java version: 21
• Java SDK version:

Additional context
I believe this is rather an edge case however I wanted to log an issue to make people aware it could be happening when concurrently using the same client to talk to 2 different indices

[nicoloboschi]

I ran into these exact problem. It's not clear at all that a Pinecone client is bound to a specific index.

[rohanshah18]

@borowis Thank you for reporting the issue, I am able to reproduce it by:

1. Added print statement in getConnection() of Pinecone class to print the connection map after a connection is added to the map.
2. Added toString() in PineconeConnection class to print config.getHost().
3. Established connections to index-1 and index-2 using same pinecone object consecutively to find that in step#2, the value for index-1 is being overwritten to reflect index-2's connection.

I have actively started working on this issue and hoping to get back soon with a fix.

[rohanshah18]

@borowis @nicoloboschi I have released Java SDK v4.0.1 that fixes the multithreading issue for getIndexConnection() and getAsycIndexConnection(). Please let me know if the latest SDK version resolves the problem on your end.

[rohanshah18]

Closing this issue since the fix was released before 2 weeks.