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:13:23 UTC
Review Request 39328: Adding an internal listener that can modify
region attributes
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/
-----------------------------------------------------------
Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
Repository: geode
Description
-------
This listener is attached to the cache and gets a callback before any
region is created. This can be used by extensions to modify region
attributes to create resources need for a region before the region is
created.
This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/39328/diff/
Testing
-------
Thanks,
Dan Smith
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Dan Smith <ds...@pivotal.io>.
> On Oct. 14, 2015, 9:32 p.m., Darrel Schneider wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java, line 3735
> > <https://reviews.apache.org/r/39328/diff/1/?file=1098538#file1098538line3735>
> >
> > Would it be better for the collection to be synchronized instead of using a possibly wider cache sync?
I'll change it.
> On Oct. 14, 2015, 9:32 p.m., Darrel Schneider wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java, line 7
> > <https://reviews.apache.org/r/39328/diff/1/?file=1098540#file1098540line7>
> >
> > "on a ??? that"
Fixed that!
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102706
-----------------------------------------------------------
On Oct. 15, 2015, 5:18 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2015, 5:18 p.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102706
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java (line 3735)
<https://reviews.apache.org/r/39328/#comment160456>
Would it be better for the collection to be synchronized instead of using a possibly wider cache sync?
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java (line 7)
<https://reviews.apache.org/r/39328/#comment160455>
"on a ??? that"
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java (line 8)
<https://reviews.apache.org/r/39328/#comment160457>
Its probably worth pointing out that if you have multiple listeners then they are all invoked but in random order.
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java (line 16)
<https://reviews.apache.org/r/39328/#comment160458>
If you want to modify the internal region args then they must be done in place. Since it is a mutable object this should be ok
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java (line 17)
<https://reviews.apache.org/r/39328/#comment160459>
Should you javadoc what happens if methods on this interface throw an exception?
- Darrel Schneider
On Oct. 14, 2015, 2:13 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2015, 2:13 p.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102799
-----------------------------------------------------------
Ship it!
Ship It!
- xiaojian zhou
On Oct. 15, 2015, 5:18 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2015, 5:18 p.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Jacob Barrett <jb...@amduat.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102798
-----------------------------------------------------------
Great addition to the extensions!!
- Jacob Barrett
On Oct. 15, 2015, 10:18 a.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2015, 10:18 a.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Dan Smith <ds...@pivotal.io>.
> On Oct. 15, 2015, 5:23 p.m., Jacob Barrett wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java, line 167
> > <https://reviews.apache.org/r/39328/diff/2/?file=1098996#file1098996line167>
> >
> > Where is this class in use?
Whoops, unused import. I'll fix it.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102797
-----------------------------------------------------------
On Oct. 15, 2015, 5:18 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2015, 5:18 p.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Jacob Barrett <jb...@amduat.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102797
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java (line 167)
<https://reviews.apache.org/r/39328/#comment160573>
Where is this class in use?
- Jacob Barrett
On Oct. 15, 2015, 10:18 a.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2015, 10:18 a.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/#review102796
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On Oct. 15, 2015, 10:18 a.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39328/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2015, 10:18 a.m.)
>
>
> Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This listener is attached to the cache and gets a callback before any
> region is created. This can be used by extensions to modify region
> attributes to create resources need for a region before the region is
> created.
>
> This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39328/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 39328: Adding an internal listener that can modify
region attributes
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39328/
-----------------------------------------------------------
(Updated Oct. 15, 2015, 5:18 p.m.)
Review request for geode, Ashvin A, Darrel Schneider, and xiaojian zhou.
Changes
-------
I've applied all of Darrel's feedback and updated the diff.
Repository: geode
Description
-------
This listener is attached to the cache and gets a callback before any
region is created. This can be used by extensions to modify region
attributes to create resources need for a region before the region is
created.
This listener is needed to allow lucene to add an async event queue to the region attributes at region creation time.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b0f0f43be2d2beaac66256d5b07b0fd8c35ffe9c
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 9e5bcd286f802d03bc558a03a33e5cfb100b4683
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionListener.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RegionListenerJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/39328/diff/
Testing
-------
Thanks,
Dan Smith