Skip to content

Fix RedisVectorStore not closing Jedis pipelines #423

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Fix RedisVectorStore not closing Jedis pipelines #423

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 31 additions & 29 deletions ...es/spring-ai-redis/src/main/java/org/springframework/ai/vectorstore/RedisVectorStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,25 +303,26 @@ public JedisPooled getJedis() {

@Override
public void add(List<Document> documents) {
Pipeline pipeline = this.jedis.pipelined();
for (Document document : documents) {
var embedding = this.embeddingClient.embed(document);
document.setEmbedding(embedding);

var fields = new HashMap<String, Object>();
fields.put(this.config.embeddingFieldName, embedding);
fields.put(this.config.contentFieldName, document.getContent());
fields.putAll(document.getMetadata());
pipeline.jsonSetWithEscape(key(document.getId()), JSON_SET_PATH, fields);
}
List<Object> responses = pipeline.syncAndReturnAll();
Optional<Object> errResponse = responses.stream().filter(Predicate.not(RESPONSE_OK)).findAny();
if (errResponse.isPresent()) {
String message = MessageFormat.format("Could not add document: {0}", errResponse.get());
if (logger.isErrorEnabled()) {
logger.error(message);
try (Pipeline pipeline = this.jedis.pipelined()) {
for (Document document : documents) {
var embedding = this.embeddingClient.embed(document);
document.setEmbedding(embedding);

var fields = new HashMap<String, Object>();
fields.put(this.config.embeddingFieldName, embedding);
fields.put(this.config.contentFieldName, document.getContent());
fields.putAll(document.getMetadata());
pipeline.jsonSetWithEscape(key(document.getId()), JSON_SET_PATH, fields);
}
List<Object> responses = pipeline.syncAndReturnAll();
Optional<Object> errResponse = responses.stream().filter(Predicate.not(RESPONSE_OK)).findAny();
if (errResponse.isPresent()) {
String message = MessageFormat.format("Could not add document: {0}", errResponse.get());
if (logger.isErrorEnabled()) {
logger.error(message);
}
throw new RuntimeException(message);
}
throw new RuntimeException(message);
}
}

Expand All @@ -331,19 +332,20 @@ private String key(String id) {

@Override
public Optional<Boolean> delete(List<String> idList) {
Pipeline pipeline = this.jedis.pipelined();
for (String id : idList) {
pipeline.jsonDel(key(id));
}
List<Object> responses = pipeline.syncAndReturnAll();
Optional<Object> errResponse = responses.stream().filter(Predicate.not(RESPONSE_DEL_OK)).findAny();
if (errResponse.isPresent()) {
if (logger.isErrorEnabled()) {
logger.error("Could not delete document: {}", errResponse.get());
try (Pipeline pipeline = this.jedis.pipelined()) {
for (String id : idList) {
pipeline.jsonDel(key(id));
}
List<Object> responses = pipeline.syncAndReturnAll();
Optional<Object> errResponse = responses.stream().filter(Predicate.not(RESPONSE_DEL_OK)).findAny();
if (errResponse.isPresent()) {
if (logger.isErrorEnabled()) {
logger.error("Could not delete document: {}", errResponse.get());
}
return Optional.of(false);
}
return Optional.of(false);
return Optional.of(true);
}
return Optional.of(true);
}

@Override
Expand Down