You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by xiaojian zhou <zh...@gmail.com> on 2016/04/01 02:37:02 UTC
Re: Review Request 45336: GEODE-920: Not to create CCN for gateway
receiver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45336/
-----------------------------------------------------------
(Updated April 1, 2016, 12:37 a.m.)
Review request for geode, anilkumar gingade and Dan Smith.
Changes
-------
This is a new fix based on a new idea.
The CCN will be created by even the gateway receiver. However, the haContainer will be kept null and wait for real cache server to initialize it.
Bugs: GEM-454, GEM-525 and GEODE-920
https://issues.apache.org/jira/browse/GEM-454
https://issues.apache.org/jira/browse/GEM-525
https://issues.apache.org/jira/browse/GEODE-920
Repository: geode
Description
-------
It's both GEODE-920, GEM-454, GEM-525.
If gateway receiver is created before cache server, the notify subscriptions attributes can never been set, since CCN singleton is created by the gateway receiver.
The easy fix is not to let gateway receiver create the CCN singleton.
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java 9bd3faf
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 95616fe
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerRegion.java 3bed769
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java 5b20e86
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 6ac4690
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/CacheClientNotifierDUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45336/diff/
Testing
-------
- introduced new dunit test.
- precheckin
- regression tests:
Thanks,
xiaojian zhou
Re: Review Request 45336: GEODE-920: Not to create CCN for gateway
receiver
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45336/#review126608
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java (line 2182)
<https://reviews.apache.org/r/45336/#comment189624>
Since its debug level log...we can keep it simple by having only one condition...
if (ccn != null && ccn.getHaContainer() != null) {
//log...
}
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java (line 3713)
<https://reviews.apache.org/r/45336/#comment189627>
We could move this to, below try catch block...
CacheClientProxy proxy;
HaContainerWrapper container;
if (ccn != null) {
container = ccn.getHAContainer();
if (container != null) {
proxy = container.getProxy();
}
}
And below we do log the message if proxy is null...in that we could log values of ccn and hacontainer...that will tell why proxy is null...
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java (line 533)
<https://reviews.apache.org/r/45336/#comment189641>
We should create HA contianer in CacheClientNotifier.getInstance() than here...As the container is owned by CCN.
Moving to getInstance() keeps the HAContainer creating thread safe...
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java (line 2152)
<https://reviews.apache.org/r/45336/#comment189642>
We can change this to:
if (!gateway) {
init()
}
No need to set it to null...as hacontainer is no more final...
- anilkumar gingade
On April 1, 2016, 12:37 a.m., xiaojian zhou wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45336/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 12:37 a.m.)
>
>
> Review request for geode, anilkumar gingade and Dan Smith.
>
>
> Bugs: GEM-454, GEM-525 and GEODE-920
> https://issues.apache.org/jira/browse/GEM-454
> https://issues.apache.org/jira/browse/GEM-525
> https://issues.apache.org/jira/browse/GEODE-920
>
>
> Repository: geode
>
>
> Description
> -------
>
> It's both GEODE-920, GEM-454, GEM-525.
>
> If gateway receiver is created before cache server, the notify subscriptions attributes can never been set, since CCN singleton is created by the gateway receiver.
>
> The easy fix is not to let gateway receiver create the CCN singleton.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java 9bd3faf
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 95616fe
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerRegion.java 3bed769
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java 5b20e86
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 6ac4690
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/CacheClientNotifierDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45336/diff/
>
>
> Testing
> -------
>
> - introduced new dunit test.
> - precheckin
> - regression tests:
>
>
> Thanks,
>
> xiaojian zhou
>
>
Re: Review Request 45336: GEODE-920: Not to create CCN for gateway
receiver
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45336/#review126883
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java (line 164)
<https://reviews.apache.org/r/45336/#comment189972>
Can we move creating HAContainer here...That way it will be called in one place under a signle condition....
E.g.:
if (ccn == null ) {
// create ccn
}
if (!isGateWayReciver && haContainer == null)
{
// create HAContainer.
}
return.
We can remove the logic creating HAContainer in CCN constructor....
- anilkumar gingade
On April 1, 2016, 10:11 p.m., xiaojian zhou wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45336/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 10:11 p.m.)
>
>
> Review request for geode, anilkumar gingade and Dan Smith.
>
>
> Bugs: GEM-454, GEM-525 and GEODE-920
> https://issues.apache.org/jira/browse/GEM-454
> https://issues.apache.org/jira/browse/GEM-525
> https://issues.apache.org/jira/browse/GEODE-920
>
>
> Repository: geode
>
>
> Description
> -------
>
> It's both GEODE-920, GEM-454, GEM-525.
>
> If gateway receiver is created before cache server, the notify subscriptions attributes can never been set, since CCN singleton is created by the gateway receiver.
>
> The easy fix is not to let gateway receiver create the CCN singleton.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java 9bd3faf
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8e30a7a
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerRegion.java 3bed769
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 6ac4690
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/CacheClientNotifierDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45336/diff/
>
>
> Testing
> -------
>
> - introduced new dunit test.
> - precheckin
> - regression tests:
>
>
> Thanks,
>
> xiaojian zhou
>
>
Re: Review Request 45336: GEODE-920: Not to create CCN for gateway
receiver
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45336/
-----------------------------------------------------------
(Updated April 1, 2016, 10:11 p.m.)
Review request for geode, anilkumar gingade and Dan Smith.
Changes
-------
fix based on anil's comments
Bugs: GEM-454, GEM-525 and GEODE-920
https://issues.apache.org/jira/browse/GEM-454
https://issues.apache.org/jira/browse/GEM-525
https://issues.apache.org/jira/browse/GEODE-920
Repository: geode
Description
-------
It's both GEODE-920, GEM-454, GEM-525.
If gateway receiver is created before cache server, the notify subscriptions attributes can never been set, since CCN singleton is created by the gateway receiver.
The easy fix is not to let gateway receiver create the CCN singleton.
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/InitialImageOperation.java 9bd3faf
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8e30a7a
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerRegion.java 3bed769
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java 6ac4690
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/CacheClientNotifierDUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45336/diff/
Testing
-------
- introduced new dunit test.
- precheckin
- regression tests:
Thanks,
xiaojian zhou