I am seeing build failures with:

***
FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':geode-core:spotlessJavaCheck'.
> Format violations were found. Run 'gradlew spotlessApply' to fix them.

geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java

geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionEntryTest.java

***

Planning to commit changes after running "spotlessApply" to fix the issue...

-Anil.



On Fri, Oct 28, 2016 at 2:41 PM, <dschnei...@apache.org> wrote:

> GEODE-2043: change makeTombstone to handle exception
>
> Now if makeTombstone has an exception but had already changed
> the region entry value to a TOMBSTONE, it will now change
> the value to REMOVE_PHASE2 instead of leaving it as a TOMBSTONE.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
> Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/
> commit/2ef50b24
> Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/2ef50b24
> Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/2ef50b24
>
> Branch: refs/heads/feature/GEM-983
> Commit: 2ef50b24de1457ab91729f22a3c2ff4a8a07557b
> Parents: 765a55a
> Author: Darrel Schneider <dschnei...@pivotal.io>
> Authored: Thu Oct 27 16:05:08 2016 -0700
> Committer: Darrel Schneider <dschnei...@pivotal.io>
> Committed: Fri Oct 28 14:12:59 2016 -0700
>
> ----------------------------------------------------------------------
>  .../internal/cache/AbstractRegionEntry.java     |  15 ++-
>  .../internal/cache/AbstractRegionEntryTest.java | 112 +++++++++++++++++++
>  2 files changed, 125 insertions(+), 2 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-geode/
> blob/2ef50b24/geode-core/src/main/java/org/apache/geode/internal/cache/
> AbstractRegionEntry.java
> ----------------------------------------------------------------------
> diff --git 
> a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
> b/geode-core/src/main/java/org/apache/geode/internal/
> cache/AbstractRegionEntry.java
> index 4e1f0aa..2138af9 100644
> --- a/geode-core/src/main/java/org/apache/geode/internal/
> cache/AbstractRegionEntry.java
> +++ b/geode-core/src/main/java/org/apache/geode/internal/
> cache/AbstractRegionEntry.java
> @@ -209,7 +209,7 @@ public abstract class AbstractRegionEntry implements
> RegionEntry, HashEntry<Obje
>          version.getRegionVersion())) {
>        // distributed gc with higher vector version preempts this operation
>        if (!isTombstone()) {
> -        setValue(r, Token.TOMBSTONE);
> +        basicMakeTombstone(r);
>          r.incTombstoneCount(1);
>        }
>        r.getRegionMap().removeTombstone(this, version, false, true);
> @@ -220,7 +220,7 @@ public abstract class AbstractRegionEntry implements
> RegionEntry, HashEntry<Obje
>        }
>        setRecentlyUsed();
>        boolean newEntry = (getValueAsToken() == Token.REMOVED_PHASE1);
> -      setValue(r, Token.TOMBSTONE);
> +      basicMakeTombstone(r);
>        r.scheduleTombstone(this, version);
>        if (newEntry) {
>          // bug #46631 - entry count is decremented by scheduleTombstone
> but this is a new entry
> @@ -228,6 +228,17 @@ public abstract class AbstractRegionEntry implements
> RegionEntry, HashEntry<Obje
>        }
>      }
>    }
> +  private void basicMakeTombstone(LocalRegion r) throws
> RegionClearedException {
> +    boolean setValueCompleted = false;
> +    try {
> +      setValue(r, Token.TOMBSTONE);
> +      setValueCompleted = true;
> +    } finally {
> +      if (!setValueCompleted && isTombstone()) {
> +        removePhase2();
> +      }
> +    }
> +  }
>
>
>    @Override
>
> http://git-wip-us.apache.org/repos/asf/incubator-geode/
> blob/2ef50b24/geode-core/src/test/java/org/apache/geode/internal/cache/
> AbstractRegionEntryTest.java
> ----------------------------------------------------------------------
> diff --git a/geode-core/src/test/java/org/apache/geode/internal/
> cache/AbstractRegionEntryTest.java b/geode-core/src/test/java/
> org/apache/geode/internal/cache/AbstractRegionEntryTest.java
> new file mode 100644
> index 0000000..36e3e30
> --- /dev/null
> +++ b/geode-core/src/test/java/org/apache/geode/internal/
> cache/AbstractRegionEntryTest.java
> @@ -0,0 +1,112 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements.  See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.geode.internal.cache;
> +
> +import static org.junit.Assert.*;
> +import static org.mockito.Mockito.*;
> +
> +import org.assertj.core.api.Assertions;
> +import org.junit.Test;
> +import org.junit.experimental.categories.Category;
> +
> +import org.apache.geode.internal.cache.versions.RegionVersionVector;
> +import org.apache.geode.internal.cache.versions.VersionTag;
> +import org.apache.geode.internal.offheap.annotations.Unretained;
> +import org.apache.geode.internal.util.concurrent.
> CustomEntryConcurrentHashMap.HashEntry;
> +import org.apache.geode.test.junit.categories.UnitTest;
> +
> +@Category(UnitTest.class)
> +public class AbstractRegionEntryTest {
> +
> +  @Test
> +  public void whenMakeTombstoneHasSetValueTh
> atThrowsExceptionDoesNotChangeValueToTombstone() throws
> RegionClearedException {
> +    LocalRegion lr = mock(LocalRegion.class);
> +    RegionVersionVector<?> rvv = mock(RegionVersionVector.class);
> +    when(lr.getVersionVector()).thenReturn(rvv);
> +    VersionTag<?> vt = mock(VersionTag.class);
> +    Object value = "value";
> +    AbstractRegionEntry re = new TestableRegionEntry(lr, value);
> +    assertEquals(value, re.getValueField());
> +    Assertions.assertThatThrownBy(() -> re.makeTombstone(lr, vt))
> +    .isInstanceOf(RuntimeException.class)
> +    .hasMessage("throw exception on setValue(TOMBSTONE)");
> +    assertEquals(Token.REMOVED_PHASE2, re.getValueField());
> +  }
> +
> +
> +  public static class TestableRegionEntry extends AbstractRegionEntry {
> +
> +    private Object value;
> +
> +    protected TestableRegionEntry(RegionEntryContext context, Object
> value) {
> +      super(context, value);
> +    }
> +
> +    @Override
> +    protected Object getValueField() {
> +      return this.value;
> +    }
> +
> +    @Override
> +    protected void setValueField(Object v) {
> +      this.value = v;
> +    }
> +
> +    @Override
> +    public void setValue(RegionEntryContext context, @Unretained Object
> value)
> +        throws RegionClearedException {
> +      super.setValue(context, value);
> +      if (value == Token.TOMBSTONE) {
> +        throw new RuntimeException("throw exception on
> setValue(TOMBSTONE)");
> +      }
> +    }
> +
> +    @Override
> +    public int getEntryHash() {
> +      return 0;
> +    }
> +
> +    @Override
> +    public HashEntry<Object, Object> getNextEntry() {
> +      return null;
> +    }
> +
> +    @Override
> +    public void setNextEntry(HashEntry<Object, Object> n) {
> +    }
> +
> +    @Override
> +    public Object getKey() {
> +      return null;
> +    }
> +
> +    @Override
> +    protected long getlastModifiedField() {
> +      return 0;
> +    }
> +
> +    @Override
> +    protected boolean compareAndSetLastModifiedField(long expectedValue,
> long newValue) {
> +      return false;
> +    }
> +
> +    @Override
> +    protected void setEntryHash(int v) {
> +    }
> +
> +  }
> +}
>
>

Reply via email to