agingade commented on a change in pull request #6075:
URL: https://github.com/apache/geode/pull/6075#discussion_r597887213
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/TxCallbackEventFactoryImpl.java
##########
@@ -127,13 +126,21 @@ public EntryEventImpl createCallbackEvent(final
InternalRegion internalRegion,
}
}
} else if (computeFilterInfo) { // not a bucket
- if (TxCallbackEventFactoryImpl.logger.isTraceEnabled()) {
- TxCallbackEventFactoryImpl.logger.trace("createCBEvent computing
routing for non-bucket");
- }
- FilterProfile fp = internalRegion.getFilterProfile();
- if (fp != null) {
- retVal.setLocalFilterInfo(fp.getLocalFilterRouting(retVal));
+ if (!isOriginRemote) {
Review comment:
Can both condition be replaced with AND condition. And move the comments
from bottom to top...
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java
##########
@@ -591,13 +593,14 @@ private void lockFilterRegistrationOnTxRegions() {
for (InternalRegion region : getRegions()) {
if (lockableRegionForFilterRegistration(region)) {
region.getFilterProfile().lockFilterRegistrationDuringTx();
+ filterLockAcquiredRegions.add(region);
}
}
}
private void unlockFilterRegistrationOnTxRegions() {
- for (InternalRegion region : getRegions()) {
- if (lockableRegionForFilterRegistration(region)) {
+ for (InternalRegion region : filterLockAcquiredRegions) {
+ if (region != null) {
Review comment:
Will this region be destroyed; do we need check for destroyed regions...
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java
##########
@@ -522,15 +522,15 @@ public void commit() throws CommitConflictException {
// Take filter registration lock
lockFilterRegistrationOnTxRegions();
try {
+ attachFilterProfileInformation(entries);
+
Review comment:
This will work with locking changes...But still feel that, the attach
should be called after applying to cache...
I believe the "applyChanges" is creating the CacheEvent/EntryEvent by not
setting the old value; in that case can we change that code to set the old
value....If the old value is not available in apply, can we get the old value
as we are doing in attach and pass it to apply?
--
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]
