itschrispeck commented on code in PR #11558:
URL: https://github.com/apache/pinot/pull/11558#discussion_r1322407044
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java:
##########
@@ -109,27 +112,46 @@ public ImmutableRoaringBitmap getDictIds(String
searchQuery) {
@Override
public MutableRoaringBitmap getDocIds(String searchQuery) {
MutableRoaringBitmap docIDs = new MutableRoaringBitmap();
- Collector docIDCollector = new RealtimeLuceneDocIdCollector(docIDs);
- IndexSearcher indexSearcher = null;
- try {
- Query query = new QueryParser(_column, _analyzer).parse(searchQuery);
- indexSearcher = _searcherManager.acquire();
- indexSearcher.search(query, docIDCollector);
- return getPinotDocIds(indexSearcher, docIDs);
- } catch (Exception e) {
- LOGGER
- .error("Failed while searching the realtime text index for column
{}, search query {}, exception {}", _column,
- searchQuery, e.getMessage());
- throw new RuntimeException(e);
- } finally {
+ RealtimeLuceneDocIdCollector docIDCollector = new
RealtimeLuceneDocIdCollector(docIDs);
+ // A thread interrupt during indexSearcher.search() can break the
underlying FSDirectory used by the IndexWriter
+ // which the SearcherManager is created with. To ensure the index is never
corrupted the search is executed
+ // in a child thread and the interrupt is handled in the current thread by
canceling the search gracefully
+ Callable<MutableRoaringBitmap> searchCallable = () -> {
+ IndexSearcher indexSearcher = null;
try {
- if (indexSearcher != null) {
- _searcherManager.release(indexSearcher);
+ Query query = new QueryParser(_column, _analyzer).parse(searchQuery);
+ indexSearcher = _searcherManager.acquire();
+ indexSearcher.search(query, docIDCollector);
+ return getPinotDocIds(indexSearcher, docIDs);
+ } finally {
+ try {
+ if (indexSearcher != null) {
+ _searcherManager.release(indexSearcher);
+ }
+ } catch (Exception e) {
+ LOGGER.error(
+ "Failed while releasing the searcher manager for realtime text
index for column {}, exception {}",
+ _column, e.getMessage());
+ }
+ }
+ };
+ FutureTask<MutableRoaringBitmap> searchTask = new
FutureTask<>(searchCallable);
+ Thread searcherThread = new Thread(searchTask);
+ searcherThread.start();
+ try {
+ while (!searchTask.isDone()) {
Review Comment:
ahh, I don't think I can use `.notify()` in the finally block since it's
likely to get called before the `.wait()` is even reached - and moving the
`.wait()` into a loop as required ends up in about the same place we started
But I can simplify it with just `future.get()` and catch the interrupt, so
thank you 😄
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java:
##########
@@ -109,27 +112,46 @@ public ImmutableRoaringBitmap getDictIds(String
searchQuery) {
@Override
public MutableRoaringBitmap getDocIds(String searchQuery) {
MutableRoaringBitmap docIDs = new MutableRoaringBitmap();
- Collector docIDCollector = new RealtimeLuceneDocIdCollector(docIDs);
- IndexSearcher indexSearcher = null;
- try {
- Query query = new QueryParser(_column, _analyzer).parse(searchQuery);
- indexSearcher = _searcherManager.acquire();
- indexSearcher.search(query, docIDCollector);
- return getPinotDocIds(indexSearcher, docIDs);
- } catch (Exception e) {
- LOGGER
- .error("Failed while searching the realtime text index for column
{}, search query {}, exception {}", _column,
- searchQuery, e.getMessage());
- throw new RuntimeException(e);
- } finally {
+ RealtimeLuceneDocIdCollector docIDCollector = new
RealtimeLuceneDocIdCollector(docIDs);
+ // A thread interrupt during indexSearcher.search() can break the
underlying FSDirectory used by the IndexWriter
+ // which the SearcherManager is created with. To ensure the index is never
corrupted the search is executed
+ // in a child thread and the interrupt is handled in the current thread by
canceling the search gracefully
+ Callable<MutableRoaringBitmap> searchCallable = () -> {
+ IndexSearcher indexSearcher = null;
try {
- if (indexSearcher != null) {
- _searcherManager.release(indexSearcher);
+ Query query = new QueryParser(_column, _analyzer).parse(searchQuery);
+ indexSearcher = _searcherManager.acquire();
+ indexSearcher.search(query, docIDCollector);
+ return getPinotDocIds(indexSearcher, docIDs);
+ } finally {
+ try {
+ if (indexSearcher != null) {
+ _searcherManager.release(indexSearcher);
+ }
+ } catch (Exception e) {
+ LOGGER.error(
+ "Failed while releasing the searcher manager for realtime text
index for column {}, exception {}",
+ _column, e.getMessage());
+ }
+ }
+ };
+ FutureTask<MutableRoaringBitmap> searchTask = new
FutureTask<>(searchCallable);
+ Thread searcherThread = new Thread(searchTask);
+ searcherThread.start();
+ try {
+ while (!searchTask.isDone()) {
Review Comment:
ahh, I don't think I can use `.notify()` in the finally block since it's
likely to get called before the `.wait()` is even reached - and moving the
`.wait()` into a loop as required ends up in about the same place we started
But I can simplify it with just `future.get()` and catch the interrupt, so
thank you 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
