-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add concurrent HashMap for storing indexNames and connection objects #92
Conversation
FWIW, I was also the one that suggested you rename them from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, added a few comments.
At some point we'll need to add clear guidance on the proper (safe) usage especially in a multi-threaded context.
src/integration/java/io/pinecone/integration/dataPlane/QueryErrorTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some Qs just for my understanding and nits
Problem
Everytime createIndexConnection() or createAsyncConnection() is called, we are internally calling describeIndex() to get index host which is required for creating blocking or async stubs.
Solution
Create a singleton ConcurrentHashMap of indexName (key) and PineconeConnection (value) such that every time the PineconeConnection object is instantiated, the indexName along with the corresponding PineconeConnection is stored into the HashMap. The entry is removed when the connection object is closed.
Also for readability reasons, @ssmith-pc suggested to update the
createIndexConnection()
andcreateAsyncIndexConnection()
methods togetIndexConnection()
andgetAsyncConnection()
methods.Type of Change
Test Plan
Added unit tests and ran integration tests.