[
https://issues.apache.org/jira/browse/GEODE-8964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17291317#comment-17291317
]
ASF GitHub Bot commented on GEODE-8964:
---------------------------------------
DonalEvans commented on a change in pull request #6053:
URL: https://github.com/apache/geode/pull/6053#discussion_r583301238
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/TXCommitMessage.java
##########
@@ -754,8 +754,14 @@ void firePendingCallbacks(List<EntryEventImpl> callbacks) {
ee.getRegion().invokeTXCallbacks(EnumListenerEvent.AFTER_CREATE, ee,
true,
isLastTransactionEvent);
} else {
- ee.getRegion().invokeTXCallbacks(EnumListenerEvent.AFTER_UPDATE, ee,
true,
- isLastTransactionEvent);
+ if (ee.getNewValue() == null) { // GEODE-8964, fixes GII and TX
create conflict that
Review comment:
Small nitpick, but could the comment here be put on its own line?
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/TXCommitMessageTest.java
##########
@@ -80,4 +81,48 @@ public void
firePendingCallbacksSetsChangeAppliedToCacheInEventLocalFilterInfo()
verify(filterInfo2).setChangeAppliedToCache(true);
}
+ @Test
+ public void
firePendingCallbacksSendsAFTER_CREATECallbackIfUpdateEntryEventHasNullNewValue()
{
+ TXCommitMessage message = spy(new TXCommitMessage());
+ LocalRegion region = mock(LocalRegion.class, RETURNS_DEEP_STUBS);
+ EntryEventImpl updateEvent = mock(EntryEventImpl.class,
RETURNS_DEEP_STUBS);
+ EntryEventImpl lastTxEvent = mock(EntryEventImpl.class,
RETURNS_DEEP_STUBS);
+
+ List<EntryEventImpl> callbacks = new ArrayList<>();
+ callbacks.add(updateEvent);
+ callbacks.add(lastTxEvent);
+
+ when(updateEvent.getLocalFilterInfo()).thenReturn(null);
+ when(updateEvent.getNewValue()).thenReturn(null);
+ when(updateEvent.getRegion()).thenReturn(region);
+
+ doReturn(lastTxEvent).when(message).getLastTransactionEvent(callbacks);
+
+ message.firePendingCallbacks(callbacks);
+
+ verify(region).invokeTXCallbacks(EnumListenerEvent.AFTER_CREATE,
updateEvent, true, false);
Review comment:
The two test cases here can be condensed to one if this line is changed
to `verify(region, only()).invokeTXCallbacks(EnumListenerEvent.AFTER_CREATE,
updateEvent, true, false);` which will validate both that `invokeTXCallbacks()`
is called with the correct arguments and that it is *only* called with those
arguments, removing the need for a separate test to confirm that it's not
called with the `AFTER_UPDATE` argument.
----------------------------------------------------------------
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]
> Entry created via TX putIfAbsent with null value can write incorrect data to
> cache
> ----------------------------------------------------------------------------------
>
> Key: GEODE-8964
> URL: https://issues.apache.org/jira/browse/GEODE-8964
> Project: Geode
> Issue Type: Bug
> Components: client/server
> Affects Versions: 1.10.0, 1.14.0
> Reporter: Louis R. Jacome
> Assignee: Louis R. Jacome
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.15.0
>
>
> In backwards compatibility testing, an error was seen wherein a putIfAbsent
> with a null value was being processed by a server but interrupted by a GII
> with the same key. The operation was subsequently enqueued in the client's HA
> Region Queue as an update with a null value rather than a create with a null
> value -- a path that's not supported -- resulting in erroneous data sometimes
> being written to the client cache.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
