You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "agingade (GitHub)" <gi...@apache.org> on 2018/11/15 01:34:33 UTC
[GitHub] [geode] agingade opened pull request #2857: GEODE-6013: Use
expected initial image requester's rvv information
Re-submitting after fixing NPE from previous checkin.
Made changes to use the expected initial image requester's rvv
information instead of the image provider's local rvv while
determining full or delta GII.
There was logical error where it was using provider's local
exception(rvv) instead of using requester's local exception(rvv).
This could result in performing Delta GII instead of Full GII.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [Yes ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [Yes ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [Yes] Is your initial contribution a single, squashed commit?
- [Yes] Does `gradlew build` run cleanly?
- [Yes ] Have you written or updated unit tests to verify your changes?
- [NA] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
[ Full content available at: https://github.com/apache/geode/pull/2857 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] agingade closed pull request #2857: GEODE-6013: Use
expected initial image requester's rvv information
Posted by "agingade (GitHub)" <gi...@apache.org>.
[ pull request closed by agingade ]
[ Full content available at: https://github.com/apache/geode/pull/2857 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] bschuchardt commented on issue #2857: GEODE-6013:
Use expected initial image requester's rvv information
Posted by "bschuchardt (GitHub)" <gi...@apache.org>.
For other reviewers, here is a diff between Anil's last PR and this one:
~~~~
118c118
< index 7b03d48bfc8..ce10dfdc1b3 100644
---
> index 7b03d48bfc8..b53b5029b8d 100644
121c121
< @@ -977,34 +977,35 @@ public boolean isNewerThanOrCanFillExceptionsFor(RegionVersionVector<T> other) {
---
> @@ -977,7 +977,8 @@ public boolean isNewerThanOrCanFillExceptionsFor(RegionVersionVector<T> other) {
131,134c131
< RegionVersionHolder<T> holder = new RegionVersionHolder<T>(gcVersion.longValue());
< - return !holder.isNewerThanOrCanFillExceptionsFor(otherHolder);
< + return otherHolder.dominates(holder);
< }
---
> @@ -987,24 +988,24 @@ private boolean isGCVersionDominatedByHolder(Long gcVersion, RegionVersionHolder
184c181
< index deb7d4eaa3d..a018aaff6fa 100644
---
> index deb7d4eaa3d..ed671a6f09c 100644
187c184
< @@ -661,6 +661,145 @@ public void doesNotHangIfOtherThreadChangedVersion() throws Exception {
---
> @@ -661,6 +661,167 @@ public void doesNotHangIfOtherThreadChangedVersion() throws Exception {
235a233,254
> + public void isRvvGcDominatedByRequesterRvvReturnsFalseIfProviderRvvIsNotPresent()
> + throws Exception {
> + final String local = getIPLiteral();
> + InternalDistributedMember provider = new InternalDistributedMember(local, 101);
> + InternalDistributedMember requester = new InternalDistributedMember(local, 102);
> +
> + RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
> + 1, null, 1, false, null);
> +
> + ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
> + new ConcurrentHashMap<>();
> + RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
> + regionVersionHolder.setVersion(0);
> + // memberToRegionVersionHolder.put(provider, regionVersionHolder);
> + RegionVersionVector requesterRvv =
> + new VMRegionVersionVector(requester, memberToRegionVersionHolder,
> + 0, null, 0, false, null);
> +
> + assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isFalse();
> + }
> +
> + @Test
~~~~
[ Full content available at: https://github.com/apache/geode/pull/2857 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] bschuchardt commented on issue #2857: GEODE-6013:
Use expected initial image requester's rvv information
Posted by "bschuchardt (GitHub)" <gi...@apache.org>.
For other reviewers, here is a diff between Anil's last PR and this one:
118c118
< index 7b03d48bfc8..ce10dfdc1b3 100644
---
> index 7b03d48bfc8..b53b5029b8d 100644
121c121
< @@ -977,34 +977,35 @@ public boolean isNewerThanOrCanFillExceptionsFor(RegionVersionVector<T> other) {
---
> @@ -977,7 +977,8 @@ public boolean isNewerThanOrCanFillExceptionsFor(RegionVersionVector<T> other) {
131,134c131
< RegionVersionHolder<T> holder = new RegionVersionHolder<T>(gcVersion.longValue());
< - return !holder.isNewerThanOrCanFillExceptionsFor(otherHolder);
< + return otherHolder.dominates(holder);
< }
---
> @@ -987,24 +988,24 @@ private boolean isGCVersionDominatedByHolder(Long gcVersion, RegionVersionHolder
184c181
< index deb7d4eaa3d..a018aaff6fa 100644
---
> index deb7d4eaa3d..ed671a6f09c 100644
187c184
< @@ -661,6 +661,145 @@ public void doesNotHangIfOtherThreadChangedVersion() throws Exception {
---
> @@ -661,6 +661,167 @@ public void doesNotHangIfOtherThreadChangedVersion() throws Exception {
235a233,254
> + public void isRvvGcDominatedByRequesterRvvReturnsFalseIfProviderRvvIsNotPresent()
> + throws Exception {
> + final String local = getIPLiteral();
> + InternalDistributedMember provider = new InternalDistributedMember(local, 101);
> + InternalDistributedMember requester = new InternalDistributedMember(local, 102);
> +
> + RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
> + 1, null, 1, false, null);
> +
> + ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
> + new ConcurrentHashMap<>();
> + RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
> + regionVersionHolder.setVersion(0);
> + // memberToRegionVersionHolder.put(provider, regionVersionHolder);
> + RegionVersionVector requesterRvv =
> + new VMRegionVersionVector(requester, memberToRegionVersionHolder,
> + 0, null, 0, false, null);
> +
> + assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isFalse();
> + }
> +
> + @Test
[ Full content available at: https://github.com/apache/geode/pull/2857 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org