sabbey37 commented on a change in pull request #6296:
URL: https://github.com/apache/geode/pull/6296#discussion_r627521495
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
Review comment:
We already have the class variable `reflectionObjectSizer` that we could
use instead of getting another instance.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisHashTest.java
##########
@@ -258,11 +261,380 @@ public void
hscanSnaphots_shouldExpireAfterExpiryPeriod() {
});
}
+ /************* Hash Size *************/
+ /******* constants *******/
+ @Test
+ public void constantBaseRedisHashOverhead_shouldEqualCalculatedOverhead() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisHashOverhead = reflectionObjectSizer.sizeof(new RedisHash());
+
+ assertThat(baseRedisHashOverhead).isEqualTo(BASE_REDIS_HASH_OVERHEAD);
+ }
Review comment:
This test is the same as
`should_calculateSize_equalToROSSize_ofEmptyIndividualInstance()`. Do we need
both? Also, it looks like we already have the class variable
`reflectionObjectSizer` that we could use instead of getting another instance.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisSetOverhead = reflectionObjectSizer.sizeof(new RedisSet()) +
18;
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ int base_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
Review comment:
This could be done in one line so you wouldn't need the `temp_hashset`
variable. Also, not sure why we switched to snake case for the variable name?
Could we change it to `baseHashSetSize` for consistency?
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java
##########
@@ -351,4 +354,95 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ /************* Size in Bytes Tests *************/
+ /******* constructors *******/
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofLargeStrings() {
+ String javaString = makeStringOfSpecifiedSize(10_000);
+ RedisString string = new RedisString(new
ByteArrayWrapper(javaString.getBytes()));
+
+ int actual = string.getSizeInBytes();
+ int expected = reflectionObjectSizer.sizeof(string);
+
+ assertThat(actual).isEqualTo(expected);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofStringOfVariousSizes() {
Review comment:
This title could be `equalToROSSize` since we expect exact equality.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisSetOverhead = reflectionObjectSizer.sizeof(new RedisSet()) +
18;
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ int base_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+ baseRedisSetOverhead += base_hashset_size;
+
+ assertThat(baseRedisSetOverhead).isEqualTo(BASE_REDIS_SET_OVERHEAD);
+ }
+
+ @Test
+ public void perMemberOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ ByteArrayWrapper member1 = new ByteArrayWrapper("a".getBytes());
+ ByteArrayWrapper member2 = new ByteArrayWrapper("b".getBytes());
+ temp_hashset.add(member1);
+ int one_entry_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ temp_hashset.add(member2);
+ int two_entries_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ int perMemberOverhead = two_entries_hashset_size - one_entry_hashset_size
+ 5;
Review comment:
Not sure why we switched to snake case for these variable names? Could
we change it to camel case for consistency?
A comment explaining the `+5` would be good.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisSetOverhead = reflectionObjectSizer.sizeof(new RedisSet()) +
18;
Review comment:
I'm wondering how we came up with `18` here? It is close to the size of
an empty `ByteArrayWrapper` (which is 16)... not sure what the additional 2
would be... maybe padding? Either way, a comment would be good explaining it.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisSetOverhead = reflectionObjectSizer.sizeof(new RedisSet()) +
18;
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ int base_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+ baseRedisSetOverhead += base_hashset_size;
+
+ assertThat(baseRedisSetOverhead).isEqualTo(BASE_REDIS_SET_OVERHEAD);
+ }
+
+ @Test
+ public void perMemberOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ ByteArrayWrapper member1 = new ByteArrayWrapper("a".getBytes());
+ ByteArrayWrapper member2 = new ByteArrayWrapper("b".getBytes());
+ temp_hashset.add(member1);
+ int one_entry_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ temp_hashset.add(member2);
+ int two_entries_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ int perMemberOverhead = two_entries_hashset_size - one_entry_hashset_size
+ 5;
+
+ assertThat(perMemberOverhead).isEqualTo(PER_MEMBER_OVERHEAD);
+ }
+
+ @Test
+ public void
internalHashsetStorageOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ int base_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ ByteArrayWrapper baw1 = new ByteArrayWrapper("a".getBytes());
+ ByteArrayWrapper baw2 = new ByteArrayWrapper("b".getBytes());
+
+ temp_hashset.add(baw1);
+ temp_hashset.add(baw2);
+
+ int two_entries_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ int internalHashsetStorageOverhead =
+ two_entries_hashset_size - (2 * PER_MEMBER_OVERHEAD) -
base_hashset_size;
+
+
assertThat(internalHashsetStorageOverhead).isEqualTo(INTERNAL_HASH_SET_STORAGE_OVERHEAD);
Review comment:
Not sure why we switched to snake case for these variable names? Could
we change it to camel case for consistency?
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisSetOverhead = reflectionObjectSizer.sizeof(new RedisSet()) +
18;
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ int base_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+ baseRedisSetOverhead += base_hashset_size;
+
+ assertThat(baseRedisSetOverhead).isEqualTo(BASE_REDIS_SET_OVERHEAD);
+ }
+
+ @Test
+ public void perMemberOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ ByteArrayWrapper member1 = new ByteArrayWrapper("a".getBytes());
+ ByteArrayWrapper member2 = new ByteArrayWrapper("b".getBytes());
+ temp_hashset.add(member1);
+ int one_entry_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ temp_hashset.add(member2);
+ int two_entries_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+
+ int perMemberOverhead = two_entries_hashset_size - one_entry_hashset_size
+ 5;
+
+ assertThat(perMemberOverhead).isEqualTo(PER_MEMBER_OVERHEAD);
+ }
+
+ @Test
+ public void
internalHashsetStorageOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
Review comment:
We already have the class variable `reflectionObjectSizer` that we could
use instead of getting another instance.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSetTest.java
##########
@@ -165,4 +174,142 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberCounts() {
+ for (int i = 0; i < 1024; i += 16) {
+ RedisSet set = createRedisSetOfSpecifiedSize(i);
+
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROS_withVaryingMemberSize() {
+ for (int i = 0; i < 16; i++) {
+ RedisSet set = createRedisSetWithMemberOfSpecifiedSize(i * 64);
+ int expected = reflectionObjectSizer.sizeof(set);
+ Long actual = Long.valueOf(set.getSizeInBytes());
+ Offset<Long> offset = Offset.offset(Math.round(expected * 0.06));
+
+ assertThat(actual).isCloseTo(expected, offset);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void size_shouldDecrease_WhenValueIsRemoved() {
+ Region region = mock(Region.class);
+ final RedisData returnData = mock(RedisData.class);
+ when(region.put(Object.class, Object.class)).thenReturn(returnData);
+ final RedisKey key = new RedisKey("key".getBytes());
+ final ByteArrayWrapper value = new ByteArrayWrapper("value".getBytes());
+
+ RedisSet set = createRedisSetOfSpecifiedSize(10);
+
+ ArrayList<ByteArrayWrapper> members = new ArrayList<>();
+ members.add(value);
+ set.sadd(members, region, key);
+
+ int initialSize = set.getSizeInBytes();
+
+ set.srem(members, region, key);
+
+ int finalSize = set.getSizeInBytes();
+ int expectedSize = initialSize - value.length() - PER_MEMBER_OVERHEAD;
+
+ assertThat(finalSize).isEqualTo(expectedSize);
+ }
+
+ /******** constants *******/
+ @Test
+ public void baseOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
+ int baseRedisSetOverhead = reflectionObjectSizer.sizeof(new RedisSet()) +
18;
+
+ HashSet<ByteArrayWrapper> temp_hashset = new HashSet<>();
+ int base_hashset_size = reflectionObjectSizer.sizeof(temp_hashset);
+ baseRedisSetOverhead += base_hashset_size;
+
+ assertThat(baseRedisSetOverhead).isEqualTo(BASE_REDIS_SET_OVERHEAD);
+ }
+
+ @Test
+ public void perMemberOverheadConstant_shouldMatchCalculatedValue() {
+ ReflectionObjectSizer reflectionObjectSizer =
ReflectionObjectSizer.getInstance();
Review comment:
We already have the class variable `reflectionObjectSizer` that we could
use instead of getting another instance.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java
##########
@@ -351,4 +354,95 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ /************* Size in Bytes Tests *************/
+ /******* constructors *******/
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofLargeStrings() {
Review comment:
This title could be `equalToROSSize` since we expect exact equality.
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java
##########
@@ -351,4 +354,95 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ /************* Size in Bytes Tests *************/
+ /******* constructors *******/
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofLargeStrings() {
+ String javaString = makeStringOfSpecifiedSize(10_000);
+ RedisString string = new RedisString(new
ByteArrayWrapper(javaString.getBytes()));
+
+ int actual = string.getSizeInBytes();
+ int expected = reflectionObjectSizer.sizeof(string);
+
+ assertThat(actual).isEqualTo(expected);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofStringOfVariousSizes() {
+ String javaString;
+ for (int i = 0; i < 512; i += 8) {
+ javaString = makeStringOfSpecifiedSize(i);
+ RedisString string = new RedisString(new
ByteArrayWrapper(javaString.getBytes()));
+
+ int expected = reflectionObjectSizer.sizeof(string);
+ int actual = string.getSizeInBytes();
+
+ assertThat(actual).isEqualTo(expected);
+ }
+ }
+
+ /******* changing values *******/
+ @Test
+ public void changingStringValue_toShorterString_shouldDecreaseSizeInBytes() {
+ String baseString = "baseString";
+ RedisString string = new RedisString(new ByteArrayWrapper((baseString +
"asdf").getBytes()));
+
+ int initialSize = string.getSizeInBytes();
+
+ string.set(new ByteArrayWrapper(baseString.getBytes()));
+
+ int finalSize = string.getSizeInBytes();
+
+ assertThat(finalSize).isEqualTo(initialSize - 4);
+ }
+
+ @Test
+ public void changingStringValue_toLongerString_shouldIncreaseSizeInBytes() {
+ String baseString = "baseString";
+ RedisString string = new RedisString(new
ByteArrayWrapper(baseString.getBytes()));
+
+ int initialSize = string.getSizeInBytes();
+
+ string.set(new ByteArrayWrapper((baseString + "asdf").getBytes()));
+
+ int finalSize = string.getSizeInBytes();
+
+ assertThat(finalSize).isEqualTo(initialSize + 4);
+ }
+
+ @Test
+ public void
changingStringValue_toEmptyString_shouldDecreaseSizeInBytes_toBaseSize() {
+ String baseString = "baseString";
+ RedisString string = new RedisString(new ByteArrayWrapper((baseString +
"asdf").getBytes()));
+
+ string.set(new ByteArrayWrapper("".getBytes()));
+
+ int finalSize = string.getSizeInBytes();
+
+ assertThat(finalSize).isEqualTo(BASE_REDIS_STRING_OVERHEAD);
+ }
+
+ /******* constants *******/
+ @Test
+ public void overheadConstants_shouldMatchCalculatedValue() {
+ assertThat(RedisString.PER_OBJECT_OVERHEAD).isEqualTo(8); // see todo in
RedisString
Review comment:
I'm a little confused about why we're checking this since we don't use
the `PER_OBJECT_OVERHEAD` at all to calculate the size of `RedisString`. Is
the todo mentioned here this one: `TODO add delta support` ?
##########
File path:
geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/data/PartitionedRegionStatsUpdateTest.java
##########
@@ -0,0 +1,396 @@
+/*
+ * 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.redis.internal.data;
+
+import static
org.apache.geode.distributed.ConfigurationProperties.MAX_WAIT_TIME_RECONNECT;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Ignore;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+
+public class PartitionedRegionStatsUpdateTest {
+
+ @ClassRule
+ public static RedisClusterStartupRule clusterStartUpRule = new
RedisClusterStartupRule(3);
+
+ private static MemberVM server1;
+ private static MemberVM server2;
+
+ private static Jedis jedis1;
+ private static Jedis jedis2;
+
+ private static final int JEDIS_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ private static final String LOCAL_HOST = "127.0.0.1";
+ public static final String STRING_KEY = "string key";
+ public static final String SET_KEY = "set key";
+ public static final String HASH_KEY = "hash key";
+ public static final String LONG_APPEND_VALUE =
String.valueOf(Integer.MAX_VALUE);
+ public static final String FIELD = "field";
+
+ @BeforeClass
+ public static void classSetup() {
+ Properties locatorProperties = new Properties();
+ locatorProperties.setProperty(MAX_WAIT_TIME_RECONNECT, "15000");
+
+ MemberVM locator = clusterStartUpRule.startLocatorVM(0, locatorProperties);
+ int locatorPort = locator.getPort();
+
+ server1 = clusterStartUpRule.startRedisVM(1, locatorPort);
+ int redisServerPort1 = clusterStartUpRule.getRedisPort(1);
+ jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+
+ server2 = clusterStartUpRule.startRedisVM(2, locatorPort);
+ int redisServerPort2 = clusterStartUpRule.getRedisPort(1);
+ jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+ }
+
+ @Before
+ public void setup() {
+ jedis1.flushAll();
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenStringValueSizeIncreases() {
+ String LONG_APPEND_VALUE = String.valueOf(Integer.MAX_VALUE);
+ jedis1.set(STRING_KEY, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.append(STRING_KEY, LONG_APPEND_VALUE);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showDecreaseInDatastoreBytesInUse_givenStringValueDeleted() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.set(STRING_KEY, "value");
+
+ long intermediateDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
assertThat(intermediateDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+
+ jedis1.del(STRING_KEY);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showDecreaseInDatastoreBytesInUse_givenStringValueShortened() {
+ jedis1.set(STRING_KEY, "longer value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.set(STRING_KEY, "value");
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isLessThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void should_resetMemoryUsage_givenFlushAllCommand() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(initialDataStoreBytesInUse).isEqualTo(0L);
+
+ jedis1.set(STRING_KEY, "value");
+
+ jedis1.flushAll();
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenStringValueSizeDoesNotIncrease()
{
+ jedis1.set(STRING_KEY, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.set(STRING_KEY, "value");
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenSetValueSizeIncreases() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.sadd(SET_KEY, "value" + i);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenSetValueSizeDoesNotIncrease() {
+ jedis1.sadd(SET_KEY, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.sadd(SET_KEY, "value");
+ }
+
+ assertThat(jedis1.scard(SET_KEY)).isEqualTo(1);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void should_showDecreaseInDatastoreBytesInUse_givenSetValueDeleted() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.sadd(SET_KEY, "value");
+
+ long intermediateDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
assertThat(intermediateDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+
+ jedis1.del(SET_KEY);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showDecreaseInDatastoreBytesInUse_givenSetValueSizeDecreases() {
+ for (int i = 0; i < 10; i++) {
+ jedis1.sadd(SET_KEY, "value" + i);
+ }
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 10; i++) {
+ jedis1.srem(SET_KEY, "value" + i);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isLessThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenHashValueSizeIncreases() {
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.hset(HASH_KEY, FIELD + i, LONG_APPEND_VALUE);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void should_showDecreaseInDatastoreBytesInUse_givenHashValueDeleted()
{
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long intermediateDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
assertThat(intermediateDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+
+ jedis1.del(HASH_KEY);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenHSetDoesNotIncreaseHashSize() {
+ jedis2.hset(HASH_KEY, FIELD, "initialvalue"); // two hsets are required to
force
+ jedis2.hset(HASH_KEY, FIELD, "value"); // deserialization on both servers
+ // otherwise primary/secondary can disagree on size, and which server is
primary varies
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ for (int i = 0; i < 10; i++) {
+ jedis2.hset(HASH_KEY, FIELD, "value");
+ }
+
+ assertThat(jedis2.hgetAll(HASH_KEY).size()).isEqualTo(1);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenHSetNXIncreasesHashSize() {
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.hsetnx(HASH_KEY, FIELD + i, "value");
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenHSetNXDoesNotIncreaseHashSize()
{
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.hsetnx(HASH_KEY, FIELD, "value");
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ /******* confirm that the other member agrees upon size *******/
+
+ @Test
+ public void
should_showMembersAgreeUponUsedHashMemory_afterDeltaPropagation() {
+ jedis1.hset(HASH_KEY, FIELD, "initialvalue"); // two hsets are required to
force
+ jedis1.hset(HASH_KEY, FIELD, "finalvalue"); // deserialization on both
servers
+ // otherwise primary/secondary can disagree on size, and which server is
primary varies
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ for (int i = 0; i < 10; i++) {
+ jedis1.hset(HASH_KEY, FIELD, "finalvalue");
+ }
+
+ assertThat(jedis1.hgetAll(HASH_KEY).size()).isEqualTo(1);
+
+ long server2FinalDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+ long server1FinalDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(server1FinalDataStoreBytesInUse)
+ .isEqualTo(server2FinalDataStoreBytesInUse)
+ .isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ @Ignore("confirm that bucket size stats are being calculated correctly
before enabling")
+ public void should_showMembersAgreeUponUsedSetMemory_afterDeltaPropagation()
{
+ jedis1.sadd(SET_KEY, "other"); // two sadds are required to force
+ jedis1.sadd(SET_KEY, "value"); // deserialization on both servers
+ // otherwise primary/secondary can disagree on size, and which server is
primary varies
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ for (int i = 0; i < 10; i++) {
+ jedis1.sadd(SET_KEY, "value");
+ }
+
+ assertThat(jedis1.scard(SET_KEY)).isEqualTo(2);
+
+ long server1FinalDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+ long server2FinalDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ assertThat(server1FinalDataStoreBytesInUse)
+ .isEqualTo(server2FinalDataStoreBytesInUse)
+ .isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ @Ignore("confirm that bucket size stats are being calculated correctly
before enabling")
+ public void
should_showMembersAgreeUponUsedStringMemory_afterDeltaPropagation() {
Review comment:
Could we add the Jira ticket to the `@Ignore` comment? Like
`@Ignore("GEODE-9243: Force deserialization on Radish members")`
##########
File path:
geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/data/PartitionedRegionStatsUpdateTest.java
##########
@@ -0,0 +1,396 @@
+/*
+ * 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.redis.internal.data;
+
+import static
org.apache.geode.distributed.ConfigurationProperties.MAX_WAIT_TIME_RECONNECT;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Ignore;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+
+public class PartitionedRegionStatsUpdateTest {
+
+ @ClassRule
+ public static RedisClusterStartupRule clusterStartUpRule = new
RedisClusterStartupRule(3);
+
+ private static MemberVM server1;
+ private static MemberVM server2;
+
+ private static Jedis jedis1;
+ private static Jedis jedis2;
+
+ private static final int JEDIS_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ private static final String LOCAL_HOST = "127.0.0.1";
+ public static final String STRING_KEY = "string key";
+ public static final String SET_KEY = "set key";
+ public static final String HASH_KEY = "hash key";
+ public static final String LONG_APPEND_VALUE =
String.valueOf(Integer.MAX_VALUE);
+ public static final String FIELD = "field";
+
+ @BeforeClass
+ public static void classSetup() {
+ Properties locatorProperties = new Properties();
+ locatorProperties.setProperty(MAX_WAIT_TIME_RECONNECT, "15000");
+
+ MemberVM locator = clusterStartUpRule.startLocatorVM(0, locatorProperties);
+ int locatorPort = locator.getPort();
+
+ server1 = clusterStartUpRule.startRedisVM(1, locatorPort);
+ int redisServerPort1 = clusterStartUpRule.getRedisPort(1);
+ jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+
+ server2 = clusterStartUpRule.startRedisVM(2, locatorPort);
+ int redisServerPort2 = clusterStartUpRule.getRedisPort(1);
+ jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+ }
+
+ @Before
+ public void setup() {
+ jedis1.flushAll();
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenStringValueSizeIncreases() {
+ String LONG_APPEND_VALUE = String.valueOf(Integer.MAX_VALUE);
+ jedis1.set(STRING_KEY, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.append(STRING_KEY, LONG_APPEND_VALUE);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showDecreaseInDatastoreBytesInUse_givenStringValueDeleted() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.set(STRING_KEY, "value");
+
+ long intermediateDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
assertThat(intermediateDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+
+ jedis1.del(STRING_KEY);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showDecreaseInDatastoreBytesInUse_givenStringValueShortened() {
+ jedis1.set(STRING_KEY, "longer value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.set(STRING_KEY, "value");
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isLessThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void should_resetMemoryUsage_givenFlushAllCommand() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(initialDataStoreBytesInUse).isEqualTo(0L);
+
+ jedis1.set(STRING_KEY, "value");
+
+ jedis1.flushAll();
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenStringValueSizeDoesNotIncrease()
{
+ jedis1.set(STRING_KEY, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.set(STRING_KEY, "value");
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenSetValueSizeIncreases() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.sadd(SET_KEY, "value" + i);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenSetValueSizeDoesNotIncrease() {
+ jedis1.sadd(SET_KEY, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.sadd(SET_KEY, "value");
+ }
+
+ assertThat(jedis1.scard(SET_KEY)).isEqualTo(1);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void should_showDecreaseInDatastoreBytesInUse_givenSetValueDeleted() {
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.sadd(SET_KEY, "value");
+
+ long intermediateDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
assertThat(intermediateDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+
+ jedis1.del(SET_KEY);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showDecreaseInDatastoreBytesInUse_givenSetValueSizeDecreases() {
+ for (int i = 0; i < 10; i++) {
+ jedis1.sadd(SET_KEY, "value" + i);
+ }
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 10; i++) {
+ jedis1.srem(SET_KEY, "value" + i);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isLessThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenHashValueSizeIncreases() {
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.hset(HASH_KEY, FIELD + i, LONG_APPEND_VALUE);
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void should_showDecreaseInDatastoreBytesInUse_givenHashValueDeleted()
{
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long intermediateDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
assertThat(intermediateDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+
+ jedis1.del(HASH_KEY);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenHSetDoesNotIncreaseHashSize() {
+ jedis2.hset(HASH_KEY, FIELD, "initialvalue"); // two hsets are required to
force
+ jedis2.hset(HASH_KEY, FIELD, "value"); // deserialization on both servers
+ // otherwise primary/secondary can disagree on size, and which server is
primary varies
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ for (int i = 0; i < 10; i++) {
+ jedis2.hset(HASH_KEY, FIELD, "value");
+ }
+
+ assertThat(jedis2.hgetAll(HASH_KEY).size()).isEqualTo(1);
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showIncreaseInDatastoreBytesInUse_givenHSetNXIncreasesHashSize() {
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.hsetnx(HASH_KEY, FIELD + i, "value");
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+
assertThat(finalDataStoreBytesInUse).isGreaterThan(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ public void
should_showNoIncreaseInDatastoreBytesInUse_givenHSetNXDoesNotIncreaseHashSize()
{
+ jedis1.hset(HASH_KEY, FIELD, "value");
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ for (int i = 0; i < 1000; i++) {
+ jedis1.hsetnx(HASH_KEY, FIELD, "value");
+ }
+
+ long finalDataStoreBytesInUse =
clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(finalDataStoreBytesInUse).isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ /******* confirm that the other member agrees upon size *******/
+
+ @Test
+ public void
should_showMembersAgreeUponUsedHashMemory_afterDeltaPropagation() {
+ jedis1.hset(HASH_KEY, FIELD, "initialvalue"); // two hsets are required to
force
+ jedis1.hset(HASH_KEY, FIELD, "finalvalue"); // deserialization on both
servers
+ // otherwise primary/secondary can disagree on size, and which server is
primary varies
+
+ long initialDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+
+ for (int i = 0; i < 10; i++) {
+ jedis1.hset(HASH_KEY, FIELD, "finalvalue");
+ }
+
+ assertThat(jedis1.hgetAll(HASH_KEY).size()).isEqualTo(1);
+
+ long server2FinalDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server2);
+ long server1FinalDataStoreBytesInUse =
+ clusterStartUpRule.getDataStoreBytesInUseForDataRegion(server1);
+
+ assertThat(server1FinalDataStoreBytesInUse)
+ .isEqualTo(server2FinalDataStoreBytesInUse)
+ .isEqualTo(initialDataStoreBytesInUse);
+ }
+
+ @Test
+ @Ignore("confirm that bucket size stats are being calculated correctly
before enabling")
+ public void should_showMembersAgreeUponUsedSetMemory_afterDeltaPropagation()
{
Review comment:
Could we add the Jira ticket to the `@Ignore` comment? Like
`@Ignore("GEODE-9243: Force deserialization on Radish members")`
--
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]
