-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4774/#review7008
-----------------------------------------------------------


Here are some comments, mostly superficial changes, since I didn't spot any 
logical errors (as far as my understanding of the modules interfaces goes).


jsongadgets.json
<https://reviews.apache.org/r/4774/#comment15577>

    Did you intend to remove these? Surely this should be in a different 
review, since it has nothing to do with the Lucene implementation.



src/org/waveprotocol/box/server/CoreSettings.java
<https://reviews.apache.org/r/4774/#comment15578>

    'remake_index' isn't being used anywhere else. Remove it?



src/org/waveprotocol/box/server/CoreSettings.java
<https://reviews.apache.org/r/4774/#comment15579>

    Default value of 'memory' here disagrees with the comment in the config 
file of 'lucene'



src/org/waveprotocol/box/server/waveserver/IndexFieldType.java
<https://reviews.apache.org/r/4774/#comment15582>

    Perhaps my misunderstanding here, but 'abou' -> 'about'?



src/org/waveprotocol/box/server/waveserver/LucenePerUserWaveViewHandlerImpl.java
<https://reviews.apache.org/r/4774/#comment15583>

    Seems arbitrary.
    
    Is it going to cause performance problems for Lucene with it allocating 
large amounts of memory expecting up to this many results?
    



src/org/waveprotocol/box/server/waveserver/LucenePerUserWaveViewHandlerImpl.java
<https://reviews.apache.org/r/4774/#comment15584>

    With a CorruptIndexException, can we not try to recover more gracefully: as 
in force full re-index?



src/org/waveprotocol/box/server/waveserver/PerUserWaveViewDistpatcher.java
<https://reviews.apache.org/r/4774/#comment15585>

    'where' -> 'were'



src/org/waveprotocol/box/server/waveserver/PerUserWaveViewHandler.java
<https://reviews.apache.org/r/4774/#comment15586>

    Might want to just add a note here such that the emptiness is deliberate, 
for implementation elsewhere...


- Ali


On 2012-04-18 06:34:20, Yuri Zelikov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4774/
> -----------------------------------------------------------
> 
> (Updated 2012-04-18 06:34:20)
> 
> 
> Review request for wave, Michael MacFadden, vjrj, Ali Lown, and Lennard de 
> Rijk.
> 
> 
> Summary
> -------
> 
> Adds Lucene based implementation of per user wave view provider. The 
> intention is to keep in the memory only the index of waves and load the 
> wavelets into memory only when requested.
> 
> The lucene jars can be downloaded from here:
> http://apache.spd.co.il/lucene/java/3.5.0/
> 
> 
> Diffs
> -----
> 
>   .classpath d8def03 
>   .gitignore 949276a 
>   build.xml f8cba2b 
>   jsongadgets.json f512ceb 
>   server-config.xml 65c6d62 
>   server.config.example 7990e6e 
>   src/org/waveprotocol/box/server/CoreSettings.java a28ec66 
>   src/org/waveprotocol/box/server/SearchModule.java PRE-CREATION 
>   src/org/waveprotocol/box/server/ServerMain.java 5910c88 
>   src/org/waveprotocol/box/server/ServerModule.java 4debe3b 
>   src/org/waveprotocol/box/server/frontend/ClientFrontendImpl.java 5238182 
>   src/org/waveprotocol/box/server/persistence/file/FileUtils.java c8b4894 
>   src/org/waveprotocol/box/server/persistence/lucene/FSIndexDirectory.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/persistence/lucene/IndexDirectory.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/persistence/lucene/RAMIndexDirectory.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/AbstractWaveIndexer.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/IndexException.java PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/IndexFieldType.java PRE-CREATION 
>   
> src/org/waveprotocol/box/server/waveserver/LucenePerUserWaveViewHandlerImpl.java
>  PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/LuceneWaveIndexerImpl.java 
> PRE-CREATION 
>   
> src/org/waveprotocol/box/server/waveserver/MemoryPerUserWaveViewHandlerImpl.java
>  PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/MemorySearchProvider.java 
> 032a0ec 
>   src/org/waveprotocol/box/server/waveserver/MemoryWaveIndexerImpl.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/NoOpWaveIndexerImpl.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/PerUserWaveViewBus.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/PerUserWaveViewDistpatcher.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/PerUserWaveViewHandler.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/PerUserWaveViewProvider.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/PerUserWaveViewSubscriber.java 
> 23e0992 
>   src/org/waveprotocol/box/server/waveserver/ReadableWaveletDataProvider.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java 
> PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/TextCollator.java PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/WaveIndexer.java PRE-CREATION 
>   src/org/waveprotocol/box/server/waveserver/WaveMap.java a0d72d6 
>   src/org/waveprotocol/box/server/waveserver/WaveServerImpl.java 38208c8 
>   src/org/waveprotocol/box/server/waveserver/WaveServerModule.java eb43a5c 
>   test/org/waveprotocol/box/server/frontend/ClientFrontendImplTest.java 
> 59bc10d 
>   
> test/org/waveprotocol/box/server/waveserver/LucenePerUserWaveViewProviderTest.java
>  PRE-CREATION 
>   
> test/org/waveprotocol/box/server/waveserver/MemoryPerUserWaveViewProviderTest.java
>  PRE-CREATION 
>   test/org/waveprotocol/box/server/waveserver/MemorySearchProviderTest.java 
> 3a2ae13 
>   
> test/org/waveprotocol/box/server/waveserver/PerUserWaveViewDistpatcherTest.java
>  PRE-CREATION 
>   
> test/org/waveprotocol/box/server/waveserver/PerUserWaveViewProviderTestBase.java
>  PRE-CREATION 
>   
> test/org/waveprotocol/box/server/waveserver/PerUserWaveViewSubscriberTest.java
>  007ccf4 
>   
> test/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImplTest.java 
> PRE-CREATION 
>   third_party/runtime/lucene/COPYING PRE-CREATION 
>   third_party/runtime/lucene/README PRE-CREATION 
>   third_party/runtime/lucene/lucene-core-3.5.0-javadoc.jar PRE-CREATION 
>   third_party/runtime/lucene/lucene-core-3.5.0.jar PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/4774/diff
> 
> 
> Testing
> -------
> 
> Verified that the index is properly created and the search functionality 
> works.
> All tests pass (besides WaveServerTest - issue WAVE-308).
> 
> 
> Thanks,
> 
> Yuri
> 
>

Reply via email to