> On May 5, 2016, 9:13 p.m., Dan Smith wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java, > > line 1151 > > <https://reviews.apache.org/r/46243/diff/2/?file=1370375#file1370375line1151> > > > > We're not generating event ids if there is an async event queue? What > > is this code for? > > > > It seems like this check ought to be extracted into it's own method.
The destroy event for expiration on PR wasn't setting eventId which was causing NPE in the code: at com.gemstone.gemfire.cache.asyncqueue.internal.SerialAsyncEventQueueImpl.setModifiedEventId(SerialAsyncEventQueueImpl.java:243) Earlier this was not an issue, as we were not sending destroy events to AEQs. The eventId is getting set based on the call to generateEventID()...the change was made to retrun true when there is an AEQ on the bucket region. As we spoke I will remove the check for bucketRegion and allow it to generated event id in all cases except for admin region. > On May 5, 2016, 9:13 p.m., Dan Smith wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java, > > line 6645 > > <https://reviews.apache.org/r/46243/diff/2/?file=1370375#file1370375line6645> > > > > Are you sure there aren't another internal regions that ought to notify > > the gateway? Why did you add this check? > > > > THis might read cleaner if you had a separate if for each check with a > > comment explaining why it's skipped. This was to filter out events from internal region, in the begining itself...So that its clean and easy to debug in the later part of the code. I observed there were lot of events coming from monitoring region. I will remove the check for internal region. > On May 5, 2016, 9:13 p.m., Dan Smith wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java, > > line 6650 > > <https://reviews.apache.org/r/46243/diff/2/?file=1370375#file1370375line6650> > > > > Remove this log message. Sorry, missed removing log messages before sending it for review. > On May 5, 2016, 9:13 p.m., Dan Smith wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java, > > line 857 > > <https://reviews.apache.org/r/46243/diff/2/?file=1370376#file1370376line857> > > > > Remove this log message. Sorry, missed removing log messages before sending it for review. > On May 5, 2016, 9:13 p.m., Dan Smith wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java, > > line 865 > > <https://reviews.apache.org/r/46243/diff/2/?file=1370376#file1370376line865> > > > > I wonder if there are use cases where we would want to pass these > > events to the WAN? Is there a reason we need is AEQ check here? By talking to barry, it seems its a design choice not send the local operation over the wan (or propogate it to the remote cluster)...And thinking about it, its valid option as the cluster setups/configuration/requirements could be different. > On May 5, 2016, 9:13 p.m., Dan Smith wrote: > > geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java, > > line 63 > > <https://reviews.apache.org/r/46243/diff/2/?file=1370383#file1370383line63> > > > > This is not a unit test if it's creating a cache. Should be marked as > > an integration test. You are right, i will change this as integration test. - anilkumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46243/#review131925 ----------------------------------------------------------- On May 6, 2016, 12:45 a.m., anilkumar gingade wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46243/ > ----------------------------------------------------------- > > (Updated May 6, 2016, 12:45 a.m.) > > > Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, > Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou. > > > Repository: geode > > > Description > ------- > > GEODE-1209: Added new attribute to forward eviction/expiration to AEQ. Tested > with manual testing. > > Sending the changes for early feedback on attribute/method names... > > Need to add support for xml, gfsh...And new tests... > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueue.java > a2b8b0f62333082e8eaf87354c0a2700f7428608 > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueFactory.java > 3e30b3847c2cd1c4fc72f342f0898a1e56cbce2e > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/AsyncEventQueueFactoryImpl.java > 312e880251cb5fdcfb79b81d888ec5a85649d9da > > geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/AsyncEventQueueImpl.java > 6b3eb4a827add8c8551bcd095eba0ab69a74dbd4 > geode-core/src/main/java/com/gemstone/gemfire/cache/wan/GatewaySender.java > c5b5d3aca14000e82b25e0eb156f789f7e1fc753 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java > 3ad294c10463b87ed9685b4c7f767523d0a52335 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java > fe09d031c7e326b4eb0cc3f63700c282b734b842 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderAttributes.java > 1cef940f8ebacb22cd3b08513ea3083ecd1df909 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/AsyncEventQueueCreation.java > 001566504db6eb2828cdcc3eba824e90d1df70e3 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXml.java > aa7d49a654fcc0c7d2419546f4b4b5e1c4444271 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXmlGenerator.java > ea3c975267b3b05cd1e9350ca464aba70e459113 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheXmlParser.java > f344938350a1043655f27f75eb7a9cbf9246ac30 > > geode-core/src/main/resources/META-INF/schemas/geode.apache.org/schema/cache/cache-1.0.xsd > cc6d189e7fb4ed05f7bf35a9e43bafc8dc53056f > > geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/cache30/CacheXmlGeode10DUnitTest.java > 57e3a13b320e302f4be4ff40aa976eabd929435a > > Diff: https://reviews.apache.org/r/46243/diff/ > > > Testing > ------- > > Did manual testing with AEQ with and without eviction and expiration. > > > Thanks, > > anilkumar gingade > >
