Re: Review Request 46243: GEODE-1209: Added new attribute to forward eviction/expiration to AEQ. Tested with manual testing.

[anilkumar gingade]

> 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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/ge

Re: Review Request 46243: GEODE-1209: Added new attribute to forward eviction/expiration to AEQ. Tested with manual testing.

[anilkumar gingade]

---
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.


Changes
---

Added changes based on the review comments. Also adding Darrel for review.


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 (updated)
-

  
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
 aa7d49a654fcc0c7d2419546f4b4b5e1c271 
  
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

Re: Review Request 46243: GEODE-1209: Added new attribute to forward eviction/expiration to AEQ. Tested with manual testing.

[anilkumar gingade]

> On April 19, 2016, 12:18 a.m., Dan Smith wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java,
> >  line 6727
> > 
> >
> > What is this check for for isInternalRegion for?

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 April 19, 2016, 12:18 a.m., Dan Smith wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java,
> >  line 870
> > 
> >
> > Is looks like in the old code this isLocal check was commented out?

isLocal check was done in LocalRegion.notifyGateWaySender()...To keep the logic 
simple, in one place i moved this check as part of checkForDistribution().


- anilkumar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46243/#review129453
---


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
>  aa7d49a654fcc0c7d2419546f4b4b5e1c271 
>   
> 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
> 
>

Re: Review Request 47046: The test code needs to be enhanced

[Dan Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47046/#review131976
---


Ship it!




Ship It!

- Dan Smith


On May 5, 2016, 11:58 p.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47046/
> ---
> 
> (Updated May 5, 2016, 11:58 p.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: geode-1183
> https://issues.apache.org/jira/browse/geode-1183
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The test code is not mature enough. Sometimes it will fail when the primary 
> is not yet setPrimary().
> 
> 
> Diffs
> -
> 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/Simple2CacheServerDUnitTest.java
>  684660b 
> 
> Diff: https://reviews.apache.org/r/47046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Re: Review Request 47046: The test code needs to be enhanced

[xiaojian zhou]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47046/
---

(Updated May 5, 2016, 11:58 p.m.)


Review request for geode and Dan Smith.


Changes
---

a new version of fix


Bugs: geode-1183
https://issues.apache.org/jira/browse/geode-1183


Repository: geode


Description
---

The test code is not mature enough. Sometimes it will fail when the primary is 
not yet setPrimary().


Diffs (updated)
-

  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/Simple2CacheServerDUnitTest.java
 684660b 

Diff: https://reviews.apache.org/r/47046/diff/


Testing
---


Thanks,

xiaojian zhou

Re: Review Request 47046: The test code needs to be enhanced

[Dan Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47046/#review131963
---



Wait is deprecated. Use Awaitility.

And, looking it your code, I don't think it will ever recheck the condition. 
It's only going to set the vm0_proxy, vm1_proxy fields once.

- Dan Smith


On May 5, 2016, 11:04 p.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47046/
> ---
> 
> (Updated May 5, 2016, 11:04 p.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: geode-1183
> https://issues.apache.org/jira/browse/geode-1183
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The test code is not mature enough. Sometimes it will fail when the primary 
> is not yet setPrimary().
> 
> 
> Diffs
> -
> 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/Simple2CacheServerDUnitTest.java
>  684660b 
> 
> Diff: https://reviews.apache.org/r/47046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Review Request 47046: The test code needs to be enhanced

[xiaojian zhou]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47046/
---

Review request for geode and Dan Smith.


Bugs: geode-1183
https://issues.apache.org/jira/browse/geode-1183


Repository: geode


Description
---

The test code is not mature enough. Sometimes it will fail when the primary is 
not yet setPrimary().


Diffs
-

  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/Simple2CacheServerDUnitTest.java
 684660b 

Diff: https://reviews.apache.org/r/47046/diff/


Testing
---


Thanks,

xiaojian zhou

Passed: apache/incubator-geode#474 (develop - 2da99e7)

[Travis CI]

Build Update for apache/incubator-geode
-

Build: #474
Status: Passed

Duration: 13 minutes and 33 seconds
Commit: 2da99e7 (develop)
Author: Bruce Schuchardt
Message: Revert "GEODE-613 CI Failure: 
LocatorLoadSnapshotJUnitTest.testConcurrentBalancing"

This reverts commit f788d698e877b15570052f89b9d0b5ab23023f81.

This commit accidentally included an unrelated change that broke the build.

View the changeset: 
https://github.com/apache/incubator-geode/compare/f788d698e877...2da99e76593d

View the full build log and details: 
https://travis-ci.org/apache/incubator-geode/builds/128170948

--

You can configure recipients for build notifications in your .travis.yml file. 
See https://docs.travis-ci.com/user/notifications

Re: JIRA labels for newbies

[William Markito]

Or better yet - project = GEODE and status in (Open) and labels in
(starter, gsoc2016, low-hanging-fruit, newbie, easyfix)  -  Total of 15
items...

On Thu, May 5, 2016 at 3:03 PM, William Markito  wrote:

> You may find a couple of more interesting items on this query -  project =
> GEODE and status in (Open) and labels in (starter, gsoc2016,
> low-hanging-fruit)
>
>
> https://issues.apache.org/jira/browse/GEODE-1113?jql=project%20%3D%20GEODE%20and%20status%20in%20(Open)%20and%20labels%20in%20(starter%2C%20gsoc2016%2C%20low-hanging-fruit)
>
> But yeah, we may need to look at marking more items with those labels as
> well.
>
> Thanks
>
>
>
> On Thu, May 5, 2016 at 2:52 PM, sri man  wrote:
>
>> Hi,
>>
>> I am new to Geode/Gemfire.
>> Yesterday in the Geode Clubhouse I heard someone mentioning that there
>> are 30-50 items that can be worked by newbies.
>> The labels mentioned were “lowhangingfruit” and “GoogleSummerOfCode”. I
>> could not find the labels in JIRA.
>> Instead I found these beginner friendly labels - “easyfix", “newbie",
>> “starter”. Among these 3 there are 9 issues.
>>
>> Can somebody pl let me know what is the right way to look for beginner
>> friendly JIRAs ?
>>
>> Thanks,
>> Srikanth Manvi
>
>
>
>
> --
>
> ~/William
>
>
>
>


-- 

~/William

Re: JIRA labels for newbies

[William Markito]

You may find a couple of more interesting items on this query -  project =
GEODE and status in (Open) and labels in (starter, gsoc2016,
low-hanging-fruit)

https://issues.apache.org/jira/browse/GEODE-1113?jql=project%20%3D%20GEODE%20and%20status%20in%20(Open)%20and%20labels%20in%20(starter%2C%20gsoc2016%2C%20low-hanging-fruit)

But yeah, we may need to look at marking more items with those labels as
well.

Thanks



On Thu, May 5, 2016 at 2:52 PM, sri man  wrote:

> Hi,
>
> I am new to Geode/Gemfire.
> Yesterday in the Geode Clubhouse I heard someone mentioning that there are
> 30-50 items that can be worked by newbies.
> The labels mentioned were “lowhangingfruit” and “GoogleSummerOfCode”. I
> could not find the labels in JIRA.
> Instead I found these beginner friendly labels - “easyfix", “newbie",
> “starter”. Among these 3 there are 9 issues.
>
> Can somebody pl let me know what is the right way to look for beginner
> friendly JIRAs ?
>
> Thanks,
> Srikanth Manvi




-- 

~/William

Re: Review Request 46908: GEODE-92: PR with entry eviction 1 leaves 3 entries in memory with async overflow

[Darrel Schneider]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46908/#review131950
---


Ship it!




Ship It!

- Darrel Schneider


On May 2, 2016, 1:40 p.m., Sai Boorlagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46908/
> ---
> 
> (Updated May 2, 2016, 1:40 p.m.)
> 
> 
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken 
> Howe.
> 
> 
> Bugs: GEODE-92
> https://issues.apache.org/jira/browse/GEODE-92
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * Disabled lruUpdateCallback at a higher level as calling multiple times 
> could leave LRU entries in Memory.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractLRURegionMap.java
>  2cc7a555eeac754a4be786bb1e4c04a5e2f0cebb 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 
> c32a7d20f0f5c484e088e8c0b57bc4aa37e89378 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java
>  089d7239fb2f8d57db20a62ad76d0c7a64945e2a 
> 
> Diff: https://reviews.apache.org/r/46908/diff/
> 
> 
> Testing
> ---
> 
> precheckin, smoke
> 
> 
> Thanks,
> 
> Sai Boorlagadda
> 
>

JIRA labels for newbies

[sri man]

Hi,

I am new to Geode/Gemfire.
Yesterday in the Geode Clubhouse I heard someone mentioning that there are 
30-50 items that can be worked by newbies.
The labels mentioned were “lowhangingfruit” and “GoogleSummerOfCode”. I could 
not find the labels in JIRA. 
Instead I found these beginner friendly labels - “easyfix", “newbie", 
“starter”. Among these 3 there are 9 issues.

Can somebody pl let me know what is the right way to look for beginner friendly 
JIRAs ?

Thanks,
Srikanth Manvi

Re: Review Request 46916: GEODE-641: Adding default methods to the Function interface

[anilkumar gingade]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46916/#review131934
---


Ship it!




Ship It!

- anilkumar gingade


On May 2, 2016, 10:55 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46916/
> ---
> 
> (Updated May 2, 2016, 10:55 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Adding default methods to Function so that Function is now a functional
> interface and lambda expressions can be used for functions.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/execute/Function.java 
> 2e4e0660f1242797906de268e4a11e736215c7c6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/execute/FunctionAdapter.java
>  c4cfeed909c843ab1eb4775258685b2691cf5fb9 
>   
> geode-core/src/test/java/com/gemstone/gemfire/cache/execute/FunctionAdapterJUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46916/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

[GitHub] incubator-geode pull request: GEODE-1340: Refactored the names of ...

[upthewaterspout]

Github user upthewaterspout commented on the pull request:

https://github.com/apache/incubator-geode/pull/141#issuecomment-217282247
  
+1 I'll merge this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 46243: GEODE-1209: Added new attribute to forward eviction/expiration to AEQ. Tested with manual testing.

[Dan Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46243/#review131925
---



Nice job! I had a few comments, see below.

It also might be nice to try to start adding some real unit tests for some of 
these core classes. Would it be possible to add a unit test for the distribute 
method (Which calls your new checkForDistribution code)?


geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 
(line 1151)


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.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 
(line 6645)


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.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 
(line 6650)


Remove this log message.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java
 (line 808)


Remove this log message.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySender.java
 (line 816)


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?



geode-core/src/test/java/com/gemstone/gemfire/cache/asyncqueue/AsyncEventQueueEvictionAndExpirationJUnitTest.java
 (line 63)


This is not a unit test if it's creating a cache. Should be marked as an 
integration test.


- Dan Smith


On May 4, 2016, 1:09 a.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46243/
> ---
> 
> (Updated May 4, 2016, 1:09 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, 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
>  aa7d49a654fcc0c7d2419546f4b4b5e1c271 
>   
> 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
>  57e3a13b320e3

Re: Review Request 47004: a quick solution to wait for entry flushed into index

[Dan Smith]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47004/#review131921
---



It seems like maybe the Flush API ought to be on the index, not the 
LuceneServiceImpl. And I thought the intention was to add this to the public 
API, but it looks like right now this is just an internal API?

Please add true unit test for these changes.

Regarding your integration test, I don't think this is the right way to test 
this. You're just assuming the entries won't be flushed by the time your 
waitForFlush call happens, but what's to prevent them from being flushed by 
that point? Also, the integration tests should run as fast as possible, so an 
integration test that puts 1K entries seems excessive.


geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
 (line 243)


Remove this log message - we don't need an info level log here.



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
 (line 81)


Maybe name this something like
entriesFlushedToIndexAfterWaitForFlushCalled?


- Dan Smith


On May 5, 2016, 6:23 a.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47004/
> ---
> 
> (Updated May 5, 2016, 6:23 a.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: geode-1351
> https://issues.apache.org/jira/browse/geode-1351
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is for test purpose.
> 
> An advanced version will be implemented later using function execution.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
>  58a9b20 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  c467a18 
> 
> Diff: https://reviews.apache.org/r/47004/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #300 has FAILED (2 tests failed, no failures were new)

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #300 failed.
---
Scheduled
2/1401 tests failed, no failures were new.

https://build.spring.io/browse/SGF-NAG-300/

-
Currently Responsible
-

No one is responsible for fixing this build.



--
Failing Jobs
--
  - Default Job (Default Stage): 2 of 1401 tests failed.




--
Tests
--
Existing Test Failures (2)
   - GemfireUtilsTest: Is gemfire version65 or above
   - GemfireUtilsTest: Is gemfire version7 or above

--
This message is automatically generated by Atlassian Bamboo

[GitHub] incubator-geode pull request: GEODE-1340: Refactored the names of ...

[nabarunnag]

GitHub user nabarunnag opened a pull request:

https://github.com/apache/incubator-geode/pull/141

GEODE-1340: Refactored the names of lucene integration tests.

* Renamed the names of integration tests to not end with JUnitTest but just 
IntegrationTest
* Modified LuceneIndexXmlParserJUnitTest code to use mockito so that the 
test is more aligned to Unit tests.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nabarunnag/incubator-geode feature/GEODE-1340

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-geode/pull/141.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #141


commit fd6da6c53af32d0f4727473d64d44eebe63b45e6
Author: nabarun 
Date:   2016-05-04T17:07:23Z

GEODE-1340: Refactored the names of lucene integration tests.

* Renamed the names of integration tests to not end with JUnitTest but just 
IntegrationTest
* Modified LuceneIndexXmlParserJUnitTest code to use mockito so that the 
test is more aligned to Unit tests.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 46915: Add test coverage for using a custom log4j2.xml with Geode

[Jens Deppe]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46915/#review131876
---


Ship it!




Ship It!

- Jens Deppe


On May 4, 2016, 11:03 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46915/
> ---
> 
> (Updated May 4, 2016, 11:03 p.m.)
> 
> 
> Review request for geode, Jens Deppe, Jinmei Liao, Kirk Lund, and William 
> Markito.
> 
> 
> Bugs: GEODE-1255
> https://issues.apache.org/jira/browse/GEODE-1255
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1255: Add test coverage for using a custom log4j2.xml with Geode
> 
> * refactor launcher integration tests (rename files, use TemporaryFolder)
> * refactor cluster config tests (use TemporaryFolder)
> * change several additional tests to use TemporaryFolder
> * add better support for TemporaryFolder usage to HeadlessGfsh
> * alter HeadlessGfsh tests to use TemporaryFolder
> * rescope variables and methods to private where possible
> * fixup file formatting
> * prevent eating of exceptions
> * remove incomplete or useless javadocs
> * rename LocatorJUnitTest test methods from TRAC #s to meaningful names
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherAssemblyIntegrationTest.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherAssemblyJUnitTest.java
>  0984508 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationEndToEndDUnitTest.java
>  653bbdc 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java
>  b632edb 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/SharedConfiguration.java
>  8f190eb 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherIntegrationJUnitTest.java
>  745090d 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherIntegrationTestCase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherJUnitTest.java
>  f5867d4 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherJUnitTestCase.java
>  77961e0 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherServiceStatusJUnitTest.java
>  ca24a4e 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherServiceStatusTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLauncherTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLocatorLauncherIntegrationTestCase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLocatorLauncherJUnitTestCase.java
>  62c4d86 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractLocatorLauncherRemoteIntegrationTestCase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractServerLauncherIntegrationTestCase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractServerLauncherJUnitTestCase.java
>  0ee3e7c 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/AbstractServerLauncherRemoteIntegrationTestCase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/DistributedTestSuite.java
>  58b8d2a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/HostedLocatorsDUnitTest.java
>  a5b07ee 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LauncherMemberMXBeanIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LauncherMemberMXBeanJUnitTest.java
>  30e21e1 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LauncherTestSuite.java
>  b5e787a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorJUnitTest.java
>  3c80d9a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherIntegrationJUnitTest.java
>  3b56554 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherJUnitTest.java
>  a8ddd6a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherLocalFileIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorLauncherLocalFileJUnitTest.java
>  7dc5452 
>   

Re: POSTED: Geode Clubhouse - Where Does Geode Fit in Modern System Architectures?

[Gregory Chase]

Greetings Geode Community,
We've posted the replay of yesterday's Geode Clubhouse:
https://www.youtube.com/watch?v=tOAe1n7Qyd8

-Greg

On Tue, May 3, 2016 at 8:21 AM, Gregory Chase  wrote:

> Greetings Apache Geode Community,
> This is a reminder that we're hosting a Geode Clubhouse meeting tomorrow.
> See you there!
>
> As always it will be recorded and posted.
>
> -Greg
>
> On Wed, Apr 27, 2016 at 2:44 PM, Gregory Chase  wrote:
>
>> It's time for the next Apache Geode Clubhouse Meeting on May 4th!
>>
>> Each meeting starts with a user technology discussion topic, followed by
>> a contributor & committer discussion of open JIRA issues, release
>> coordination, and open Q&A.
>>
>> User Tech Talk: Where Does Geode Fit in Modern System Architectures
>>
>> Eitan Suez, consulting instructor and deveoper with Pivotal will recap
>> his talk from Geode Summit covering: Geode in context of a CQRS
>> architecture.
>>
>> About the speaker: Eitan is a long time developer, instructor and
>> consultant.  Prior to Pivotal he was a principal consultant helping
>> customers develop applications based on GemFire.
>>
>> Standing agenda:
>>
>> -Open Jiras, and ways to contribute
>> -Next release coordination
>> -Open Q&A
>>
>> Please join us on May 4th, 2016 at:
>> https://pivotalcommunity.adobeconnect.com/clubhouse/
>>
>> 5/4 San Francisco, CA 9:00 AM PDT UTC-8 hours
>> 5/4 New York, NY 12:00 PM EDT UTC-5 hours
>>
>> As always, this meeting will be recorded and posted.
>>
>> 
>> Adobe Connect tips: For issues with Chrome.  A little icon appears in the
>> address bar.  It's EASY to miss. You click it and it allows Adobe to use
>> the mic, even if you select 'allow' in the popup, it doesn't work until you
>> change it in address bar too.
>>
>> Also, preload as much content as is possible to help bandwidth issues.
>> 
>> If you have never attended an Adobe Connect meeting before:
>>
>> Test your connection:
>> https://pivotalcommunity.adobeconnect.com/common/help/en/support/meeting_test.htm
>>
>> Get a quick overview: http://www.adobe.com/products/adobeconnect.html
>>
>>
>> --
>> Greg Chase
>>
>> Global Head, Big Data Communities
>> http://www.pivotal.io/big-data
>>
>> Pivotal Software
>> http://www.pivotal.io/
>>
>> 650-215-0477
>> @GregChase
>> Blog: http://geekmarketing.biz/
>>
>>
>
>
> --
> Greg Chase
>
> Global Head, Big Data Communities
> http://www.pivotal.io/big-data
>
> Pivotal Software
> http://www.pivotal.io/
>
> 650-215-0477
> @GregChase
> Blog: http://geekmarketing.biz/
>
>


-- 
Greg Chase

Global Head, Big Data Communities
http://www.pivotal.io/big-data

Pivotal Software
http://www.pivotal.io/

650-215-0477
@GregChase
Blog: http://geekmarketing.biz/