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/10/14 23:21:06 UTC
Review Request 39329: Requiring lucene index to be created before the
region
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39329/
-----------------------------------------------------------
Review request for geode, Ashvin A and xiaojian zhou.
Repository: geode
Description
-------
At the moment, adding an AsyncEventQueue after a region is created has
issues - See GEODE-404.
Therefore, in order to create a lucene index programmatically, the
lucene index needs to be created before the region exists so that we can
create the AEQ ahead of time.
With this change, when an LuceneIndex is created, the actual creation is
deferred in a RegionListener callback. When the region is created, we
add the AEQ to the region attributes and create the colocated regions.
Diffs
-----
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java eed80d97dee539cc3bfb0f9778f957984f3340a7
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 2bf848f6685c21f4199245771c4be3d2a3ba4e83
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 776d005b1091d58c1be46eb9a7a8113ba3daf0d0
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexCreation.java 5520f96242ab02f14e0919144fc9aa3ed4166b92
gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneRebalanceJUnitTest.java 478981f56aa4d4eec1a853f85c84014dfd05b5a6
gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java eff28130e2cd03de875b4debf6823a63cded2faa
gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java 6e44b72574d0289f44bbd715ec84409f36fc9aeb
gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexXmlGeneratorIntegrationJUnitTest.java 65c73f70e69c77bbfa8ef084b1c1712c5d31037b
gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexXmlParserIntegrationJUnitTest.java 56a726f591f9df0cadd2d4fcf9413abe272e93ea
Diff: https://reviews.apache.org/r/39329/diff/
Testing
-------
Thanks,
Dan Smith
Re: Review Request 39329: Requiring lucene index to be created before
the region
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39329/#review102811
-----------------------------------------------------------
Ship it!
Besides the comments, others look good.
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexCreation.java (line 78)
<https://reviews.apache.org/r/39329/#comment160582>
You assumed the region to be PR here, because only PR has getAttributesMutator().addAEQid().
It's ok, but we need to add a TODO for DR.
- xiaojian zhou
On Oct. 14, 2015, 9:21 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39329/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2015, 9:21 p.m.)
>
>
> Review request for geode, Ashvin A and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> At the moment, adding an AsyncEventQueue after a region is created has
> issues - See GEODE-404.
>
> Therefore, in order to create a lucene index programmatically, the
> lucene index needs to be created before the region exists so that we can
> create the AEQ ahead of time.
>
> With this change, when an LuceneIndex is created, the actual creation is
> deferred in a RegionListener callback. When the region is created, we
> add the AEQ to the region attributes and create the colocated regions.
>
>
> Diffs
> -----
>
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java eed80d97dee539cc3bfb0f9778f957984f3340a7
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 2bf848f6685c21f4199245771c4be3d2a3ba4e83
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 776d005b1091d58c1be46eb9a7a8113ba3daf0d0
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexCreation.java 5520f96242ab02f14e0919144fc9aa3ed4166b92
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneRebalanceJUnitTest.java 478981f56aa4d4eec1a853f85c84014dfd05b5a6
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java eff28130e2cd03de875b4debf6823a63cded2faa
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java 6e44b72574d0289f44bbd715ec84409f36fc9aeb
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexXmlGeneratorIntegrationJUnitTest.java 65c73f70e69c77bbfa8ef084b1c1712c5d31037b
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexXmlParserIntegrationJUnitTest.java 56a726f591f9df0cadd2d4fcf9413abe272e93ea
>
> Diff: https://reviews.apache.org/r/39329/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39329: Requiring lucene index to be created before
the region
Posted by Ashvin A <aa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39329/#review102833
-----------------------------------------------------------
Ship it!
Looks good !
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java (line 65)
<https://reviews.apache.org/r/39329/#comment160635>
Javadoc: The method will not return anything.
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java (line 75)
<https://reviews.apache.org/r/39329/#comment160636>
No return from the method.
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java (line 94)
<https://reviews.apache.org/r/39329/#comment160637>
Now getIndex can return null in two cases: index does not exist or index is created but region is still not created. If the region is not created, should getIndex throw an exception?
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java (line 106)
<https://reviews.apache.org/r/39329/#comment160638>
I believe all instances of RegionListeners will always perform this check. Would it be better if the listener could be added to a regionPath itself? That ways listener will be invoked incase of a match only?
- Ashvin A
On Oct. 14, 2015, 9:21 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39329/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2015, 9:21 p.m.)
>
>
> Review request for geode, Ashvin A and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> At the moment, adding an AsyncEventQueue after a region is created has
> issues - See GEODE-404.
>
> Therefore, in order to create a lucene index programmatically, the
> lucene index needs to be created before the region exists so that we can
> create the AEQ ahead of time.
>
> With this change, when an LuceneIndex is created, the actual creation is
> deferred in a RegionListener callback. When the region is created, we
> add the AEQ to the region attributes and create the colocated regions.
>
>
> Diffs
> -----
>
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java eed80d97dee539cc3bfb0f9778f957984f3340a7
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 2bf848f6685c21f4199245771c4be3d2a3ba4e83
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 776d005b1091d58c1be46eb9a7a8113ba3daf0d0
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexCreation.java 5520f96242ab02f14e0919144fc9aa3ed4166b92
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneRebalanceJUnitTest.java 478981f56aa4d4eec1a853f85c84014dfd05b5a6
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java eff28130e2cd03de875b4debf6823a63cded2faa
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java 6e44b72574d0289f44bbd715ec84409f36fc9aeb
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexXmlGeneratorIntegrationJUnitTest.java 65c73f70e69c77bbfa8ef084b1c1712c5d31037b
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/xml/LuceneIndexXmlParserIntegrationJUnitTest.java 56a726f591f9df0cadd2d4fcf9413abe272e93ea
>
> Diff: https://reviews.apache.org/r/39329/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>