dschneider-pivotal commented on a change in pull request #5978:
URL: https://github.com/apache/geode/pull/5978#discussion_r569626096
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
##########
@@ -165,6 +165,9 @@
*/
private byte[] deltaBytes = null;
+ /** If true, causes deltas to trigger recalculation of bucket size **/
+ private boolean forceRecalculateSize = false;
Review comment:
I'd like to see this new variable removed and instead, in the two places
we use it, call delta.getForceRecalculateSize().
You can easily do this on line 1722 because it had just done (on line 1718)
a test that v is an instanceof Delta. So add the next line: Delta delta =
(Delta)v;
On line 1865 the delta instance is the "value" local variable. You can see
this on line 1841 when this code casts "value" to Delta. So I'd recommend you
add a local variable at 1841 like so: Delta delta = (Delta)value;
and then use that on 1865 to decide if we should recalculate.
Having the extra state on EntryEventImpl adds complexity and in the future
it could get out of sync with Delta.getForceRecalculateSize(). By just calling
this new method I think the code will be better.
##########
File path: geode-core/src/main/java/org/apache/geode/Delta.java
##########
@@ -53,7 +51,13 @@
* This method throws an {@link InvalidDeltaException} when the delta in the
{@link DataInput}
* cannot be applied to the object. GemFire automatically handles an {@link
InvalidDeltaException}
* by reattempting the update by sending the full application object.
- *
*/
void fromDelta(DataInput in) throws IOException, InvalidDeltaException;
+
+ /**
+ * Allows Delta implementations to ensure bucket sizes are recalculated
after delta is applied
Review comment:
Since this method is part of the external API improve these javadocs.
1. Add a @since 1.14
2. Explain what geode does for delta size by default (it is calculated once
when the entry is first created and any changes in actual size will not be
noticed by geode. This can cause the statistics that report memory use to not
be accurate), why this default was chosen (to optimize the use case in which
the actual size of the object implementing Delta does not change), how to
override this default behavior (implement this method in your class that
implements Delta and have it return true), and explain why you would want to do
that (if your Delta causes the actual size of the instance implementing Delta
to change). It would also be good to point out that changing this default
behavior to recalculate may hurt performance.
----------------------------------------------------------------
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]
