From b5086b98b6a4958af15beef1eb2c73d9295e61b7 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 9 Jun 2020 08:40:49 +0200 Subject: [PATCH 1/4] rebuild index in background if lucene throws an IllegalArgumentException (incompatible/old index). split getFSDirectory(boolean delete) into two methods so that resources can be properly closed. --- .../business/search/IndexManagerImpl.java | 71 ++++++++----------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java b/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java index aa2cb5d0f8..60df84d99a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.lang.reflect.InvocationTargetException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -74,7 +75,7 @@ public class IndexManagerImpl implements IndexManager { private boolean searchEnabled = true; - File indexConsistencyMarker; + private final File indexConsistencyMarker; private String indexDir = null; @@ -125,15 +126,11 @@ public void initialize() throws InitializationException { // only initialize the index if search is enabled if (this.searchEnabled) { - // 1. If inconsistency marker exists. - // Delete index - // 2. if we're using RAM index - // load ram index wrapper around index - // + // delete index if inconsistency marker exists if (indexConsistencyMarker.exists()) { - getFSDirectory(true); - inconsistentAtStartup = true; mLogger.debug("Index inconsistent: marker exists"); + inconsistentAtStartup = true; + deleteIndex(); } else { try { File makeIndexDir = new File(indexDir); @@ -153,26 +150,26 @@ public void initialize() throws InitializationException { // test if the index is readable, if the version is outdated or it fails we rebuild. try { synchronized(this) { - reader = DirectoryReader.open(getFSDirectory(false)); + reader = DirectoryReader.open(getIndexDirectory()); } - } catch (IOException ex) { - mLogger.warn("Error opening search index, scheduling rebuild.", ex); - getFSDirectory(true); + } catch (IOException | IllegalArgumentException ex) { // IAE for incompatible codecs + mLogger.warn("Failed to open search index, scheduling rebuild.", ex); inconsistentAtStartup = true; + deleteIndex(); } } else { mLogger.debug("Creating index"); inconsistentAtStartup = true; - - createIndex(getFSDirectory(true)); + deleteIndex(); + createIndex(getIndexDirectory()); } - if (isInconsistentAtStartup()) { + if (inconsistentAtStartup) { mLogger.info("Index was inconsistent. Rebuilding index in the background..."); try { rebuildWebsiteIndex(); - } catch (WebloggerException e) { - mLogger.error("ERROR: scheduling re-index operation"); + } catch (WebloggerException ex) { + mLogger.error("ERROR: scheduling re-index operation", ex); } } else { mLogger.info("Index initialized and ready for use."); @@ -314,7 +311,13 @@ public synchronized IndexReader getSharedIndexReader() { * @return Directory The directory containing the index, or null if error. */ public Directory getIndexDirectory() { - return getFSDirectory(false); + + try { + return FSDirectory.open(Path.of(indexDir)); + } catch (IOException e) { + mLogger.error("Problem accessing index directory", e); + } + return null; } private boolean indexExists() { @@ -326,31 +329,19 @@ private boolean indexExists() { return false; } - private FSDirectory getFSDirectory(boolean delete) { - - FSDirectory directory = null; - - try { - - directory = FSDirectory.open(Path.of(indexDir)); - - if (delete && directory != null) { - // clear old files - String[] files = directory.listAll(); - for (int i = 0; i < files.length; i++) { - File file = new File(indexDir, files[i]); - if (!file.delete()) { - throw new IOException("couldn't delete " + files[i]); - } - } + + private void deleteIndex() { + + try(FSDirectory directory = FSDirectory.open(Path.of(indexDir))) { + + String[] files = directory.listAll(); + for (String file : files) { + Files.delete(Path.of(indexDir, file)); } - - } catch (IOException e) { - mLogger.error("Problem accessing index directory", e); + } catch (IOException ex) { + mLogger.error("Problem accessing index directory", ex); } - return directory; - } private void createIndex(Directory dir) { From 7e3238d43421906b0c6353559e73c8872be29c3e Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 9 Jun 2020 08:54:12 +0200 Subject: [PATCH 2/4] code cleanup. --- .../business/search/IndexManagerImpl.java | 46 +++++-------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java b/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java index 60df84d99a..077382b264 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/search/IndexManagerImpl.java @@ -61,30 +61,22 @@ */ @com.google.inject.Singleton public class IndexManagerImpl implements IndexManager { - // ~ Static fields/initializers - // ============================================= private IndexReader reader; private final Weblogger roller; - static Log mLogger = LogFactory.getFactory().getInstance( - IndexManagerImpl.class); - - // ~ Instance fields - // ======================================================== + private final static Log mLogger = LogFactory.getFactory().getInstance(IndexManagerImpl.class); private boolean searchEnabled = true; - private final File indexConsistencyMarker; + private final String indexDir; - private String indexDir = null; + private final File indexConsistencyMarker; private boolean inconsistentAtStartup = false; private final ReadWriteLock rwl = new ReentrantReadWriteLock(); - // ~ Constructors - // =========================================================== /** * Creates a new lucene index manager. This should only be created once. @@ -178,48 +170,34 @@ public void initialize() throws InitializationException { } - // ~ Methods - // ================================================================ - @Override public void rebuildWebsiteIndex() throws WebloggerException { - scheduleIndexOperation(new RebuildWebsiteIndexOperation(roller, this, - null)); + scheduleIndexOperation(new RebuildWebsiteIndexOperation(roller, this, null)); } @Override public void rebuildWebsiteIndex(Weblog website) throws WebloggerException { - scheduleIndexOperation(new RebuildWebsiteIndexOperation(roller, this, - website)); + scheduleIndexOperation(new RebuildWebsiteIndexOperation(roller, this, website)); } @Override public void removeWebsiteIndex(Weblog website) throws WebloggerException { - scheduleIndexOperation(new RemoveWebsiteIndexOperation(roller, this, - website)); + scheduleIndexOperation(new RemoveWebsiteIndexOperation(roller, this, website)); } @Override - public void addEntryIndexOperation(WeblogEntry entry) - throws WebloggerException { - AddEntryOperation addEntry = new AddEntryOperation(roller, this, entry); - scheduleIndexOperation(addEntry); + public void addEntryIndexOperation(WeblogEntry entry) throws WebloggerException { + scheduleIndexOperation(new AddEntryOperation(roller, this, entry)); } @Override - public void addEntryReIndexOperation(WeblogEntry entry) - throws WebloggerException { - ReIndexEntryOperation reindex = new ReIndexEntryOperation(roller, this, - entry); - scheduleIndexOperation(reindex); + public void addEntryReIndexOperation(WeblogEntry entry) throws WebloggerException { + scheduleIndexOperation(new ReIndexEntryOperation(roller, this, entry)); } @Override - public void removeEntryIndexOperation(WeblogEntry entry) - throws WebloggerException { - RemoveEntryOperation removeOp = new RemoveEntryOperation(roller, this, - entry); - executeIndexOperationNow(removeOp); + public void removeEntryIndexOperation(WeblogEntry entry) throws WebloggerException { + executeIndexOperationNow(new RemoveEntryOperation(roller, this, entry)); } public ReadWriteLock getReadWriteLock() { From 6abff018ba4ac3e93b5210987d2c4365c987dbe8 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 9 Jun 2020 09:07:09 +0200 Subject: [PATCH 3/4] added lucene-backward-codecs dependency for index compatibility between point-release updates. updated most other dependencies to latest versions. tested on 11, 14 and 15b26. --- app/pom.xml | 22 +++++++++++++++------- pom.xml | 2 +- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/app/pom.xml b/app/pom.xml index 3f9fde5589..81e5c8fe79 100644 --- a/app/pom.xml +++ b/app/pom.xml @@ -39,22 +39,22 @@ limitations under the License. 1.4.7 1.2 1.7.8 - 1.10.7 - 7.2 + 1.10.8 + 8.0.1 1.6 1.9.4 3.1 - 1.13 - 2.7.5 - 4.2.2 + 1.14 + 2.7.7 + 4.2.3 1.2.17 2.10.0 - 8.3.0 + 8.5.1 20100527 3.2.3 2.17 1.0b3 - 1.12.2 + 1.13.1 1.7.26 4.1.4.RELEASE 3.2.5.RELEASE @@ -264,6 +264,7 @@ limitations under the License. jquery-validation 1.19.0 + org.apache.lucene lucene-analyzers-common @@ -271,6 +272,13 @@ limitations under the License. ${lucene.version} + + org.apache.lucene + lucene-backward-codecs + compile + ${lucene.version} + + org.apache.lucene lucene-queryparser diff --git a/pom.xml b/pom.xml index cd3db9b612..a4e20a56b9 100644 --- a/pom.xml +++ b/pom.xml @@ -108,7 +108,7 @@ limitations under the License. org.junit.jupiter junit-jupiter-engine - 5.5.2 + 5.6.2 test From c124e3b7079005570fe382b796ea0ea677d60e71 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 9 Jun 2020 09:18:56 +0200 Subject: [PATCH 4/4] removed redundant synchronized map wrapper since cache is only accessed from within synchronized blocks and doesn't escape. added @Override. --- .../util/cache/ExpiringLRUCacheImpl.java | 6 +----- .../weblogger/util/cache/LRUCacheImpl.java | 16 ++++++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/util/cache/ExpiringLRUCacheImpl.java b/app/src/main/java/org/apache/roller/weblogger/util/cache/ExpiringLRUCacheImpl.java index f4ea13ad04..27049c2f8b 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/cache/ExpiringLRUCacheImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/cache/ExpiringLRUCacheImpl.java @@ -75,11 +75,7 @@ public synchronized void put(String key, Object value) { public synchronized Object get(String key) { Object value = null; - ExpiringCacheEntry entry = null; - - synchronized(this) { - entry = (ExpiringCacheEntry) super.get(key); - } + ExpiringCacheEntry entry = (ExpiringCacheEntry) super.get(key); if (entry != null) { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/cache/LRUCacheImpl.java b/app/src/main/java/org/apache/roller/weblogger/util/cache/LRUCacheImpl.java index f2d702da3f..8ffc4ed56d 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/cache/LRUCacheImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/cache/LRUCacheImpl.java @@ -18,7 +18,6 @@ package org.apache.roller.weblogger.util.cache; -import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.LinkedHashMap; @@ -31,8 +30,8 @@ */ public class LRUCacheImpl implements Cache { - private String id = null; - private Map cache = null; + private final String id; + private final Map cache; // for metrics protected double hits = 0; @@ -44,18 +43,18 @@ public class LRUCacheImpl implements Cache { protected LRUCacheImpl(String id) { - this.id = id; - this.cache = Collections.synchronizedMap(new LRULinkedHashMap(100)); + this(id, 100); } protected LRUCacheImpl(String id, int maxsize) { this.id = id; - this.cache = Collections.synchronizedMap(new LRULinkedHashMap(maxsize)); + this.cache = new LRULinkedHashMap(maxsize); } + @Override public String getId() { return this.id; } @@ -64,6 +63,7 @@ public String getId() { /** * Store an entry in the cache. */ + @Override public synchronized void put(String key, Object value) { this.cache.put(key, value); @@ -74,6 +74,7 @@ public synchronized void put(String key, Object value) { /** * Retrieve an entry from the cache. */ + @Override public synchronized Object get(String key) { Object obj = this.cache.get(key); @@ -89,6 +90,7 @@ public synchronized Object get(String key) { } + @Override public synchronized void remove(String key) { this.cache.remove(key); @@ -96,6 +98,7 @@ public synchronized void remove(String key) { } + @Override public synchronized void clear() { this.cache.clear(); @@ -109,6 +112,7 @@ public synchronized void clear() { } + @Override public Map getStats() { Map stats = new HashMap();