SourceForge.net Logo
Main Overview Wiki Issues Forum Build Fisheye
Issue Details (XML | Word | Printable)

Key: CMP-896
Type: Bug Bug
Status: Open Open
Priority: Minor Minor
Assignee: Shay Banon
Reporter: Stefan Fussenegger
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Compass

Unnecessary Exception from ConcurrentParallelIndexExecutor when creating index for single entity with 2 or more devices

Created: 18/Aug/09 07:55 AM   Updated: 19/Aug/09 04:39 AM
Component/s: Compass::Core
Affects Version/s: 2.2.0 GA
Fix Version/s: None

File Attachments: 1. Text File cmp896.patch (1 kB)



 Description  « Hide
When using two devices, index operations (AbstractCompassGps.index(Class...)) with entities of just a single device will fail as ConcurrentParallelIndexExecutor of the other device will be called with an empty array of IndexEntities.

stacktrace summary:

1) DefaultReplaceIndexCallback.buildIndexIfNeeded() calls index for any device
2) AbstractParallelGpsDevice.index(IndexPlan) search for entities of this device and passes them without further checking to parallelIndexExecutor
3) ConcurrentParallelIndexExecutor throws an IllegalArgumentException if entities.length == 0

stacktrace:

java.lang.IllegalArgumentException No entities listed to be indexed, have you defined your entities correctly?
at org.compass.gps.device.support.parallel.ConcurrentParallelIndexExecutor.performIndex(ConcurrentParallelIndexExecutor.java:88)
at org.compass.gps.device.support.parallel.AbstractParallelGpsDevice.index(AbstractParallelGpsDevice.java:119)
at org.compass.gps.impl.DefaultReplaceIndexCallback.buildIndexIfNeeded(DefaultReplaceIndexCallback.java:42)
at org.compass.core.lucene.engine.manager.DefaultLuceneSearchEngineIndexManager$ReplaceIndexOperationCallback.firstStep(DefaultLuceneSearchEngineIndexManager.java:281)
at org.compass.core.lucene.engine.manager.DefaultLuceneSearchEngineIndexManager.doOperate(DefaultLuceneSearchEngineIndexManager.java:218)
at org.compass.core.lucene.engine.manager.DefaultLuceneSearchEngineIndexManager.doReplaceIndex(DefaultLuceneSearchEngineIndexManager.java:266)
at org.compass.core.lucene.engine.manager.DefaultLuceneSearchEngineIndexManager.replaceIndex(DefaultLuceneSearchEngineIndexManager.java:261)
at org.compass.gps.impl.SingleCompassGps.doIndex(SingleCompassGps.java:118)
at org.compass.gps.impl.AbstractCompassGps.index(AbstractCompassGps.java:154)
at org.compass.gps.impl.AbstractCompassGps.index(AbstractCompassGps.java:132)

Fix

either check entitiesToIndex.length in AbstractParallelGpsDevice.index(IndexPlan) or allow entities.length == 0 in ConcurrentParallelIndexExecutor

Workaround

This class can be used instead of ConcurrentParallelIndexExecutor to avoid the exception:

public class FixedConcurrentParallelIndexExecutor extends ConcurrentParallelIndexExecutor {
public FixedConcurrentParallelIndexExecutor() { super(); }
public FixedConcurrentParallelIndexExecutor(final int maxThreads) { super(maxThreads); }

@Override
public void performIndex(final IndexEntity[][] entities, final IndexEntitiesIndexer indexEntitiesIndexer, final CompassGpsInterfaceDevice compassGps) {
if (entities.length == 0) { // avoid exception return; }
super.performIndex(entities, indexEntitiesIndexer, compassGps);
}
}



 All   Comments   Change History      Sort Order: Ascending order - Click to sort in descending order
Stefan Fussenegger added a comment - 18/Aug/09 08:08 AM
patch for second suggested possible fix:

1) allow entities.length == 0 in ConcurrentParallelIndexExecutor
2) add javadoc to ParallelIndexExecutor that empty entities arrays should be silently ignored


Shay Banon added a comment - 18/Aug/09 03:06 PM
I have added a flag to the Concurrent indexer where you can set it to ignore entities. Many users run into this problem where they try to index something and they forgot to add Compass mappings, so I think that by default, in most cases, this exception is important (as a first user experience).

Stefan Fussenegger added a comment - 19/Aug/09 01:57 AM
could you please merge this into the 2.2 branch?

Shay Banon added a comment - 19/Aug/09 02:26 AM
For the 2.2 branch you can use the Fixed extension you did, I think its clean and thats why the extension is there.

Stefan Fussenegger added a comment - 19/Aug/09 04:39 AM
Yeah, I'm happy with my workaround and can definitely live with it. However, I've spent some time figuring out what the actual reason for the exception was (especially as the exception message pointed me towards the wrong direction). As I'd like to save others from doing the same work, I'd suggest to add this property and a hint to set this property to the exception message - for ease of use and productivity, not only to make things work. It's a trivial and non-breaking change anyway. But if you don't like the idea, I don't mind. I'll keep submitting patches anyway