You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2015/08/27 02:13:02 UTC

Review Request 37822: Adding an AsyncEventListener to write events to Lucene

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

Review request for geode, Ashvin A and xiaojian zhou.


Repository: geode


Description
-------

Adding an implementation of the listener and a unit test. I also added
several other interfaces for collaborators with this AsyncEventListener.
    
SingleIndexRepository - Interface for a single lucene index (in other
words, a single index on a single bucket for a PR). This currently has
methods for writing to the index.
    
RepositoryManager - interface for obtaining SingleIndexRepository
objects based on a user region and a key.
    
ObjectToDocumentMapper - interface for translating a gemfire object
into an indexable document.


Diffs
-----

  gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java PRE-CREATION 
  gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java PRE-CREATION 
  gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java PRE-CREATION 
  gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepository.java PRE-CREATION 
  gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java PRE-CREATION 
  gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListenerJUnitTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/37822/diff/


Testing
-------


Thanks,

Dan Smith


Re: Review Request 37822: Adding an AsyncEventListener to write events to Lucene

Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37822/#review96839
-----------------------------------------------------------



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java (line 57)
<https://reviews.apache.org/r/37822/#comment152521>

    how to simulate invalidate, destroy, clear events? 
    
    For example, what are actually cleared by a clear() event in AEQ?


- xiaojian zhou


On Aug. 27, 2015, 12:13 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37822/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 12:13 a.m.)
> 
> 
> Review request for geode, Ashvin A and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adding an implementation of the listener and a unit test. I also added
> several other interfaces for collaborators with this AsyncEventListener.
>     
> SingleIndexRepository - Interface for a single lucene index (in other
> words, a single index on a single bucket for a PR). This currently has
> methods for writing to the index.
>     
> RepositoryManager - interface for obtaining SingleIndexRepository
> objects based on a user region and a key.
>     
> ObjectToDocumentMapper - interface for translating a gemfire object
> into an indexable document.
> 
> 
> Diffs
> -----
> 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepository.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java PRE-CREATION 
>   gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListenerJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37822/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 37822: Adding an AsyncEventListener to write events to Lucene

Posted by Dan Smith <ds...@pivotal.io>.

> On Aug. 27, 2015, 10:22 p.m., Ashvin A wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java, line 16
> > <https://reviews.apache.org/r/37822/diff/1/?file=1055469#file1055469line16>
> >
> >     Given the name of the interface and javaDoc for this method, I thought the return type will be a Lucene Document object?

I'll change it to Document


> On Aug. 27, 2015, 10:22 p.m., Ashvin A wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java, line 22
> > <https://reviews.apache.org/r/37822/diff/1/?file=1055469#file1055469line22>
> >
> >     I may not have understood this correctly. Is there one lucene doc per KV? If so then there will be a 1-to-1 mapping between Key and docid. DocId is an integer. Geode will need to add and delete docId from the index. What is the role of Term?

For deletes, we don't know the docid. We need a way to convert the region key into something we can use to delete or update the document. What this method does is convert the region key into a search term, which is what IndexWriter.deleteDocuments requires.


> On Aug. 27, 2015, 10:22 p.m., Ashvin A wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java, line 7
> > <https://reviews.apache.org/r/37822/diff/1/?file=1055470#file1055470line7>
> >
> >     Can a user can define multiple indexes on a region? If so getRepository will need more inputs to return a SingleIndexRepository?

I was thinking of creating something like a MultipleIndexRepostory that broadcasts the op to each index, but based on our converation this morning, I think each LuceneIndex will have it's own RepositoryManager. In any case, maybe I should rename SingleIndexRepository.


> On Aug. 27, 2015, 10:22 p.m., Ashvin A wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java, line 22
> > <https://reviews.apache.org/r/37822/diff/1/?file=1055472#file1055472line22>
> >
> >     This is so confusing. The latest IndexWriter on Lucene trunk has the following method:
> >     public void addDocument(IndexDocument doc)
> >     
> >     and origin/lucene_solr_5_3 branch's IndexWriter has this method
> >     public void addDocument(Iterable<? extends IndexableField> doc)
> >     
> >     Is the API going to change?

Interesting. Document implements IndexDocument, so I don't this is a breaking change, but I think they are making it easier to serialize your own object into the lucene index.


> On Aug. 27, 2015, 10:22 p.m., Ashvin A wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java, line 32
> > <https://reviews.apache.org/r/37822/diff/1/?file=1055472#file1055472line32>
> >
> >     javadoc for writer.deleteDocument = Deletes the document(s) containing any of the terms. 
> >     
> >     The intent is to remove one document related to key and not to remove all documents containing the term.

Thats the way the IndexWriter API works. You tell it to delete based on a query essentially. In this case, we're adding a unique "key" field to the document and then telling it to delete based on the key.


- Dan


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


On Aug. 27, 2015, 12:13 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37822/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 12:13 a.m.)
> 
> 
> Review request for geode, Ashvin A and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adding an implementation of the listener and a unit test. I also added
> several other interfaces for collaborators with this AsyncEventListener.
>     
> SingleIndexRepository - Interface for a single lucene index (in other
> words, a single index on a single bucket for a PR). This currently has
> methods for writing to the index.
>     
> RepositoryManager - interface for obtaining SingleIndexRepository
> objects based on a user region and a key.
>     
> ObjectToDocumentMapper - interface for translating a gemfire object
> into an indexable document.
> 
> 
> Diffs
> -----
> 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepository.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java PRE-CREATION 
>   gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListenerJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37822/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 37822: Adding an AsyncEventListener to write events to Lucene

Posted by Ashvin A <aa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37822/#review96766
-----------------------------------------------------------



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java (line 16)
<https://reviews.apache.org/r/37822/#comment152416>

    Given the name of the interface and javaDoc for this method, I thought the return type will be a Lucene Document object?



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java (line 22)
<https://reviews.apache.org/r/37822/#comment152418>

    I may not have understood this correctly. Is there one lucene doc per KV? If so then there will be a 1-to-1 mapping between Key and docid. DocId is an integer. Geode will need to add and delete docId from the index. What is the role of Term?



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java (line 7)
<https://reviews.apache.org/r/37822/#comment152412>

    Can a user can define multiple indexes on a region? If so getRepository will need more inputs to return a SingleIndexRepository?



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java (line 22)
<https://reviews.apache.org/r/37822/#comment152427>

    This is so confusing. The latest IndexWriter on Lucene trunk has the following method:
    public void addDocument(IndexDocument doc)
    
    and origin/lucene_solr_5_3 branch's IndexWriter has this method
    public void addDocument(Iterable<? extends IndexableField> doc)
    
    Is the API going to change?



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java (line 32)
<https://reviews.apache.org/r/37822/#comment152421>

    javadoc for writer.deleteDocument = Deletes the document(s) containing any of the terms. 
    
    The intent is to remove one document related to key and not to remove all documents containing the term.


- Ashvin A


On Aug. 27, 2015, 12:13 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37822/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 12:13 a.m.)
> 
> 
> Review request for geode, Ashvin A and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adding an implementation of the listener and a unit test. I also added
> several other interfaces for collaborators with this AsyncEventListener.
>     
> SingleIndexRepository - Interface for a single lucene index (in other
> words, a single index on a single bucket for a PR). This currently has
> methods for writing to the index.
>     
> RepositoryManager - interface for obtaining SingleIndexRepository
> objects based on a user region and a key.
>     
> ObjectToDocumentMapper - interface for translating a gemfire object
> into an indexable document.
> 
> 
> Diffs
> -----
> 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepository.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java PRE-CREATION 
>   gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListenerJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37822/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 37822: Adding an AsyncEventListener to write events to Lucene

Posted by Dan Smith <ds...@pivotal.io>.

> On Aug. 27, 2015, 12:35 a.m., anilkumar gingade wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java, line 49
> > <https://reviews.apache.org/r/37822/diff/1/?file=1055468#file1055468line49>
> >
> >     Why its called SingleIndexRepository...Will there be a AsyncListner for each Lucene index created on the region?

Good point, I suppose a single AsyncEventListener could handle multiple LuceneIndex objects. I was going to call it LuceneRepository but I think Gester was already creating a class with that name. Will discuss.


- Dan


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


On Aug. 27, 2015, 12:13 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37822/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 12:13 a.m.)
> 
> 
> Review request for geode, Ashvin A and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adding an implementation of the listener and a unit test. I also added
> several other interfaces for collaborators with this AsyncEventListener.
>     
> SingleIndexRepository - Interface for a single lucene index (in other
> words, a single index on a single bucket for a PR). This currently has
> methods for writing to the index.
>     
> RepositoryManager - interface for obtaining SingleIndexRepository
> objects based on a user region and a key.
>     
> ObjectToDocumentMapper - interface for translating a gemfire object
> into an indexable document.
> 
> 
> Diffs
> -----
> 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepository.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java PRE-CREATION 
>   gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListenerJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37822/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 37822: Adding an AsyncEventListener to write events to Lucene

Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37822/#review96618
-----------------------------------------------------------



gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java (line 49)
<https://reviews.apache.org/r/37822/#comment152209>

    Why its called SingleIndexRepository...Will there be a AsyncListner for each Lucene index created on the region?


- anilkumar gingade


On Aug. 27, 2015, 12:13 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37822/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 12:13 a.m.)
> 
> 
> Review request for geode, Ashvin A and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adding an implementation of the listener and a unit test. I also added
> several other interfaces for collaborators with this AsyncEventListener.
>     
> SingleIndexRepository - Interface for a single lucene index (in other
> words, a single index on a single bucket for a PR). This currently has
> methods for writing to the index.
>     
> RepositoryManager - interface for obtaining SingleIndexRepository
> objects based on a user region and a key.
>     
> ObjectToDocumentMapper - interface for translating a gemfire object
> into an indexable document.
> 
> 
> Diffs
> -----
> 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/ObjectToDocumentMapper.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/RepositoryManager.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepository.java PRE-CREATION 
>   gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/SingleIndexRepositoryImpl.java PRE-CREATION 
>   gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListenerJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37822/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>