DonalEvans commented on a change in pull request #5978:
URL: https://github.com/apache/geode/pull/5978#discussion_r567142675



##########
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/TestDelta.java
##########
@@ -62,10 +79,18 @@ public boolean hasDelta() {
     return hasDelta;
   }
 
+  @Override
+  public boolean getForceRecalculateSize() {
+    // new Exception("RINGLES - getForceRecalculateSize Called: " +
+    // forceRecalculateSize).printStackTrace();

Review comment:
       This comment should be removed.

##########
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/DeltaForceSizingFlagDUnitTest.java
##########
@@ -0,0 +1,1070 @@
+/*
+ * 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.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.File;
+import java.io.IOException;
+import java.util.Random;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.Test;
+
+import org.apache.geode.DataSerializable;
+import org.apache.geode.DataSerializer;
+import org.apache.geode.cache.AttributesFactory;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.InterestPolicy;
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.cache.SubscriptionAttributes;
+import org.apache.geode.cache.control.ResourceManager;
+import org.apache.geode.cache.partition.PartitionRegionHelper;
+import org.apache.geode.cache.util.CacheListenerAdapter;
+import org.apache.geode.cache.util.ObjectSizer;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.test.dunit.Assert;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.SerializableCallable;
+import org.apache.geode.test.dunit.SerializableRunnable;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+
+/**
+ * A test of the when we will use the object sizer to determine the actual 
size of objects wrapped
+ * in CacheDeserializables.
+ * <p>
+ * <p>
+ * <p>
+ * TODO - I was intending to add tests that have an index and object sizer, 
but it appears we don't
+ * support indexes on regions with overflow to disk.

Review comment:
       This comment should be cleaned up a bit. The first sentence has some 
grammatical errors and the TODO should be removed.

##########
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/TestDelta.java
##########
@@ -62,10 +79,18 @@ public boolean hasDelta() {
     return hasDelta;
   }
 
+  @Override
+  public boolean getForceRecalculateSize() {
+    // new Exception("RINGLES - getForceRecalculateSize Called: " +
+    // forceRecalculateSize).printStackTrace();
+    return forceRecalculateSize;
+  }
+
   @Override
   public synchronized void toDelta(DataOutput out) throws IOException {
     // new Exception("DAN - To Delta Called").printStackTrace();

Review comment:
       This comment should also be removed.

##########
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/DeltaForceSizingFlagDUnitTest.java
##########
@@ -0,0 +1,1070 @@
+/*
+ * 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.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.File;
+import java.io.IOException;
+import java.util.Random;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.Test;
+
+import org.apache.geode.DataSerializable;
+import org.apache.geode.DataSerializer;
+import org.apache.geode.cache.AttributesFactory;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.InterestPolicy;
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.cache.SubscriptionAttributes;
+import org.apache.geode.cache.control.ResourceManager;
+import org.apache.geode.cache.partition.PartitionRegionHelper;
+import org.apache.geode.cache.util.CacheListenerAdapter;
+import org.apache.geode.cache.util.ObjectSizer;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.test.dunit.Assert;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.SerializableCallable;
+import org.apache.geode.test.dunit.SerializableRunnable;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+
+/**
+ * A test of the when we will use the object sizer to determine the actual 
size of objects wrapped
+ * in CacheDeserializables.
+ * <p>
+ * <p>
+ * <p>
+ * TODO - I was intending to add tests that have an index and object sizer, 
but it appears we don't
+ * support indexes on regions with overflow to disk.
+ */
+
+public class DeltaForceSizingFlagDUnitTest extends JUnit4CacheTestCase {

Review comment:
       JUnit4CacheTestCase is deprecated, so this test class should not extend 
it.

##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
##########
@@ -165,6 +165,9 @@
    */
   private byte[] deltaBytes = null;
 
+  /* Is this special about recalculating size? */

Review comment:
       Can this comment be removed?

##########
File path: geode-core/src/main/java/org/apache/geode/Delta.java
##########
@@ -23,27 +23,25 @@
  * This interface defines a contract between the application and GemFire that 
allows GemFire to
  * determine whether an application object contains a delta, allows GemFire to 
extract the delta
  * from an application object, and generate a new application object by 
applying a delta to an
- * existing application object. The difference in object state is contained in 
the
- * {@link DataOutput} and {@link DataInput} parameters.
+ * existing application object. The difference in object state is contained in 
the {@link
+ * DataOutput} and {@link DataInput} parameters.
  *
  * @since GemFire 6.1
- *
  */
 public interface Delta {
 
   /**
-   * Returns true if this object has pending changes it can write out as a 
delta.
-   * Returns false if this object must be transmitted in its entirety.
+   * Returns true if this object has pending changes it can write out as a 
delta. Returns false if
+   * this object must be transmitted in its entirety.
    */
   boolean hasDelta();
 
   /**
    * This method is invoked on an application object at the delta sender, if 
GemFire determines the
    * presence of a delta by calling {@link Delta#hasDelta()} on the object. 
The delta is written to
    * the {@link DataOutput} object provided by GemFire.
-   *
+   * <p>

Review comment:
       Is this a necessary addition?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to