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

Add batchSize parameter to Qdrant_VectorStores and QdrantUpsert_Vecto… #2215

Merged
merged 13 commits into from
Apr 29, 2024

Conversation

mrabbah
Copy link
Contributor

@mrabbah mrabbah commented Apr 19, 2024

Resolve the bug related to #2191 and #1824 :
Error: 400 Bad Request: Payload error: JSON payload (xxxxxx bytes) is larger than allowed (limit: 33554432 bytes)

@@ -257,7 +265,10 @@ class Qdrant_VectorStores implements INode {

return res
} else {
await QdrantVectorStore.fromDocuments(finalDocs, embeddings, dbConfig)
for(let i=0; i<finalDocs.length; i+=batchSize) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if no batchSize is found, can we just do it without for loop?

I tried upserting 1k+ chunks, it works fine:
image

Instead of doing this 10 times in for loop, I think its better for other to specify the batch size once they faced the error, if everything works fine for the first try, then thats great

name: 'batchSize',
type: 'number',
default: 100,
additionalParams: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have an optional: true

@xmaiconx
Copy link

This is a very important fix..

… and QdrantUpsert_VectorStores also try calling normal mode if it fail failback to batch mode
@mrabbah
Copy link
Contributor Author

mrabbah commented Apr 27, 2024

In my last commit the batchSize is not required, and I try to POST all at once first and if it fail I fall back to batch mode

Copy link
Contributor

@HenryHengZJ HenryHengZJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks guys for the PR, merging!

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.

None yet

3 participants