Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ private long getSeq(final IndexWriter writer, final String key, final long defau
return Long.parseLong(entry.getValue());
}
}
return 0L;
return defaultValue;
}

private void close(final String name, final IndexHolder holder) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.couchdb.nouveau.api.DocumentDeleteRequest;
Expand Down Expand Up @@ -107,6 +108,7 @@ public class LuceneIndex extends Index {
private final Analyzer analyzer;
private final IndexWriter writer;
private final SearcherManager searcherManager;
private final AtomicBoolean searcherManagerStale = new AtomicBoolean(false);
private final LuceneIndexSchema schema;

public LuceneIndex(
Expand Down Expand Up @@ -147,12 +149,13 @@ public void doUpdate(final String docId, final DocumentUpdateRequest request) th
final Document doc = toDocument(docId, request);
schema.update(request.fields());
writer.updateDocument(docIdTerm, doc);
searcherManagerStale.set(true);
}

@Override
public void doDelete(final String docId, final DocumentDeleteRequest request) throws IOException {
final Query query = docIdQuery(docId);
writer.deleteDocuments(query);
writer.deleteDocuments(docIdTerm(docId));
searcherManagerStale.set(true);
}

@Override
Expand Down Expand Up @@ -209,7 +212,9 @@ public SearchResults doSearch(final SearchRequest request) throws IOException {
cm = new MultiCollectorManager(hits);
}

searcherManager.maybeRefreshBlocking();
if (searcherManagerStale.getAndSet(false)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems racy if say after an update? A first search call triggers getAndSet(false) but then a concurrent, second search won't block anymore even if the first one hadn't finished updating?

searcherManager.maybeRefreshBlocking();
}

final IndexSearcher searcher = searcherManager.acquire();
try {
Expand Down Expand Up @@ -514,10 +519,6 @@ private static byte[] toBytes(final BytesRef bytesRef) {
return Arrays.copyOfRange(bytesRef.bytes, bytesRef.offset, bytesRef.offset + bytesRef.length);
}

private static Query docIdQuery(final String docId) {
return new TermQuery(docIdTerm(docId));
}

private static Term docIdTerm(final String docId) {
return new Term("_id", docId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ private static Type fromField(final Field field) {

private final ConcurrentMap<String, Type> map;

private final ConcurrentMap<Locale, Map<String, PointsConfig>> pointsConfigCache = new ConcurrentHashMap<>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is caching a locale => number format instances to share between client calls?

Since it seem to be shared I checked https://lucene.apache.org/core/10_5_0/queryparser/org/apache/lucene/queryparser/flexible/standard/config/PointsConfig.html and it doesn't mention any synchronization safety but the number format warns about using them in different threads https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/text/NumberFormat.html#synchronization maybe it need a lock or if you looked and think it's fine it's ok. I am just going by superficial impressions and docs


private LuceneIndexSchema(Map<String, Type> map) {
this.map = new ConcurrentHashMap<>(map);
this.map.put("_id", Type.STRING);
Expand All @@ -78,7 +80,11 @@ public static LuceneIndexSchema fromString(final String schemaStr) {
public void update(final Collection<Field> fields) {
Objects.requireNonNull(fields);
for (var field : fields) {
map.putIfAbsent(field.name(), Type.fromField(field));
var type = Type.fromField(field);
var result = map.putIfAbsent(field.name(), type);
if (result == null && type == Type.DOUBLE) {
pointsConfigCache.clear();
}
assertType(field);
}
}
Expand All @@ -102,6 +108,10 @@ public void assertType(final Field field) {
}

public Map<String, PointsConfig> toPointsConfigMap(final Locale locale) {
return pointsConfigCache.computeIfAbsent(locale, l -> buildPointsConfigMap(l));
}

private Map<String, PointsConfig> buildPointsConfigMap(final Locale locale) {
Objects.requireNonNull(locale);
var numberFormat = NumberFormat.getInstance(locale);
var doublePointsConfig = new PointsConfig(numberFormat, Double.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ public final class AnalyzeResource {

@POST
public AnalyzeResponse analyzeText(@NotNull @Valid AnalyzeRequest request) throws IOException {
try {
final List<String> tokens = tokenize(LuceneAnalyzerFactory.newAnalyzer(request.analyzer()), request.text());
return new AnalyzeResponse(tokens);
try (Analyzer analyzer = LuceneAnalyzerFactory.newAnalyzer(request.analyzer())) {
return new AnalyzeResponse(tokenize(analyzer, request.text()));
} catch (IllegalArgumentException e) {
throw new WebApplicationException(request.analyzer() + " not a valid analyzer", Status.BAD_REQUEST);
}
Expand Down