Skip to content

Commit 9807c6b

Browse files
patrick-austinpatrick-austin
authored
Merge pull request #55 from icatproject/bugfix_indexWriter_closed
Add handling for unexpectedly closed IndexWriter
2 parents fba454f + 2ca22b5 commit 9807c6b

File tree

2 files changed

+72
-11
lines changed

2 files changed

+72
-11
lines changed

‎src/main/java/org/icatproject/lucene/Lucene.java‎

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ public class Lucene {
104104
* A bucket for accessing the read and write functionality for a single "shard"
105105
* Lucene index which can then be grouped to represent a single document type.
106106
*/
107-
private class ShardBucket {
107+
public class ShardBucket {
108108
private FSDirectory directory;
109-
private IndexWriter indexWriter;
109+
public IndexWriter indexWriter;
110110
private SearcherManager searcherManager;
111111
private DefaultSortedSetDocValuesReaderState state;
112112
private AtomicLong documentCount;
@@ -147,8 +147,10 @@ public ShardBucket(java.nio.file.Path shardPath) throws IOException {
147147
*
148148
* @return The number of documents committed to this shard.
149149
* @throws IOException
150+
* @throws LuceneException If the IndexWriter is closed
150151
*/
151-
public int commit() throws IOException {
152+
public int commit() throws IOException, LuceneException {
153+
ensureOpen();
152154
if (indexWriter.hasUncommittedChanges()) {
153155
indexWriter.commit();
154156
searcherManager.maybeRefreshBlocking();
@@ -183,6 +185,32 @@ private void initState(IndexSearcher indexSearcher) throws IOException {
183185
searcherManager.release(indexSearcher);
184186
}
185187
}
188+
189+
/**
190+
* To be called before attempting to commit. If the IndexWriter has
191+
* been closed, e.g. due to a previous IOException, will create a new
192+
* one and throw (so that whatever process called this knows the commit
193+
* failed and changes will not have been persisted).
194+
*
195+
* @throws IOException If index directory cannot be read/written
196+
* @throws LuceneException If the IndexWriter is closed
197+
*/
198+
public void ensureOpen() throws IOException, LuceneException {
199+
if (!indexWriter.isOpen()) {
200+
IndexWriterConfig config = new IndexWriterConfig(analyzer);
201+
indexWriter = new IndexWriter(directory, config);
202+
searcherManager = new SearcherManager(indexWriter, null);
203+
IndexSearcher indexSearcher = searcherManager.acquire();
204+
int numDocs = indexSearcher.getIndexReader().numDocs();
205+
documentCount = new AtomicLong(numDocs);
206+
initState(indexSearcher);
207+
208+
String fileName = directory.getDirectory().getFileName().toString();
209+
String message = "IndexWriter for " + fileName + " was unexpectedly closed";
210+
logger.error(message);
211+
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, message);
212+
}
213+
}
186214
}
187215

188216
/**
@@ -245,8 +273,9 @@ public List<IndexSearcher> acquireSearchers() throws IOException {
245273
*
246274
* @param document The document to be added.
247275
* @throws IOException
276+
* @throws LuceneException If the IndexWriter is closed
248277
*/
249-
public void addDocument(Document document) throws IOException {
278+
public void addDocument(Document document) throws IOException, LuceneException {
250279
ShardBucket shardBucket = routeShard();
251280
shardBucket.indexWriter.addDocument(document);
252281
shardBucket.documentCount.incrementAndGet();
@@ -282,8 +311,9 @@ public void deleteDocuments(String field, long value) throws IOException {
282311
* @param icatId The ICAT id of the document to be updated.
283312
* @param document The document that will replace the old document.
284313
* @throws IOException
314+
* @throws LuceneException If the IndexWriter is closed
285315
*/
286-
public void updateDocument(long icatId, Document document) throws IOException {
316+
public void updateDocument(long icatId, Document document) throws IOException, LuceneException {
287317
deleteDocument(icatId);
288318
addDocument(document);
289319
}
@@ -311,8 +341,9 @@ public ShardBucket buildShardBucket(int shardKey) throws IOException {
311341
* @param entityName The name of the entities being committed. Only used for
312342
* debug logging.
313343
* @throws IOException
344+
* @throws LuceneException If the IndexWriter is closed
314345
*/
315-
public void commit(String command, String entityName) throws IOException {
346+
public void commit(String command, String entityName) throws IOException, LuceneException {
316347
for (ShardBucket shardBucket : shardList) {
317348
int cached = shardBucket.commit();
318349
if (cached != 0) {
@@ -332,8 +363,10 @@ public void commit(String command, String entityName) throws IOException {
332363
public void close() throws IOException {
333364
for (ShardBucket shardBucket : shardList) {
334365
shardBucket.searcherManager.close();
335-
shardBucket.indexWriter.commit();
336-
shardBucket.indexWriter.close();
366+
if (shardBucket.indexWriter.isOpen()) {
367+
shardBucket.indexWriter.commit();
368+
shardBucket.indexWriter.close();
369+
}
337370
shardBucket.directory.close();
338371
}
339372
}
@@ -354,10 +387,12 @@ public ShardBucket getCurrentShardBucket() {
354387
*
355388
* @return The ShardBucket that the relevant Document is/should be indexed in.
356389
* @throws IOException
390+
* @throws LuceneException If the IndexWriter is closed
357391
*/
358-
public ShardBucket routeShard() throws IOException {
392+
public ShardBucket routeShard() throws IOException, LuceneException {
359393
ShardBucket shardBucket = getCurrentShardBucket();
360394
if (shardBucket.documentCount.get() >= luceneMaxShardSize) {
395+
shardBucket.ensureOpen();
361396
shardBucket.indexWriter.commit();
362397
shardBucket = buildShardBucket(shardList.size());
363398
}
@@ -446,6 +481,7 @@ public void modify(@Context HttpServletRequest request) throws LuceneException {
446481
}
447482
count = operations.size();
448483
} catch (IOException e) {
484+
logger.error("IOException while attempting to modify document(s)", e);
449485
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
450486
}
451487
logger.debug("Modified {} documents", count);
@@ -474,6 +510,7 @@ public void addNow(@Context HttpServletRequest request, @PathParam("entityName")
474510
logger.error("Could not parse JSON from {}", value);
475511
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
476512
} catch (IOException e) {
513+
logger.error("IOException while attempting to add document(s)", e);
477514
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
478515
}
479516
logger.debug("Added {} {} documents", documents.size(), entityName);
@@ -495,6 +532,7 @@ public void clear() throws LuceneException {
495532
Files.walk(luceneDirectory, FileVisitOption.FOLLOW_LINKS).sorted(Comparator.reverseOrder())
496533
.filter(f -> !luceneDirectory.equals(f)).map(java.nio.file.Path::toFile).forEach(File::delete);
497534
} catch (IOException e) {
535+
logger.error("IOException while attempting to clear", e);
498536
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
499537
}
500538

@@ -519,6 +557,7 @@ public void commit() throws LuceneException {
519557
}
520558
}
521559
} catch (IOException e) {
560+
logger.error("IOException while attempting to commit", e);
522561
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
523562
}
524563
}
@@ -571,9 +610,10 @@ private void create(JsonObject operationBody) throws NumberFormatException, IOEx
571610
* @param entityId Icat id of entity to update as a JsonNumber.
572611
* @param index Index (entity) to update.
573612
* @throws IOException
613+
* @throws LuceneException If the IndexWriter is closed
574614
*/
575615
private void aggregateFileSize(long sizeToAdd, long sizeToSubtract, long deltaFileCount, JsonNumber entityId,
576-
String index) throws IOException {
616+
String index) throws IOException, LuceneException {
577617
if (entityId != null) {
578618
aggregateFileSize(sizeToAdd, sizeToSubtract, deltaFileCount, entityId.longValueExact(), index);
579619
}
@@ -592,9 +632,10 @@ private void aggregateFileSize(long sizeToAdd, long sizeToSubtract, long deltaFi
592632
* @param entityId Icat id of entity to update as a long.
593633
* @param index Index (entity) to update.
594634
* @throws IOException
635+
* @throws LuceneException If the IndexWriter is closed
595636
*/
596637
private void aggregateFileSize(long sizeToAdd, long sizeToSubtract, long deltaFileCount, long entityId,
597-
String index) throws IOException {
638+
String index) throws IOException, LuceneException {
598639
long deltaFileSize = sizeToAdd - sizeToSubtract;
599640
if (deltaFileSize != 0 || deltaFileCount != 0) {
600641
IndexBucket indexBucket = indexBuckets.computeIfAbsent(index, k -> new IndexBucket(k));
@@ -766,6 +807,7 @@ private void delete(JsonObject operationBody) throws LuceneException, IOExceptio
766807
shardBucket.indexWriter.deleteDocuments(idQuery);
767808
}
768809
} catch (IOException e) {
810+
logger.error("IOException while attempting to delete document(s)", e);
769811
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
770812
}
771813
}
@@ -921,6 +963,7 @@ public void freeSearcher(SearchBucket search) throws LuceneException {
921963
indexBuckets.computeIfAbsent(name.toLowerCase(), k -> new IndexBucket(k))
922964
.releaseSearchers(subReaders);
923965
} catch (IOException e) {
966+
logger.error("IOException while attempting to freeSearcher", e);
924967
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
925968
}
926969
}
@@ -1136,6 +1179,7 @@ public void lock(@PathParam("entityName") String entityName, @QueryParam("minId"
11361179
}
11371180
}
11381181
} catch (IOException e) {
1182+
logger.error("IOException while attempting to lock", e);
11391183
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
11401184
}
11411185
}
@@ -1699,6 +1743,7 @@ public void unlock(@PathParam("entityName") String entityName) throws LuceneExce
16991743
try {
17001744
bucket.commit("Unlock", entityName);
17011745
} catch (IOException e) {
1746+
logger.error("IOException while attempting to unlock", e);
17021747
throw new LuceneException(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getMessage());
17031748
}
17041749
}

‎src/test/java/icat/lucene/TestLucene.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import static org.junit.Assert.assertEquals;
44
import static org.junit.Assert.assertNotNull;
5+
import static org.junit.Assert.assertNotSame;
6+
import static org.junit.Assert.assertThrows;
57
import static org.junit.Assert.assertTrue;
68

79
import java.io.IOException;
@@ -52,6 +54,7 @@
5254
import org.icatproject.lucene.IcatSynonymAnalyzer;
5355
import org.icatproject.lucene.Lucene;
5456
import org.icatproject.lucene.SearchBucket;
57+
import org.icatproject.lucene.Lucene.ShardBucket;
5558
import org.icatproject.lucene.SearchBucket.SearchType;
5659
import org.icatproject.lucene.exceptions.LuceneException;
5760
import org.junit.Test;
@@ -65,6 +68,19 @@ public class TestLucene {
6568

6669
private final FacetsConfig facetsConfig = new FacetsConfig();
6770

71+
@Test
72+
public void testEnsureOpen() throws Exception {
73+
Lucene lucene = new Lucene();
74+
Path tmpLuceneDir = Files.createTempDirectory("lucene");
75+
ShardBucket shardBucket = lucene.new ShardBucket(tmpLuceneDir.resolve("Investigation"));
76+
shardBucket.ensureOpen(); // Should not throw as still open
77+
shardBucket.indexWriter.close();
78+
IndexWriter closedIndexWriter = shardBucket.indexWriter;
79+
assertThrows(LuceneException.class, () -> shardBucket.ensureOpen());
80+
assertNotSame(closedIndexWriter, shardBucket.indexWriter);
81+
assertTrue(shardBucket.indexWriter.isOpen());
82+
}
83+
6884
@Test
6985
public void testIcatAnalyzer() throws Exception {
7086
final String text = "This is a demo of the 1st (or is it number 2) all singing and dancing TokenStream's API with added aardvarks";

0 commit comments

Comments
 (0)