Suranjan,
In your run, are you seeing GII failing on one server and then continuing
from another server?
Say there are 3 nodes in cluster. node1 and node2 with region r1
(exists)....
- r1 is created on node3.
- node3 starts gii from node1 (gets disconnected in between).
- node3 starts gii from node2.
-Anil.
On Tue, May 23, 2017 at 11:54 AM, Suranjan Kumar <suranjan.ku...@gmail.com>
wrote:
> Hi Darrel,
> Actually, I reproduced the issue in unit test that was seen in one of the
> runs and initializeVersionHolder doesn't fix the issue.
> The initializeFrom is used to initialize a member's RVV from a GII
> provider.
>
> It turns out if a member has already recorded higher version then while
> initializing a version holder from a member with lower version, we
> introduce an special exception.
> But in future when new versions are recorded by the same member, the
> special exception is not handled properly and that leads to version holder
> data structure corruption leading to holder#contains(version) error. In
> fact, I see that in this case an exception can remain permanently in the
> version holder even though the version has been recorded.
>
> I have a fix, which I will send for review soon. However, I just wanted
> to understand the issue and why it is not caught in delta GII.
>
> Moreover, if you look at the method initializeFrom, then already recorded
> versions are ignored? Isn't that an issue? If not then why?
> I will file a JIRA if you suggest.
>
> On Tue, May 23, 2017 at 10:49 PM, Darrel Schneider <dschnei...@pivotal.io>
> wrote:
>
> > I see that this test directly calls initializeFrom but the product never
> > does this.
> > The product always calls it through this method:
> >
> > org.apache.geode.internal.cache.versions.RegionVersionVector.
> > initializeVersionHolder(T,
> > RegionVersionHolder<T>)
> >
> > I can see that initializeVersionHolder checks a memberToVersion map and
> on
> > a miss does not call initializeFrom but instead add the new member's RVV
> to
> > the memberToVersion map. Have you considered if calling
> > initializeVersionHolder would fix the issues you are seeing?
> >
> >
> > On Tue, May 23, 2017 at 10:04 AM, Darrel Schneider <
> dschnei...@pivotal.io>
> > wrote:
> >
> > > I made these modifications and the following compiles, runs, and fails
> on
> > > geode.
> > > I added the following to RegionVersionVectorJUnitTest:
> > > @Test
> > > public void testInitialized() {
> > >
> > > RegionVersionHolder<String> vh1 = new RegionVersionHolder<>("vh1");
> > > vh1.recordVersion(56);
> > > vh1.recordVersion(57);
> > > vh1.recordVersion(58);
> > > vh1.recordVersion(59);
> > > vh1.recordVersion(60);
> > > assertTrue(vh1.contains(57));
> > > vh1 = vh1.clone();
> > > assertTrue(vh1.contains(57));
> > > System.out.println("This node init, vh1="+vh1);
> > >
> > > RegionVersionHolder<String> vh2 = new RegionVersionHolder<>("vh2");
> > > for(int i=1;i<57;i++) {
> > > vh2.recordVersion(i);
> > > }
> > > vh2 = vh2.clone();
> > > System.out.println("GII node init, vh2="+vh2);
> > >
> > > vh1.initializeFrom(vh2);
> > > // assertTrue(vh1.contains(57)); // fails
> > > vh1 = vh1.clone();
> > > System.out.println("After initialize, vh1="+vh1);
> > >
> > > vh1.recordVersion(58);
> > > vh1.recordVersion(59);
> > > vh1.recordVersion(60);
> > >
> > > System.out.println("After initialize and record version,
> vh1="+vh1);
> > >
> > > vh1 = vh1.clone();
> > > vh1.recordVersion(57);
> > > System.out.println("After initialize and record version after
> clone,
> > > vh1="+vh1);
> > >
> > > assertTrue(vh1.contains(57)); //FAILS
> > > }
> > >
> > >
> > > On Tue, May 23, 2017 at 9:37 AM, Kirk Lund <kl...@apache.org> wrote:
> > >
> > >> Are you sure you're using Geode? The signature of
> > >> RegionVersionHolder#recordVersion
> > >> in Geode is:
> > >>
> > >> RegionVersionHolder#recordVersion(long)
> > >>
> > >> I recommend checking out develop branch of Geode, write a test to
> > confirm
> > >> your bug and then submit that test with a Jira ticket.
> > >>
> > >> On Tue, May 23, 2017 at 7:10 AM, Suranjan Kumar <
> > suranjan.ku...@gmail.com
> > >> >
> > >> wrote:
> > >>
> > >> > Hi Bruce/Dan,
> > >> >
> > >> > #1. I see some issues due to introduction of special exceptions in
> > the
> > >> > RegionVersionHolder. In some cases, it is leading to
> > RegionVersionHolder
> > >> > data structure corruption causing wrong contains(version) results.
> > >> >
> > >> > This happens because we set the version to max of currentVersion or
> > >> newly
> > >> > recorded one. But do not try to fix the special exception present if
> > >> any.
> > >> >
> > >> > It is easily reproducible even in junit tests.
> > >> > The corruption of holder data stricture causes even future record
> > >> operation
> > >> > to fail.
> > >> > For example the following test fails:
> > >> >
> > >> > public void testInitialized() {
> > >> >
> > >> > RegionVersionHolder vh1 = new RegionVersionHolder(member);
> > >> > vh1.recordVersion(56, null);
> > >> > vh1.recordVersion(57, null);
> > >> > vh1.recordVersion(58, null);
> > >> > vh1.recordVersion(59, null);
> > >> > vh1.recordVersion(60, null);
> > >> > vh1 = vh1.clone();
> > >> > System.out.println("This node init, vh1="+vh1);
> > >> >
> > >> > RegionVersionHolder vh2 = new RegionVersionHolder(member);
> > >> > for(int i=1;i<57;i++) {
> > >> > vh2.recordVersion(i,null);
> > >> > }
> > >> > vh2 = vh2.clone();
> > >> > System.out.println("GII node init, vh2="+vh2);
> > >> >
> > >> > vh1.initializeFrom(vh2);
> > >> > vh1 = vh1.clone();
> > >> > System.out.println("After initialize, vh1="+vh1);
> > >> >
> > >> >
> > >> > vh1.recordVersion(58,null);
> > >> > vh1.recordVersion(59,null);
> > >> > vh1.recordVersion(60,null);
> > >> >
> > >> > System.out.println("After initialize and record version,
> vh1="+vh1);
> > >> >
> > >> > vh1 = vh1.clone();
> > >> > vh1.recordVersion(57,null);
> > >> > System.out.println("After initialize and record version after
> clone,
> > >> > vh1="+vh1);
> > >> >
> > >> > assertTrue(vh1.contains(57)); //FAILS
> > >> > }
> > >> >
> > >> >
> > >> > #2. I have observed that
> > >> > RegionVersionHolder#initializeFrom(RegionVersionHolder<T> source)
> > >> > doesn't not record already recorded version in itself contrary to
> what
> > >> > the comment above the method claims. In fact the junit tests also
> > >> > verifies the same so I couldn't understand the rationale behind it.
> It
> > >> > just adds an special exception if any higher version was already
> > >> > recorded by self.
> > >> > Shouldn't the resulting holder after initializing from a source
> > >> > contain records for both the holders? If not then why do we even
> need
> > >> > special exception.
> > >> >
> > >> > If possible could you please let me know your thoughts on this.
> > >> >
> > >> > Regards,
> > >> > Suranjan Kumar
> > >> >
> > >>
> > >
> > >
> >
>
