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


Fix it, then Ship it!





geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java
 (line 17)
<https://reviews.apache.org/r/56355/#comment236148>

    This class should define it's own logger, not import one from another class.



geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java
 (line 101)
<https://reviews.apache.org/r/56355/#comment236149>

    Is this comment out of date?



geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
 (line 118)
<https://reviews.apache.org/r/56355/#comment236150>

    I would have thought the function execution framework would handle 
BucketNotFoundException and we wouldn't have to have our own retry loop. I 
wonder if the problem is really this code in LuceneFunction
    
        } catch (IOException | BucketNotFoundException e) {
          logger.warn("", e);
          throw new FunctionException(e);
        }


- Dan Smith


On Feb. 6, 2017, 11:47 p.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56355/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2017, 11:47 p.m.)
> 
> 
> Review request for geode, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> We will occasionally see EOF exceptions due to file segments being empty
> 
> This diff also reinstates the query retry.  This is needed to automatically 
> retry a query when a bucket is moved mid query execution.
> 
> Added a few null checks
> Added additional logging for exceptional cases
> 
> 
> Diffs
> -----
> 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java
>  f64ca82 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java
>  2b244e7 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
>  4ac987d 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/FileIndexInput.java
>  4b1fc81 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java
>  f1af7f3 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java
>  11647f1 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java
>  fb1913e 
> 
> Diff: https://reviews.apache.org/r/56355/diff/
> 
> 
> Testing
> -------
> 
> geode-lucene:precheckin
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Reply via email to