Re: Review Request 53814: GEODE-2100 Add new version of query client server messages

[Jason Huynh]

> On Nov. 16, 2016, 6:06 p.m., Dan Smith wrote:
> > What is the JUnit4DistributedTestCase change for?

If a previous test failed with suspect strings in tearDownDistributedTestCase, 
the postTearDown would not get called for the unit test.  This then caused the 
rest of the tests to fail because any clean up for that specific test was not 
being done.


- Jason


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


On Nov. 16, 2016, 5:22 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53814/
> ---
> 
> (Updated Nov. 16, 2016, 5:22 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, nabarun nag, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added new versions of messages, preventing CumulativeNonDistinctResults from 
> being sent back to a pre Geode client.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
>  08c2ecf 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  579a3e1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Query.java
>  a6dc022 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Query651.java
>  51b2a24 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/QueryGeode10.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/QueryWithParametersGeode10.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/internal/JUnit4DistributedTestCase.java
>  838bb29 
> 
> Diff: https://reviews.apache.org/r/53814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 53814: GEODE-2100 Add new version of query client server messages

[Jason Huynh]

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

Review request for geode, Barry Oglesby, nabarun nag, and Dan Smith.


Repository: geode


Description
---

Added new versions of messages, preventing CumulativeNonDistinctResults from 
being sent back to a pre Geode client.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
 08c2ecf 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
 579a3e1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Query.java
 a6dc022 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Query651.java
 51b2a24 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/QueryGeode10.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/QueryWithParametersGeode10.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/internal/JUnit4DistributedTestCase.java
 838bb29 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 53401: GEODE-1932: Protected use of global variables

[Jason Huynh]

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




geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java
 (line 668)
<https://reviews.apache.org/r/53401/#comment224277>

the message is doing a list.get(0), but it should be doing a list.get(1)


- Jason Huynh


On Nov. 2, 2016, 8:14 p.m., nabarun nag wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53401/
> ---
> 
> (Updated Nov. 2, 2016, 8:14 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian 
> zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * As per the stacktrace in the ticket we could see that the failure was 
> happening because the test was getting the wrong result for the function 
> execution.
> * In the TestFunction executeFunctionReexecuteExceptionOnServer we could see 
> that it was using global counter static variable retryCount.
> * This global variable retryCount was also used by 
> executeFunctionReexecuteException.
> * So if these functions are invoked parallely we have a chance that these 
> counter global variable may have corrupted values.
> * Hence we gave both the test functions their own retry count global static 
> variable, they dont share retry count anymore.
> * Also we made these test functions synchronized so that reexecution by 
> different threads do no corrupt the global static variables.
> * The test function executeFunctionReexecuteExceptionOnServer stops 
> re-execution when the retryCount >= 5 but in the test we validate that 
> retryCount == 5. This was made uniform by making the test validate that 
> retryCount >= 5 
> * Added more information in the assertEquals, which now output the received 
> and expected values when there is an assertion failure.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java
>  d217792 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java
>  f9f05ab 
> 
> Diff: https://reviews.apache.org/r/53401/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> IntelliJ multiple executions.
> 
> 
> Thanks,
> 
> nabarun nag
> 
>

Re: Review Request 53399: Removing some string comparisons in the AttributesDescriptor

[Jason Huynh]

> On Nov. 2, 2016, 7:58 p.m., Jason Huynh wrote:
> > Ship It!

Do you think a unit test should be written for this?  Not sure how difficult or 
useful it would be...


- Jason


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


On Nov. 2, 2016, 7:29 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53399/
> ---
> 
> (Updated Nov. 2, 2016, 7:29 p.m.)
> 
> 
> Review request for geode, Jason Huynh and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> I found this while working on GEODE-1985. AttributesDescriptor does a bunch 
> of string comparisons when it could do a single, more efficient instanceof. 
> 
> With a microbenchmark I found this sped up reevaluation of an entry when I 
> did an indexed query.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java
>  a11bea0752e8ed8184f0173f81be08b001cb6e19 
> 
> Diff: https://reviews.apache.org/r/53399/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 53399: Removing some string comparisons in the AttributesDescriptor

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On Nov. 2, 2016, 7:29 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53399/
> ---
> 
> (Updated Nov. 2, 2016, 7:29 p.m.)
> 
> 
> Review request for geode, Jason Huynh and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> I found this while working on GEODE-1985. AttributesDescriptor does a bunch 
> of string comparisons when it could do a single, more efficient instanceof. 
> 
> With a microbenchmark I found this sped up reevaluation of an entry when I 
> did an indexed query.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java
>  a11bea0752e8ed8184f0173f81be08b001cb6e19 
> 
> Diff: https://reviews.apache.org/r/53399/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 53398: GEODE-1985: Updating the SAFE_QUERY_TIME after updating indexes

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On Nov. 2, 2016, 7:26 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53398/
> ---
> 
> (Updated Nov. 2, 2016, 7:26 p.m.)
> 
> 
> Review request for geode, Jason Huynh and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is a fix for pretty specific race condition
> 1) T1 does a put and gets to the point of calling setIndexBufferTime,
> but hasn't updated the indexes
> 2) T2 starts a query and finds the entry in the index even though the
> value no longer matches the query
> 3) T1 finishes the put
> 4) T2 checks to see if should revaluate the entry, but decides not to
> because based on the SAFE_QUERY_TIME value the entry changed before the
> query started.
> 
> By moving the update to SAFE_QUERY_TIME down, if the an entry
> doesn't need reevaluation based on the SAFE_QUERY_TIME, we know the
> index was updated before the query started.
> 
> There is currently an updateInProgress flag to handle the issue of the
> query executing before the SAFE_QUERY_TIME is updated. If the entry is
> updated, but not the index, the updateInProgress flag will be set.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 360c6a9f444b41a99bec1a896e660bd506d793ff 
> 
> Diff: https://reviews.apache.org/r/53398/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 52750: GEODE-1985: Check for index expression reevalaution using a time window

[Jason Huynh]

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


Fix it, then Ship it!





geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java
 (line 145)
<https://reviews.apache.org/r/52750/#comment221082>

Would you be able to remove/modify this comment a bit?  Mostly remove the 
bug number as it's not a geode number any more



geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
(line 371)
<https://reviews.apache.org/r/52750/#comment221083>

Was this intended for this diff?


- Jason Huynh


On Oct. 11, 2016, 9:53 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52750/
> ---
> 
> (Updated Oct. 11, 2016, 9:53 p.m.)
> 
> 
> Review request for geode, Jason Huynh and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Changing the logic for how to we check to see if an entry may have been
> concurrently modified while an indexed query is in progress.
> 
> The new logic just has a time window, defaulting to 10 minutes. If the
> entry was changed less than 10 minutes for the query started, we will
> reevaluate the index expression to make sure the entry is still valid.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java
>  8ef82f1bee9831fa50a93f87bb9ed0b644a542b4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  84ba926ee35638c4263a0d93a8be3fef1e2c5f7f 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 46ccd47e96e75a8c4c6f3075b65a2d2447043213 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/IndexManagerJUnitTest.java
>  c7633ea05b0f8a90baf0c8f9e9849480421ebd45 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/AbstractIndexMaintenanceIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexMaintenanceNoReevaluationIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/RangeIndexAPIJUnitTest.java
>  7cc2c12f80ba4248edca9035bbdfd839d20f70ad 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionQueryDUnitTest.java
>  1160d4bdb755e39a2a5c52fb167b86e760e0ae88 
> 
> Diff: https://reviews.apache.org/r/52750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Hibernate module and Geode 1.0 ?

[Jason Huynh]

I've created a ticket (feel free to modify/reword/rename it).  It's
GEODE-1972: Move Geode Hibernate module to a feature branch



On Thu, Oct 6, 2016 at 10:17 AM Dave Barnes <dbar...@pivotal.io> wrote:

So, what form should the JIRA ticket assume?
(a) Move HIbernate from develop to a feature branch, or
(b) Merge Hibernate code to develop (as a future task, cf GEODE-1416 for
how the NC was handled)

After somebody (which I can do, if you like) creates a ticket for the code,
I'll create a subtask for moving the accompanying docs.

On Thu, Oct 6, 2016 at 9:34 AM, Joey McAllister <jmcallis...@pivotal.io>
wrote:

> +1 for moving to a feature branch.
>
> On Thu, Oct 6, 2016 at 9:32 AM Mark Bretl <asf.mbr...@gmail.com> wrote:
>
> > +1 for feature branch as well.
> >
> > --Mark
> >
> > On Thu, Oct 6, 2016 at 9:30 AM, Dan Smith <dsm...@pivotal.io> wrote:
> >
> > > +1 for moving it to a feature branch.
> > >
> > > -Dan
> > >
> > > On Wed, Oct 5, 2016 at 2:40 PM, Jason Huynh <jhu...@pivotal.io> wrote:
> > > > Bumping to see if we have come to a decision on whether we want to
> move
> > > > this to a feature branch and get rid of it for 1.0 or post 1.0,
> > > especially
> > > > now that the 1.0 release branch has been cut
> > > >
> > > > On Fri, Sep 23, 2016 at 5:22 PM Anthony Baker <aba...@pivotal.io>
> > wrote:
> > > >
> > > > Likewise!  Geode provides an L2 cache for Hibernate.  That is, an
> > > > application that is using Hibernate could plug in Geode for caching.
> > > > Specifically, we implement Hibernate’s cache interfaces like
> > > CacheProvider,
> > > > RegionFactory, etc.
> > > >
> > > > There are build-time dependencies on several hibernate jars
> > > > (hibernate-annotations, hibernate-core,
> hibernate-commons-annotations).
> > > No
> > > > hibernate source code or jars are shipped with any release.
> > > >
> > > > Docs:
> > > > http://geode.docs.pivotal.io/docs/tools_modules/hibernate_
> > > cache/chapter_overview.html
> > > >
> > > > Code:
> > > > https://git-wip-us.apache.org/repos/asf?p=incubator-geode.
> > > git;a=tree;f=extensions/geode-modules-hibernate;h=
> > > be8b9355934f824b9d4565ec6bfaa5d17a117f45;hb=HEAD
> > > >
> > > > ~/working/apache-geode-1.0.0-incubating.M3$ unzip -l
> > > > tools/Modules/Apache_Geode_Modules-1.0.0-incubating.M3-Hibernate.zip
> > > > Archive:
> > > > tools/Modules/Apache_Geode_Modules-1.0.0-incubating.M3-Hibernate.zip
> > > >   Length Date   TimeName
> > > >     
> > > > 0  08-01-16 17:01   lib/
> > > >114497  08-01-16 16:58   lib/geode-modules-1.0.0-
> incubating.M3.jar
> > > > 56960  08-01-16 17:01
> > > >  lib/geode-modules-hibernate-1.0.0-incubating.M3.jar
> > > >     ---
> > > >171457   3 files
> > > >
> > > > ~/working/apache-geode-1.0.0-incubating.M3$ jar tvf
> > > > lib/geode-modules-hibernate-1.0.0-incubating.M3.jar
> > > >  0 Mon Aug 01 17:01:40 PDT 2016 META-INF/
> > > >139 Mon Aug 01 17:01:40 PDT 2016 META-INF/MANIFEST.MF
> > > >  28210 Mon Jul 25 21:52:24 PDT 2016 META-INF/LICENSE
> > > >584 Fri Jul 08 12:51:12 PDT 2016 META-INF/NOTICE
> > > >  0 Mon Aug 01 17:01:40 PDT 2016 com/
> > > >  0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/
> > > >  0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/gemfire/
> > > >  0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/gemfire/modules/
> > > >  0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/gemfire/modules/
> > > hibernate/
> > > >   1210 Mon Aug 01 17:01:40 PDT 2016
> > > > com/gemstone/gemfire/modules/hibernate/EnumType.class
> > > >   5707 Mon Aug 01 17:01:40 PDT 2016
> > > > com/gemstone/gemfire/modules/hibernate/GemFireCache.class
> > > >   1700 Mon Aug 01 17:01:40 PDT 2016
> > > > com/gemstone/gemfire/modules/hibernate/GemFireCacheListener.class
> > > >   7084 Mon Aug 01 17:01:40 PDT 2016
> > > > com/gemstone/gemfire/modules/hibernate/GemFireCacheProvider.class
> > > >   1104 Mon Aug 01 17:01:40 PDT 2016
> > > > com/gemstone/gemfire/modules/hibernate/
> GemFireQueryCacheFactory.class
> > > >   9529 Mon Aug 01 17:01:40 PDT 2016

Re: Hibernate module and Geode 1.0 ?

[Jason Huynh]

Bumping to see if we have come to a decision on whether we want to move
this to a feature branch and get rid of it for 1.0 or post 1.0, especially
now that the 1.0 release branch has been cut

On Fri, Sep 23, 2016 at 5:22 PM Anthony Baker  wrote:

Likewise!  Geode provides an L2 cache for Hibernate.  That is, an
application that is using Hibernate could plug in Geode for caching.
Specifically, we implement Hibernate’s cache interfaces like CacheProvider,
RegionFactory, etc.

There are build-time dependencies on several hibernate jars
(hibernate-annotations, hibernate-core, hibernate-commons-annotations).  No
hibernate source code or jars are shipped with any release.

Docs:
http://geode.docs.pivotal.io/docs/tools_modules/hibernate_cache/chapter_overview.html

Code:
https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;a=tree;f=extensions/geode-modules-hibernate;h=be8b9355934f824b9d4565ec6bfaa5d17a117f45;hb=HEAD

~/working/apache-geode-1.0.0-incubating.M3$ unzip -l
tools/Modules/Apache_Geode_Modules-1.0.0-incubating.M3-Hibernate.zip
Archive:
tools/Modules/Apache_Geode_Modules-1.0.0-incubating.M3-Hibernate.zip
  Length Date   TimeName
    
0  08-01-16 17:01   lib/
   114497  08-01-16 16:58   lib/geode-modules-1.0.0-incubating.M3.jar
56960  08-01-16 17:01
 lib/geode-modules-hibernate-1.0.0-incubating.M3.jar
    ---
   171457   3 files

~/working/apache-geode-1.0.0-incubating.M3$ jar tvf
lib/geode-modules-hibernate-1.0.0-incubating.M3.jar
 0 Mon Aug 01 17:01:40 PDT 2016 META-INF/
   139 Mon Aug 01 17:01:40 PDT 2016 META-INF/MANIFEST.MF
 28210 Mon Jul 25 21:52:24 PDT 2016 META-INF/LICENSE
   584 Fri Jul 08 12:51:12 PDT 2016 META-INF/NOTICE
 0 Mon Aug 01 17:01:40 PDT 2016 com/
 0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/
 0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/gemfire/
 0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/gemfire/modules/
 0 Mon Aug 01 17:01:40 PDT 2016 com/gemstone/gemfire/modules/hibernate/
  1210 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/EnumType.class
  5707 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/GemFireCache.class
  1700 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/GemFireCacheListener.class
  7084 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/GemFireCacheProvider.class
  1104 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/GemFireQueryCacheFactory.class
  9529 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/GemFireRegionFactory.class
 0 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/
  1020 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/Access$1.class
  9535 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/Access.class
   343 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/ClientServerRegionFactoryDelegate$1.class
  1508 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/ClientServerRegionFactoryDelegate$LocatorHolder.class
  9639 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/ClientServerRegionFactoryDelegate.class
  9739 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/CollectionAccess.class
  2409 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/EntityRegionWriter.class
   240 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/EntityVersion.class
   964 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/EntityVersionImpl.class
  2446 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/EntityWrapper.class
  5001 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/GemFireBaseRegion.class
  2563 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/GemFireCollectionRegion.class
  7702 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/GemFireEntityRegion.class
  3058 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/GemFireQueryResultsRegion.class
  2547 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/KeyWrapper.class
  2911 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/NonStrictReadWriteAccess.class
  1670 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/ReadOnlyAccess.class
  1121 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/ReadWriteAccess.class
  7073 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/RegionFactoryDelegate.class
   581 Mon Aug 01 17:01:40 PDT 2016
com/gemstone/gemfire/modules/hibernate/internal/TransactionalAccess.class


Anthony

> On Sep 23, 

Review Request 52575: GEODE-1962: increment notQueuedConflated stat is peeked object is unresolvable from off heap

[Jason Huynh]

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

Review request for geode, nabarun nag and Dan Smith.


Repository: geode


Description
---

There is a scenario where the stats do not reflect an event being conflated.  
After peeking the original object from the bucketRegionQueue, when we try to 
resolve from off heap, if the resolved value is null, we just continue.  
Instead we should behave similar to the bucketRegionQueue peek method, where it 
increments the stat if the object itself is null.

Minor refactoring and changing of method visibility for testing purposes
Removed some unused code


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java
 1a9b126 
  
geode-core/src/test/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueueJUnitTest.java
 f1d4408 

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


Testing
---


Thanks,

Jason Huynh

Review Request 52518: GEODE-1963: Add lucene xsd to website

[Jason Huynh]

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

Review request for geode, Joey McAllister, William Markito, and Dan Smith.


Repository: geode


Description
---

Lucene xsd should be added to the website.  This is checking in the xsd into 
source control, after checking this in, I would generate/replace the website 
content on the asf-site branch


Diffs
-

  geode-site/website/content/schema/cache/lucene-1.0.xsd PRE-CREATION 

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


Testing
---


Thanks,

Jason Huynh

Review Request 52366: GEODE-1675: Update the tomcat 8 context-fragment files for tcserver to use the new Tomcat8 manager

[Jason Huynh]

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

Review request for geode and William Markito.


Repository: geode


Description
---

The previous changes to update tomcat 8 missed out on the changes the tcserver 
context-fragments.  

We can either create a new tc server zip file or modify the existing one.  This 
approach modifies the existing one.  If we want to create a new zip file, we 
will need to know what to name it.. tcserver32.zip?


Diffs
-

  
extensions/geode-modules-assembly/release/tcserver/geode-cs-tomcat-8/context-fragment.xml
 b7ad94e 
  
extensions/geode-modules-assembly/release/tcserver/geode-p2p-tomcat-8/context-fragment.xml
 252fa8f 

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


Testing
---


Thanks,

Jason Huynh

Review Request 52356: GEODE-1944: Index with method invocation in regionPath can throw exception when tombstones clean up during gii

[Jason Huynh]

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

Review request for geode, nabarun nag, Dan Smith, and xiaojian zhou.


Repository: geode


Description
---

The fix is to catch the EntryDestroyedException and execute a "remove" just to 
force a removal from the index incase something/somehow got in there.  This 
specific problem would only occur on start up, during recovery from disk, where 
tombstones have been reaped and we end up gii'ing and doing a 
removeOldTombstones call.

Renamed and recategorized a few tests.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
 c99da56 
  
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/HashIndex.java
 775daec 
  
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java
 4dcd1c3 
  
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexStore.java
 c26ec48 
  
geode-core/src/test/java/org/apache/geode/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java
 f8bae1b 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/AbstractIndexMaintenanceIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexMaintenanceIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexQueryIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/HashIndexJUnitTest.java
 432199a 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/HashIndexMaintenanceIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/HashIndexQueryIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/RangeIndexMaintenanceIntegrationTest.java
 PRE-CREATION 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

[Jason Huynh]

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



Not sure what an appropriate test would be for this, maybe create a dunit with 
test hooks or perhaps target the peekAhead method directly and override the 
optimalGet and makeHeapCopy methods to force/fake it down the problematic 
path... It would be nice to get a test if possible


geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java
 (line 796)
<https://reviews.apache.org/r/52176/#comment218081>

+1 on the less confusing part


- Jason Huynh


On Sept. 22, 2016, 9:46 p.m., nabarun nag wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52176/
> ---
> 
> (Updated Sept. 22, 2016, 9:46 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Eric Shu, Jason Huynh, Dan Smith, 
> and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Before my changes:
> 1. peek calls peekAhead to get the object from the sender queue to be put 
> into the batch for dispatch.
> 2. peekAhead gets the current key for which it is able to get an object back 
> by calling optimalGet.
> 3. It puts that current key into the peekedIds list and returns the object 
> back to peek.
> 4. peek then tries to make a heapcopy and only if it is successful it will 
> put the object into the dispatch batch.
> 5. Here is the issue, now conflation may kick in before peek is able to do 
> heapcopy and that object is removed from the queue and hence the object will 
> not be placed in the dispatch batch. However the the key representing that 
> object in the queue still exist in the PeekedIDs list.
> 6. So now there is an extra key in the peekedIds list.
> 7. Batch is dispatched and now the ack is received, so things need to be 
> removed from the sender queue.
> 8. remove() is called in a for loop with the count set to size of dispatch 
> batch for which ack was received. 
> 9. The remove() operation picks up keys from peekedIds list sequentially and 
> calls remove(key) on the queue.
> 10. Now since the batch size is smaller than the Ids peeked, element will 
> still linger behind after all remove calls are completed.
> 11. so tests which wait for queues to be empty will hang forever.
> 
> Solution:
> Since peek() doesnt have access to the current key but just the object and 
> hence cannot remove it from peekedIDs list, we moved the check for heapcopy 
> into peekAhead. 
> 
> So now only if a successful heap copy is made then only the key will be 
> placed into the peekedIDs list.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java
>  a8bb72d 
> 
> Diff: https://reviews.apache.org/r/52176/diff/
> 
> 
> Testing
> ---
> 
> precheck
> 
> 
> Thanks,
> 
> nabarun nag
> 
>

Re: Review Request 51734: GEODE-1864: CompactMapRangeIndex not updating old keys properly

[Jason Huynh]

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

(Updated Sept. 21, 2016, 4 p.m.)


Review request for geode, Barry Oglesby and nabarun nag.


Changes
---

GEODE-1864: Updated diff.

GEODE-1745: Additional fix for CompactRangeIndex Reevaluation causing results 
to be missed when using a MapIndex without an In clause.


Repository: geode


Description
---

Old keys that are no longer part of a new map, should be removed from the index.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactMapRangeIndex.java
 ce1ea08 
  
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
 d5dd5a6 
  
geode-core/src/test/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
 9833e43 

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


Testing
---


Thanks,

Jason Huynh

Review Request 51750: GEODE-1873: Updating geode session module to support Tomcat 7.0.70

[Jason Huynh]

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

Review request for geode, Barry Oglesby, William Markito, and nabarun nag.


Repository: geode


Description
---

There were changes in Tomcat that changed the field type of "attributes" from 
Map to ConcurrentMap.  This breaks serialization of the DeltaSession object and 
causes exceptions to be thrown.

This patch adds a new DeltaSession7 object that should fix this problem.


Diffs
-

  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
 8776c16 
  
extensions/geode-modules-tomcat7/src/test/java/com/gemstone/gemfire/modules/session/Tomcat7SessionsJUnitTest.java
 e7970d7 
  
extensions/geode-modules/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsBase.java
 5726013 
  gradle/dependency-versions.properties a19520c 

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


Testing
---


Thanks,

Jason Huynh

Review Request 51734: GEODE-1864: CompactMapRangeIndex not updating old keys properly

[Jason Huynh]

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

Review request for geode, Barry Oglesby and nabarun nag.


Repository: geode


Description
---

Old keys that are no longer part of a new map, should be removed from the index.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactMapRangeIndex.java
 c374a10 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
 e346522 

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


Testing
---


Thanks,

Jason Huynh

Review Request 51735: GEODE-1840: Certain map queries combine OR predicates into an AND junction

[Jason Huynh]

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

Review request for geode, Barry Oglesby and nabarun nag.


Repository: geode


Description
---

When deciding whether to combine the predicates into a common junction, the 
type should be checked to make sure they are the same.  This will prevent OR 
junctions from being added/converted to an AND junction and vice versa.  This 
was only affecting certain query paths.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/AbstractGroupOrRangeJunction.java
 a14dda8 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java
 8b27309 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 51010: GEODE-11 Test for Cluster Configuration changes for Lucene index.

[Jason Huynh]

> On Aug. 11, 2016, 11:16 p.m., Jason Huynh wrote:
> > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java,
> >  line 189
> > <https://reviews.apache.org/r/51010/diff/1/?file=1471614#file1471614line189>
> >
> > Will this dir and clusterConfigDir get cleaned up after the test is 
> > run?  Should this use a TemporaryFolder rule or something else?
> 
> anilkumar gingade wrote:
> It uses the TemporaryFolder...yes it clears the config dir...Without this 
> i was having problem with the second test, where it was loading the cluster 
> config from previous test...
> 
> Jason Huynh wrote:
> Out of curiosity, how is the TemporaryFolder being used here?  Isn't 
> there a TemporaryFolder rule that needs to be created and used?  This looks 
> like it's just doing new File().mkdir()?
> 
> anilkumar gingade wrote:
> I followed the logic from ohter tests...
> 
> The file path is obtained from TemporaryFolder Rule, When the test ends 
> the Rule deletes the dir using the path info.
> 
>final String nodeDir = 
> this.temporaryFolder.getRoot().getCanonicalPath()
> + File.separator + vmIndex;
> File workingDir = new File(nodeDir);
> workingDir.mkdirs();

Oh, I missed that part.  I think temporaryFolder has a method for newFolder("") 
and newFile().


- Jason


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


On Aug. 11, 2016, 10:34 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51010/
> -------
> 
> (Updated Aug. 11, 2016, 10:34 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, Kirk 
> Lund, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added test to validate Cluster configuration support for Lucene indexes.
> 
> Added a new Rule to create Distributed System with test/custom 
> configuration...The "LocatorServerConfigurationRule" makes it easier to 
> create a Locator or cluster nodes with required configuration.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51010/diff/
> 
> 
> Testing
> ---
> 
> Run the newly added test.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 51010: GEODE-11 Test for Cluster Configuration changes for Lucene index.

[Jason Huynh]

> On Aug. 11, 2016, 11:16 p.m., Jason Huynh wrote:
> > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java,
> >  line 189
> > <https://reviews.apache.org/r/51010/diff/1/?file=1471614#file1471614line189>
> >
> > Will this dir and clusterConfigDir get cleaned up after the test is 
> > run?  Should this use a TemporaryFolder rule or something else?
> 
> anilkumar gingade wrote:
> It uses the TemporaryFolder...yes it clears the config dir...Without this 
> i was having problem with the second test, where it was loading the cluster 
> config from previous test...

Out of curiosity, how is the TemporaryFolder being used here?  Isn't there a 
TemporaryFolder rule that needs to be created and used?  This looks like it's 
just doing new File().mkdir()?


- Jason


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


On Aug. 11, 2016, 10:34 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51010/
> ---
> 
> (Updated Aug. 11, 2016, 10:34 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, Kirk 
> Lund, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added test to validate Cluster configuration support for Lucene indexes.
> 
> Added a new Rule to create Distributed System with test/custom 
> configuration...The "LocatorServerConfigurationRule" makes it easier to 
> create a Locator or cluster nodes with required configuration.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51010/diff/
> 
> 
> Testing
> ---
> 
> Run the newly added test.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 51044: GEODE-1737: gfsh query on region that does not exist may cause NPE

[Jason Huynh]

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




geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
 (line 1315)
<https://reviews.apache.org/r/51044/#comment211951>

Oh interesting, it looks like this shouldn't be an issue after all.  I'll 
remove this jira ticket.  It was originally an issue for the GemFire product 
but it looks like someone already noticed and fixed it.


- Jason Huynh


On Aug. 12, 2016, 4:44 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51044/
> ---
> 
> (Updated Aug. 12, 2016, 4:44 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If a query on a region did not exist, a null is returned.  This causes an npe 
> that needs to be checked for.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
>  ae87b72 
> 
> Diff: https://reviews.apache.org/r/51044/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 51044: GEODE-1737: gfsh query on region that does not exist may cause NPE

[Jason Huynh]

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

Review request for geode, anilkumar gingade and nabarun nag.


Repository: geode


Description
---

If a query on a region did not exist, a null is returned.  This causes an npe 
that needs to be checked for.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
 ae87b72 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 51010: GEODE-11 Test for Cluster Configuration changes for Lucene index.

[Jason Huynh]

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




geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
 (line 82)
<https://reviews.apache.org/r/51010/#comment211869>

Is this method needed?



geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
 (line 115)
<https://reviews.apache.org/r/51010/#comment211874>

Should we limit the index here or rely on the host.getVM for which index 
values are valid?

If we do want to limit it in this rule, maybe break out 1 and 3 into 
constants?



geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
 (line 125)
<https://reviews.apache.org/r/51010/#comment211875>

If mcast port is set... what will happen?  This checks to see if it's not 
set.



geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
 (line 129)
<https://reviews.apache.org/r/51010/#comment211878>

Should this be a SerializableRunnable?



geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
 (line 156)
<https://reviews.apache.org/r/51010/#comment211877>

Should this be a SerializableRunnable?  It doesn't seem to matter what 
happens in this method, it will always return true and we are not actually 
using the return value anyways



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 (line 83)
<https://reviews.apache.org/r/51010/#comment211879>

I think these methods are self documenting, probably can remove the comments



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 (line 103)
<https://reviews.apache.org/r/51010/#comment211864>

Should we seperate this test into two tests?  One for when the new node is 
created in the group and one for when it is out of the group?  This test is 
named in a way that made me think every new node was in the group and should 
have the index, but vm3 is outside the group and verifies it does not get an 
index



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 (line 154)
<https://reviews.apache.org/r/51010/#comment211865>

What currently happens if we have RegionShortcut.REPLICATE?  The index 
itself is just not created but everything else keeps on running?



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 (line 189)
<https://reviews.apache.org/r/51010/#comment211863>

Will this dir and clusterConfigDir get cleaned up after the test is run?  
Should this use a TemporaryFolder rule or something else?


- Jason Huynh


On Aug. 11, 2016, 10:34 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51010/
> ---
> 
> (Updated Aug. 11, 2016, 10:34 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, Kirk 
> Lund, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added test to validate Cluster configuration support for Lucene indexes.
> 
> Added a new Rule to create Distributed System with test/custom 
> configuration...The "LocatorServerConfigurationRule" makes it easier to 
> create a Locator or cluster nodes with required configuration.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51010/diff/
> 
> 
> Testing
> ---
> 
> Run the newly added test.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: [VOTE] Release Apache Geode (incubating) 1.0.0-incubating.M3 - RC7

[Jason Huynh]

+1

- built from source distribution
- started locator, server, listed members and created regions in gfsh from
binary
- started locator and server from source built gfsh


On Fri, Aug 5, 2016 at 9:58 AM Dan Smith  wrote:

> +1
>
> Verified
> * Successful precheckin run of this release -
> https://builds.apache.org/job/
> Geode-release/24/ 
> * Signatures
> * Basic gfsh commands with binary dist
> * Built from source dist
> * Basic CRUD test with maven artifacts
> * No jars in source dist
>
> -Dan
>
> On Thu, Aug 4, 2016 at 6:02 PM, William Markito 
> wrote:
>
> > All,
> >
> > This is the seventh release candidate Apache Geode, version
> > 1.0.0-incubating.M3.
> >
> > We're including the feedback received in RC6 including a fix
> > (83f97ceef52febf92ef7737726548aa0865c1a59) to run REST API tests.
> >
> > Thanks to all the community members to drive towards this milestone!
> >
> > It fixes the following issues:
> >https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> > projectId=12318420=12335358
> >
> > *** Please download, test and vote by Monday, August 8, 0800 hrs US
> > Pacific.
> >
> > Note that we are voting upon the source (tag):
> >rel/v1.0.0-incubating.M3.RC7
> >
> > https://git-wip-us.apache.org/repos/asf?p=incubator-geode.gi
> > t;a=tag;h=refs/tags/rel/v1.0.0-incubating.M3.RC7
> >  > git;a=tag;h=refs/tags/rel/v1.0.0-incubating.M3.RC7>
> >
> > Commit ID: 83f97ceef52febf92ef7737726548aa0865c1a59
> >
> > https://git-wip-us.apache.org/repos/asf?p=incubator-geode.gi
> > t;a=commit;h=83f97ceef52febf92ef7737726548aa0865c1a59
> >
> > Source and binary files:
> > https://dist.apache.org/repos/dist/dev/incubator/geode/1.0.0
> > -incubating.M3.RC7
> >
> > For this release the documentation on how to install and use Apache Geode
> > are hosted
> > on pivotal.io:
> >http://geode.docs.pivotal.io
> >
> > Maven staging repo:
> > https://repository.apache.org/content/repositories/orgapachegeode-1011
> >
> > Geode's KEYS file containing PGP keys we use to sign the release:
> >https://github.com/apache/incubator-geode/blob/release/
> > 1.0.0-incubating.M3/KEYS
> >
> > Release Key: pub  4096R/7AAED8BB 2016-07-13
> > Fingerprint: 8E06 B711 DB13 3AE7 0CC1  ABDE 6A14 F0BC 7AAE D8BB
> >
> > Thanks,
> >
> > --
> > ~/William
> >
>

Review Request 50858: GEODE-1675: Modifying tc server templates and adding the tomcat 8 jars to the tc server distributions

[Jason Huynh]

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

Review request for geode, Jens Deppe and William Markito.


Repository: geode


Description
---

This change creates a tcserver32.zip and adds the new tomcat 85 support to 
this. It leaves the other distributions alone.


Diffs
-

  extensions/geode-modules-assembly/build.gradle f037aad 
  
extensions/geode-modules-assembly/release/tcserver/geode-cs-tomcat-85/context-fragment.xml
 PRE-CREATION 
  
extensions/geode-modules-assembly/release/tcserver/geode-p2p-tomcat-85/context-fragment.xml
 PRE-CREATION 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 50818: GEODE-1728: Removing the dispatcher elements from the session filter

[Jason Huynh]

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


Fix it, then Ship it!





extensions/geode-modules-session/src/test/java/com/gemstone/gemfire/modules/session/internal/filter/CommonTests.java
 (line 590)
<https://reviews.apache.org/r/50818/#comment210954>

type: Multiple



extensions/geode-modules-session/src/test/java/com/gemstone/gemfire/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
 (line 1251)
<https://reviews.apache.org/r/50818/#comment210955>

I think this is a typo too? and maybe remove the commented out code?


- Jason Huynh


On Aug. 4, 2016, 9:07 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50818/
> ---
> 
> (Updated Aug. 4, 2016, 9:07 p.m.)
> 
> 
> Review request for geode and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The geode session filter is being applied to all requests. Because of
> that, all request objects should already be wrapped in a RequestWrapper.
> By adding these dispatcher elements to the filter, the filter was being
> applied multiple times to the request when a request is forwarded or
> included using the RequestDipatcher. That is unnecessary, and
> problematic if we can't determine that the request has already been
> wrapped.
> 
> GEODE-1728: Recursively checking for wrapped http sessions
> 
> Our check for wrapped http sessions in the session caching module was
> not working if a request was wrapped by our module and then by a third
> party filter that also wrapped the request.
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-session/build.gradle 
> 4045a69fe1eb0d287a16954cf4fdf4ed4b18a6e0 
>   
> extensions/geode-modules-session/src/main/java/com/gemstone/gemfire/modules/session/filter/SessionCachingFilter.java
>  7abc25370660281741423aed87326cf45e4292ad 
>   
> extensions/geode-modules-session/src/main/java/com/gemstone/gemfire/modules/session/installer/Installer.java
>  7ba5b34abc86522df94de849777b5ae2a8731221 
>   
> extensions/geode-modules-session/src/test/java/com/gemstone/gemfire/modules/session/installer/InstallerJUnitTest.java
>  PRE-CREATION 
>   
> extensions/geode-modules-session/src/test/java/com/gemstone/gemfire/modules/session/internal/filter/CommonTests.java
>  c341c6f167dc5d5d5c18c7fd7976076ec43381f7 
>   
> extensions/geode-modules-session/src/test/java/com/gemstone/gemfire/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
>  da5673523f8c359bdcfa3e36513e3e4355667b5b 
>   
> extensions/geode-modules-session/src/test/resources/com/gemstone/gemfire/modules/session/installer/InstallerJUnitTest.web.xml
>  PRE-CREATION 
>   
> extensions/geode-modules-session/src/test/resources/com/gemstone/gemfire/modules/session/installer/InstallerJUnitTest.web.xml.expected
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50818/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 50811: GEODE-1241: Fixing the misspelt words in the geode-wan

[Jason Huynh]

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


Ship it!




Ship It!


geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/GatewaySenderBatchOp.java
 
<https://reviews.apache.org/r/50811/#comment210901>

YES! This is the if/else that always bugged me


- Jason Huynh


On Aug. 4, 2016, 5:25 p.m., nabarun nag wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50811/
> ---
> 
> (Updated Aug. 4, 2016, 5:25 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Dave Barnes, 
> Jason Huynh, Kirk Lund, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is related to the ticket filed by Kirk which mentioned a list of classes 
> and methods with misspelt words. Using the inspect tool in the IntelliJ IDEA, 
> I was able to a get a list of 231 spelling mistake and missed  camelCasing in 
> the geode-wan module. These are fixed in this patch.  Remaining modules will 
> be fixed in the future. NOTE: this patch doesn't fix meaningless variable 
> names like 'serv' or 'cus'. They will be fixed in the future.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
>  2254a89 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/GatewaySenderBatchOp.java
>  ef5f816 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorDiscovery.java
>  bbaed1d 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorHelper.java
>  1f36b12 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/WanLocatorDiscovererImpl.java
>  cde1e15 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java
>  faef65c 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderFactoryImpl.java
>  3e3244e 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderImpl.java
>  8f5b728 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/RemoteSerialGatewaySenderEventProcessor.java
>  56f3b39 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/cache/CacheXml70GatewayDUnitTest.java
>  3dfbfe9 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/cache/CacheXml80GatewayDUnitTest.java
>  f229e0f 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/UpdateVersionDUnitTest.java
>  f076aef 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/CacheClientNotifierDUnitTest.java
>  24b3a40 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
>  79648e1 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentParallelGatewaySenderDUnitTest.java
>  ee9edf8 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentParallelGatewaySenderOperation_2_DUnitTest.java
>  106ab4b 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentWANPropagation_1_DUnitTest.java
>  PRE-CREATION 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentWANPropagation_2_DUnitTest.java
>  PRE-CREATION 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentWANPropogation_1_DUnitTest.java
>  c01cb22 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentWANPropogation_2_DUnitTest.java
>  06e0ecb 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/CommonParallelGatewaySenderDUnitTest.java
>  6ad485f 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/NewWANConcurrencyCheckForDestroyDUnitTest.java
>  b396e85 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/PDXNewWanDUnitTest.java
>  0f76ce8 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/ReplicatedRegion_ParallelWANPersistenceDUnitTest.java
>  e2810f1 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/ReplicatedRegion_ParallelWANPropagationDUnitTest.java
>  PRE-CREATION 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/Re

Review Request 50615: GEODE-1676: Multiple Not Equals in query with Compact Range Index returns incorrect results

[Jason Huynh]

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

Review request for geode, anilkumar gingade and nabarun nag.


Repository: geode


Description
---

The keysToRemove set was being removed from during iteration of the index.  
This is as expected, however a copy of the keysToRemove should be made first 
because the same set will be passed to the next bucket.  We would end up 
removing from the original set and as we continued the query to the next 
bucket, it would eventually be an empty set.

PdxString and Strings were not being properly compared in TypeUtils (which is 
used by CompactRangeIndexes, among others)


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/MemoryIndexStore.java
 83f8c02 
  
geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtils.java
 14c798f 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtilTest.java
 PRE-CREATION 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 50587: Merge from 82 and couple of other perf related improvements

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On July 28, 2016, 11:43 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50587/
> ---
> 
> (Updated July 28, 2016, 11:43 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Bruce Schuchardt, Jason Huynh, and 
> Jacob Barrett.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This includes merge from 82. Please check your chengaes with 82
> 
> 
> jason 79be29188b440e217cda50e41af853f882dd9193
> jason 49e6458fcc2aa294fc5dfd41ba47bddc92f29dfe
> jason 30ba06dbc5ee06dab7b0ff7b7275619612b7a868
> jacob 1544bda2c722bddf94db6f2e807b302f9b5df303
> Jacob 97cd29a821b770a54412c4abf20f5df0398e9405
> barry 37a942b9ab33e301f3bd41270a63ad0433f184bd
> barry 3210ed80258a6518f8b92ae9adfd96e18db1f218
> barry 486a653d037ed3f13c7061abe887d6ab306261d4
> barry 35a6c8b6236ef329fe5ace121c657b529725ec32
> barry 09f9b953b07b26bc3a19cc00c22f8ed047f6f6cf
> barry 49fc39dc7224fe42f73b6363de66b6f8f6ed7be6
> Amogh changes 18dd055dc75f94c460fba81d7dfdc2617fafb0a5
> b3322a097cdf0d9ecaf94c600b61c7d62d772cea
> 
> Other improvement:We take lock on key while doing op on BucketRegion. 
> In that case wenotify to other thread only when there is thread waiting for 
> it.
> Modified condition to log message.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java
>  f3cb3d6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/BucketRegion.java
>  abe38b6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DistributedCacheOperation.java
>  f51717d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterRoutingInfo.java
>  7f5b587 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerMap.java
>  084140f 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerRegion.java
>  eeefaee 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HAContainerWrapper.java
>  b0f3e45 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HARegionQueue.java
>  85b50a1 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/LockObject.java
>  612b71a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java
>  d351569 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ClientUpdateMessageImpl.java
>  0fb915e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java
>  459cf5f6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java
>  1c3819e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderEventProcessor.java
>  8b6a700 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderQueue.java
>  ccdf42a 
>   
> geode-cq/src/main/java/com/gemstone/gemfire/cache/query/internal/cq/CqServiceImpl.java
>  e1e158c 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfDUnitTest.java
>  ef9e61b 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java
>  dcd3915 
> 
> Diff: https://reviews.apache.org/r/50587/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 50361: GEODE-1687: NPE during CqStatusListener method invocation.

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On July 22, 2016, 10:02 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50361/
> ---
> 
> (Updated July 22, 2016, 10:02 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, 
> William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1687: NPE during CqStatusListener method invocation.
> 
> NPE in: CqServiceImpl.java -> invokeCqsConnected()
> Line#2011
> Pool cqPool = cQuery.getCQProxy().getPool(); 
> 
> 
> When a connection relating to CQ is established or dropped, CqStatusListner 
> is invoked to indicate connection/subscription status. This allows 
> application to handle if all the subscription connection is lost for CQs.
> 
> Looking at the code path for CqStatusListener there is a chance of NPE 
> getting thrown in CqServiceImpl.invokeCqsConnected when CQ is executed using 
> the Bridge loader interface (old way of client-server
> connection)
> 
> When a pool or bridge-loader connection is made or dropped, the code gets all 
> the CQs; checks if the CQ is registered using that connection/pool name and 
> invokes the CqStatusListner...
> 
> The code to check if CQ is using the corresponding pool:
> Pool cqPool = cQuery.getCQProxy().getPool();
> 
> If the CQ is getting executed using Bridge loader, the CqProxy for CQ is not 
> set when its constructed; its set when it gets executedDuring this time 
> the call to cQuery.getCQProxy() could throw NPE.
> 
> 
> Diffs
> -
> 
>   
> geode-cq/src/main/java/com/gemstone/gemfire/cache/query/internal/cq/CqServiceImpl.java
>  e1e158c 
> 
> Diff: https://reviews.apache.org/r/50361/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 50364: Adding support for Tomcat 8.5.4 for the session module

[Jason Huynh]

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

(Updated July 26, 2016, 6:37 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, Jens Deppe, William 
Markito, nabarun nag, Dan Smith, and xiaojian zhou.


Changes
---

Latest changes.  Removed comments. Needed to change a DeltaSession to a 
DeltaSessionInterface.
All current DeltaSessionInterface implementors extend StandardSession, so will 
throw an exception if a non StandardSession is added and used.


Repository: geode


Description
---

Currently, we do not support Tomcat 8.5.4.  This is a first cut attempt to at 
doing so.

This patch will probably have a lot of debug statements to clean up as well as 
cleaning up a bunch of code.  This is the diff right after getting everything 
compiling and manually tested.

Extracted DeltaSession into an interface, this allowed us to keep support for 
Tomcat 6 and 7
-- This was needed because the value attributes is now a ConcurrentMap and no 
longer a Map
Refactored LifecycleListener and moved it to the specific DeltaSessionManagers 
as it no longer exists in Tomcat 8
Created new EmbeddedTomcat as the old Embedded class is no longer present in 
Tomcat 8
Setup of tests needed to be changed because a lot of the setup code is no 
longer needed or needed in a different order


Diffs (updated)
-

  extensions/geode-modules-assembly/build.gradle 412ba09 
  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
 d1f44bb 
  extensions/geode-modules-tomcat8/build.gradle PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession8.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat8DeltaSessionManager.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/EmbeddedTomcat8.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsTomcat8Base.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/resources/tomcat/conf/tomcat-users.xml
 PRE-CREATION 
  extensions/geode-modules-tomcat8/src/test/resources/tomcat/logs/.gitkeep 
PRE-CREATION 
  extensions/geode-modules-tomcat8/src/test/resources/tomcat/temp/.gitkeep 
PRE-CREATION 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/gatewaydelta/GatewayDeltaDestroyEvent.java
 18bfe8b 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/CommitSessionValve.java
 0ae17f2 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession.java
 c81a232 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionFacade.java
 1ac4da2 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionInterface.java
 PRE-CREATION 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionManager.java
 92d9ef6 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat6DeltaSessionManager.java
 d5b5991 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/callback/SessionExpirationCacheListener.java
 dff6d58 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionAttributeEvent.java
 90b6f28 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionAttributeEventBatch.java
 47df071 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionDestroyAttributeEvent.java
 989474f 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionUpdateAttributeEvent.java
 6678e55 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/util/ContextMapper.java
 3b7b2de 
  gradle/dependency-versions.properties aafacef 
  settings.gradle ca34692 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 50420: there is a race that cache is still initializing, some components are not ready

[Jason Huynh]

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




geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
 (line 125)
<https://reviews.apache.org/r/50420/#comment209226>

Should resumeSender call waitForRunningStatus()?  Long term we should 
probably fix these asynch pause/resume calls and make them synchronous...


- Jason Huynh


On July 25, 2016, 9:42 p.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50420/
> ---
> 
> (Updated July 25, 2016, 9:42 p.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: GEODE-1671
> https://issues.apache.org/jira/browse/GEODE-1671
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Need to add Awaitility.waitAtMost
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
>  da281ff 
> 
> Diff: https://reviews.apache.org/r/50420/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Review Request 50419: GEODE-1617: Metadata region was not having a cache listener attached

[Jason Huynh]

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

Review request for geode, Kevin Duling and William Markito.


Repository: geode


Description
---

A minor refactoring accidently missed out on a cache listener.


Diffs
-

  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/util/CreateRegionFunction.java
 342968c 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 50364: Adding support for Tomcat 8.5.4 for the session module

[Jason Huynh]

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

(Updated July 25, 2016, 6:32 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, Jens Deppe, William 
Markito, nabarun nag, Dan Smith, and xiaojian zhou.


Changes
---

Removed commented out code, added a missing header.  Removed debug type logging


Repository: geode


Description
---

Currently, we do not support Tomcat 8.5.4.  This is a first cut attempt to at 
doing so.

This patch will probably have a lot of debug statements to clean up as well as 
cleaning up a bunch of code.  This is the diff right after getting everything 
compiling and manually tested.

Extracted DeltaSession into an interface, this allowed us to keep support for 
Tomcat 6 and 7
-- This was needed because the value attributes is now a ConcurrentMap and no 
longer a Map
Refactored LifecycleListener and moved it to the specific DeltaSessionManagers 
as it no longer exists in Tomcat 8
Created new EmbeddedTomcat as the old Embedded class is no longer present in 
Tomcat 8
Setup of tests needed to be changed because a lot of the setup code is no 
longer needed or needed in a different order


Diffs (updated)
-

  extensions/geode-modules-assembly/build.gradle 412ba09 
  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
 d1f44bb 
  extensions/geode-modules-tomcat8/build.gradle PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession8.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat8DeltaSessionManager.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/EmbeddedTomcat8.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsTomcat8Base.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/resources/tomcat/conf/tomcat-users.xml
 PRE-CREATION 
  extensions/geode-modules-tomcat8/src/test/resources/tomcat/logs/.gitkeep 
PRE-CREATION 
  extensions/geode-modules-tomcat8/src/test/resources/tomcat/temp/.gitkeep 
PRE-CREATION 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/gatewaydelta/GatewayDeltaDestroyEvent.java
 18bfe8b 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/AbstractCache.java
 c14f829 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/CommitSessionValve.java
 0ae17f2 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession.java
 c81a232 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionFacade.java
 1ac4da2 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionInterface.java
 PRE-CREATION 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionManager.java
 92d9ef6 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat6DeltaSessionManager.java
 d5b5991 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/callback/SessionExpirationCacheListener.java
 dff6d58 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionAttributeEvent.java
 90b6f28 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionAttributeEventBatch.java
 47df071 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionDestroyAttributeEvent.java
 989474f 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionUpdateAttributeEvent.java
 6678e55 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/util/ContextMapper.java
 3b7b2de 
  gradle/dependency-versions.properties aafacef 
  settings.gradle ca34692 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 50304: GEODE-1674: Using a immuatable object to pass the HashIndexSet and its mask

[Jason Huynh]

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


Ship it!




can you rename tempHashIndexSetProperties to newHashIndexSetProperties,
also remove the starting _ for the variable names?
Lastly, computeNumFree can be added as a method into the new 
HashIndexSetProperties

- Jason Huynh


On July 21, 2016, 5:39 p.m., nabarun nag wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50304/
> ---
> 
> (Updated July 21, 2016, 5:39 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * Immutable ojects pairs the mask and set together
> * There is no race condition in which the mask differs from the set leading 
> to any ArrayOutOfBoundsException.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/HashIndexSet.java
>  5ed724b 
>   
> geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/HashIndexJUnitTest.java
>  e2a8643 
>   
> geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/HashIndexSetJUnitTest.java
>  7f96fde 
> 
> Diff: https://reviews.apache.org/r/50304/diff/
> 
> 
> Testing
> ---
> 
> Precheck-in with no failures.
> 
> 
> Thanks,
> 
> nabarun nag
> 
>

Review Request 50364: Adding support for Tomcat 8.5.4 for the session module

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, Jens Deppe, William 
Markito, nabarun nag, Dan Smith, and xiaojian zhou.


Repository: geode


Description
---

Currently, we do not support Tomcat 8.5.4.  This is a first cut attempt to at 
doing so.

This patch will probably have a lot of debug statements to clean up as well as 
cleaning up a bunch of code.  This is the diff right after getting everything 
compiling and manually tested.

Extracted DeltaSession into an interface, this allowed us to keep support for 
Tomcat 6 and 7
-- This was needed because the value attributes is now a ConcurrentMap and no 
longer a Map
Refactored LifecycleListener and moved it to the specific DeltaSessionManagers 
as it no longer exists in Tomcat 8
Created new EmbeddedTomcat as the old Embedded class is no longer present in 
Tomcat 8
Setup of tests needed to be changed because a lot of the setup code is no 
longer needed or needed in a different order


Diffs
-

  extensions/geode-modules-assembly/build.gradle 412ba09 
  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
 d1f44bb 
  extensions/geode-modules-tomcat8/build.gradle PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession8.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat8DeltaSessionManager.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/EmbeddedTomcat8.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsTomcat8Base.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/java/com/gemstone/gemfire/modules/session/Tomcat8SessionsJUnitTest.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat8/src/test/resources/tomcat/conf/tomcat-users.xml
 PRE-CREATION 
  extensions/geode-modules-tomcat8/src/test/resources/tomcat/logs/.gitkeep 
PRE-CREATION 
  extensions/geode-modules-tomcat8/src/test/resources/tomcat/temp/.gitkeep 
PRE-CREATION 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/gatewaydelta/GatewayDeltaDestroyEvent.java
 18bfe8b 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/bootstrap/AbstractCache.java
 c14f829 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/CommitSessionValve.java
 0ae17f2 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession.java
 c81a232 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionFacade.java
 1ac4da2 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionInterface.java
 PRE-CREATION 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSessionManager.java
 92d9ef6 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat6DeltaSessionManager.java
 d5b5991 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/callback/SessionExpirationCacheListener.java
 dff6d58 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionAttributeEvent.java
 90b6f28 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionAttributeEventBatch.java
 47df071 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionDestroyAttributeEvent.java
 989474f 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionUpdateAttributeEvent.java
 6678e55 
  
extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/util/ContextMapper.java
 3b7b2de 
  gradle/dependency-versions.properties 196a4f8 
  settings.gradle ca34692 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 50361: GEODE-1687: NPE during CqStatusListener method invocation.

[Jason Huynh]

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




geode-cq/src/main/java/com/gemstone/gemfire/cache/query/internal/cq/CqServiceImpl.java
 (line 2008)
<https://reviews.apache.org/r/50361/#comment209067>

Would it make sense to check the cq state prior to this and if it's not 
running to not check or send status?


- Jason Huynh


On July 22, 2016, 10:02 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50361/
> ---
> 
> (Updated July 22, 2016, 10:02 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, 
> William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1687: NPE during CqStatusListener method invocation.
> 
> NPE in: CqServiceImpl.java -> invokeCqsConnected()
> Line#2011
> Pool cqPool = cQuery.getCQProxy().getPool(); 
> 
> 
> When a connection relating to CQ is established or dropped, CqStatusListner 
> is invoked to indicate connection/subscription status. This allows 
> application to handle if all the subscription connection is lost for CQs.
> 
> Looking at the code path for CqStatusListener there is a chance of NPE 
> getting thrown in CqServiceImpl.invokeCqsConnected when CQ is executed using 
> the Bridge loader interface (old way of client-server
> connection)
> 
> When a pool or bridge-loader connection is made or dropped, the code gets all 
> the CQs; checks if the CQ is registered using that connection/pool name and 
> invokes the CqStatusListner...
> 
> The code to check if CQ is using the corresponding pool:
> Pool cqPool = cQuery.getCQProxy().getPool();
> 
> If the CQ is getting executed using Bridge loader, the CqProxy for CQ is not 
> set when its constructed; its set when it gets executedDuring this time 
> the call to cQuery.getCQProxy() could throw NPE.
> 
> 
> Diffs
> -
> 
>   
> geode-cq/src/main/java/com/gemstone/gemfire/cache/query/internal/cq/CqServiceImpl.java
>  e1e158c 
> 
> Diff: https://reviews.apache.org/r/50361/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 50089: GEODE-1669: Performance issue with Interest registration with impact on client side event processing

[Jason Huynh]

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




geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 
(line 565)
<https://reviews.apache.org/r/50089/#comment208024>

I think this goes along with Bruce's review, but possibily not as big of a 
problem, but the keysUnregistered may contain keys that were never actually in 
the interest list


- Jason Huynh


On July 15, 2016, 10:14 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50089/
> ---
> 
> (Updated July 15, 2016, 10:14 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, 
> Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun 
> nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1669: Performance issue with Interest registration with impact on 
> client side event processing
> 
> This is in relation to internal ticket GEM-437.
> 
> The client interests are managed in "FilterProfile" class on server side. 
> These are maintained using the concurrent data structures CopyOnWriteHashSet 
> and CopyOnWriteHashMap...
> 
> When set of keys are registered from client, the keys are added to 
> CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); 
> Where a new HashSet is created each time when an entry is added, which could 
> impact performance based on the number of keys registered.
> 
> This will have implication on the client side event processing; as the event 
> processing gets blocked when a interest registration is in progress by other 
> thread.
> 
> Change/Solution:
> 
> Instead of adding the keys one by one; the keys are added using addAll(). 
> This avoids creating one hashset per interest key, which was making the 
> registration process to take longer time.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java
>  08a6484 
> 
> Diff: https://reviews.apache.org/r/50089/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Review Request 49772: CI failure: LuceneQueriesClientDUnitTest.entriesFlushedToIndexAfterWaitForFlushCalled

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

The test was failing on a negative check to make sure the queue actually had 
contents.  There were cases where the queue could be drained due to the 
"slowness"/async pausing of the sender.

I also moved and rewrote this test out from LuceneQueriesBase as I didn't think 
we needed to test flushing on multiple base region types.  Not sure if commits 
stat is the correct one to check.  The docs stat was returning 0.

Also noticed that the pdx -> jsonstring conversion done in LuceneTestUtilities 
wasn't actually using the converted jsonstring. Instead we could avoid that 
entire if statement altogether.


Diffs
-

  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java
 7d7fa3d 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
 e817d3b 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
 8aca11c 

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


Testing
---

geode-lucene:precheckin


Thanks,

Jason Huynh

Re: Review Request 49102: WAN Ack reader thread needs to be shut down before sending a close connection

[Jason Huynh]

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

(Updated July 6, 2016, 8:42 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Changes
---

Previous changes caused a few dunit failures.  This one removed the offending 
code.


Repository: geode


Description
---

When closing a sender, the close connection message is sent on the same 
connection that is used by the ack reader thread.  This causes an issue as two 
threads are now reading off the same socket concurrently.  The fix is to 
prevent this from happening but to do so, the input stream needs to be closed 
(to free up from a socket read()).  
The dispatcher also needs to shut down before the close connection is sent out 
or it will spawn off another ack reader thread.


Diffs (updated)
-

  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySenderEventProcessor.java
 ce08e8d 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderEventProcessor.java
 07a3be5 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/ConcurrentSerialGatewaySenderEventProcessor.java
 ff810ec 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java
 b178192 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 358ffaf 

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


Testing
---


Thanks,

Jason Huynh

Re: Build failed in Jenkins: Geode-nightly #518

[Jason Huynh]

I'll revert the changes I made, it looks like they may be causing some
timing/instability with some wan tests

On Sun, Jul 3, 2016 at 7:30 AM Apache Jenkins Server <
jenk...@builds.apache.org> wrote:

> See 
>
> --
> [...truncated 689 lines...]
> :geode-rebalancer:integrationTest
> :geode-wan:assemble
> :geode-wan:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-wan:processTestResources
> :geode-wan:testClasses
> :geode-wan:checkMissedTests
> :geode-wan:test
> :geode-wan:check
> :geode-wan:build
> :geode-wan:distributedTest
>
> com.gemstone.gemfire.internal.cache.wan.serial.SerialWANStatsDUnitTest >
> testReplicatedSerialPropagationWithMultipleDispatchers FAILED
> com.gemstone.gemfire.test.dunit.RMIException: While invoking
> com.gemstone.gemfire.internal.cache.wan.serial.SerialWANStatsDUnitTest$$Lambda$1118/898548188.run
> in VM 4 running on Host asf902.gq1.ygridcore.net with 8 VMs
> at com.gemstone.gemfire.test.dunit.VM.invoke(VM.java:389)
> at com.gemstone.gemfire.test.dunit.VM.invoke(VM.java:355)
> at com.gemstone.gemfire.test.dunit.VM.invoke(VM.java:293)
> at
> com.gemstone.gemfire.internal.cache.wan.serial.SerialWANStatsDUnitTest.testReplicatedSerialPropagationWithMultipleDispatchers(SerialWANStatsDUnitTest.java:149)
>
> Caused by:
> java.lang.AssertionError: expected:<0> but was:<1>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotEquals(Assert.java:834)
> at org.junit.Assert.assertEquals(Assert.java:645)
> at org.junit.Assert.assertEquals(Assert.java:631)
> at
> com.gemstone.gemfire.internal.cache.wan.WANTestBase.checkBatchStats(WANTestBase.java:1303)
> at
> com.gemstone.gemfire.internal.cache.wan.serial.SerialWANStatsDUnitTest.lambda$testReplicatedSerialPropagationWithMultipleDispatchers$84651174$12(SerialWANStatsDUnitTest.java:149)
>
> com.gemstone.gemfire.internal.cache.wan.serial.SerialGatewaySenderOperationsDUnitTest
> > testStopOneSender_StartAnotherSender FAILED
> java.lang.AssertionError: An exception occurred during asynchronous
> invocation.
>
> Caused by:
>
> com.gemstone.gemfire.cache.client.internal.pooling.ConnectionDestroyedException
>
> com.gemstone.gemfire.internal.cache.wan.serial.SerialGatewaySenderOperationsDUnitTest
> > testStopOneSerialGatewaySenderBothPrimary FAILED
> com.gemstone.gemfire.test.dunit.RMIException: While invoking
> com.gemstone.gemfire.internal.cache.wan.serial.SerialGatewaySenderOperationsDUnitTest$$Lambda$194/420688668.run
> in VM 2 running on Host asf902.gq1.ygridcore.net with 8 VMs
> at com.gemstone.gemfire.test.dunit.VM.invoke(VM.java:389)
> at com.gemstone.gemfire.test.dunit.VM.invoke(VM.java:355)
> at com.gemstone.gemfire.test.dunit.VM.invoke(VM.java:293)
> at
> com.gemstone.gemfire.internal.cache.wan.serial.SerialGatewaySenderOperationsDUnitTest.testStopOneSerialGatewaySenderBothPrimary(SerialGatewaySenderOperationsDUnitTest.java:310)
>
> Caused by:
> com.jayway.awaitility.core.ConditionTimeoutException: Condition
> defined as a lambda expression in
> com.gemstone.gemfire.internal.cache.wan.WANTestBase that uses int,
> intcom.gemstone.gemfire.cache.Region Expected region entries: 200 but
> actual entries: 0 present region keyset [] expected:<200> but was:<0>
> within 3 milliseconds.
> at
> com.jayway.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:122)
> at
> com.jayway.awaitility.core.AssertionCondition.await(AssertionCondition.java:117)
> at
> com.jayway.awaitility.core.AssertionCondition.await(AssertionCondition.java:32)
> at
> com.jayway.awaitility.core.ConditionFactory.until(ConditionFactory.java:764)
> at
> com.jayway.awaitility.core.ConditionFactory.until(ConditionFactory.java:603)
> at
> com.gemstone.gemfire.internal.cache.wan.WANTestBase.validateRegionSize(WANTestBase.java:2764)
> at
> com.gemstone.gemfire.internal.cache.wan.WANTestBase.validateRegionSize(WANTestBase.java:2751)
> at
> com.gemstone.gemfire.internal.cache.wan.serial.SerialGatewaySenderOperationsDUnitTest.lambda$testStopOneSerialGatewaySenderBothPrimary$84651174$5(SerialGatewaySenderOperationsDUnitTest.java:310)
>
> java.lang.AssertionError: Suspicious strings were written to the log
> during this run.
> Fix the strings or use IgnoredException.addIgnoredException to ignore.
> ---
> Found suspect string in log4j at line 5735
>
> [fatal 2016/07/03 12:54:07.611 UTC  GatewaySender_ln> 

Review Request 49471: GEODE-1374: Flaky tests will now have junit xml recorded into seperate directory

[Jason Huynh]

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

Review request for geode, Kirk Lund, nabarun nag, and Dan Smith.


Repository: geode


Description
---

Flaky xml results were overwriting non flaky test results that had the same FQCN
We now can write these xml results into their own directory and have jenkins 
crawl this new folder as well


Diffs
-

  gradle/test.gradle eb10cad 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 49102: WAN Ack reader thread needs to be shut down before sending a close connection

[Jason Huynh]

> On June 29, 2016, 9:02 p.m., anilkumar gingade wrote:
> > Nice to have unit test for this.

Agreed, however this would require quite a few test hooks in product code.  
There are multiple scenarios that can cause this issue.  I've inserted a "test 
case" to the ticket that will reproduce the issue without test hooks but it 
would not really be a test I would want to check in.

The problem is that there are multiple scenarios that can cause this to occur.  
Three different threads that would need to have test hooks to sync each other 
at specific points.   The ack reader thread can be stuck on a socket read() or 
it can have already processed the header for the read, or it could be 
interrupted right before the read or it could be dead.  All the while the 
dispatcher thread could be running and spawn a new ack reader thread (if it 
were dead) on a new connection or sending off a new batch.  The gateway stopper 
thread could possibly send a close connectio on the old socket or if the ack 
reader thread was recreated, it would send on the new socket. 

All those scenarios now are invalid... we have changed the shut down to 
shutdown the dispatcher thread (not allowing it to spawn a new ack reader 
thread if it's shutting down).  The connection is also being closed which will 
force the ack reader to break out of the read() or prevent it from reading 
anything else from the input stream.  All the new test hooks would be added and 
some of these scenarios are now unable to be hit.


- Jason


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


On June 22, 2016, 5:52 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49102/
> ---
> 
> (Updated June 22, 2016, 5:52 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When closing a sender, the close connection message is sent on the same 
> connection that is used by the ack reader thread.  This causes an issue as 
> two threads are now reading off the same socket concurrently.  The fix is to 
> prevent this from happening but to do so, the input stream needs to be closed 
> (to free up from a socket read()).  
> The dispatcher also needs to shut down before the close connection is sent 
> out or it will spawn off another ack reader thread.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySenderEventProcessor.java
>  ce08e8d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderEventProcessor.java
>  07a3be5 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/ConcurrentSerialGatewaySenderEventProcessor.java
>  ff810ec 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java
>  b178192 
> 
> Diff: https://reviews.apache.org/r/49102/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 49102: WAN Ack reader thread needs to be shut down before sending a close connection

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

When closing a sender, the close connection message is sent on the same 
connection that is used by the ack reader thread.  This causes an issue as two 
threads are now reading off the same socket concurrently.  The fix is to 
prevent this from happening but to do so, the input stream needs to be closed 
(to free up from a socket read()).  
The dispatcher also needs to shut down before the close connection is sent out 
or it will spawn off another ack reader thread.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySenderEventProcessor.java
 ce08e8d 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderEventProcessor.java
 07a3be5 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/ConcurrentSerialGatewaySenderEventProcessor.java
 ff810ec 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java
 b178192 

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


Testing
---


Thanks,

Jason Huynh

Re: New committer and PPMC Member: Nabarun Nag

[Jason Huynh]

Congrats!

On Tue, Jun 14, 2016 at 3:22 PM, Dave Barnes  wrote:

> Welcome, Nabarun!
>
> On Tue, Jun 14, 2016 at 3:07 PM, Dan Smith 
> wrote:
>
> > Please welcome the newest committer to the apache Geode project - Nabarun
> > Nag!
> >
> > We appreciate all the work he has already contributed thus far and are
> > looking forward to continued involvement with Geode as a committer and
> PPMC
> > member.
> >
> > -Dan
> >
>

Re: Review Request 48699: GEODE-11: Support indexing values that are Strings or Numbers

[Jason Huynh]

> On June 14, 2016, 5:07 p.m., Jason Huynh wrote:
> > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java,
> >  line 71
> > <https://reviews.apache.org/r/48699/diff/1/?file=1418740#file1418740line71>
> >
> > If they specify the REGION_VALUE_FIELD in a list of fields, that would 
> > create an indexed field out of all their searchable fields?
> > 
> > Should we check to see if the list is size of 1 also and prevent them 
> > from doing that?
> 
> Dan Smith wrote:
> The behavior the way it's written in this diff is that if 
> REGION_VALUE_FIELD is present, and the value they are putting is a String or 
> Number, then it will create an indexed field. If their value is a complex 
> object, this REGION_VALUE_FIELD will be ignored and do nothing.
> 
> If you think it's a good idea to prevent people from mixing this 
> REGION_VALUE_FIELD with other fields, I can add a check to prevent that.

No actually I think this works better in that they can have heterogenous 
objects in the odd cases where they would want it.  Just wanted to make sure we 
weren't having it index all primitive types to this field but I think I get it 
now.  Thanks!


- Jason


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


On June 14, 2016, 5:02 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48699/
> ---
> 
> (Updated June 14, 2016, 5:02 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Adding support to index values that are strings or numbers, by providing
> a special field name LuceneIndex.REGION_VALUE_FIELD that indicates the
> entire value should be indexed.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java
>  462aa7e4010afdfed06cca23025638c0abbdeff3 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
>  47c4d76eabd893c8ce6ab352eb3eb6993be78fc4 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java
>  d2b1db121fb4d626d71adaf850c33c968b8ad447 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PrimitiveSerializer.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/SerializerUtil.java
>  0ed9d5d9ff366baf0418f0b1fd7eb8e7971690e8 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
>  c26997d779e518c611de2693dad4c9799ce57691 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializerJUnitTest.java
>  3bce9045d44430cbb0c7b5c182cbc156d7d7700a 
> 
> Diff: https://reviews.apache.org/r/48699/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 48695: Allows passing in a defaultField into the query search

[Jason Huynh]

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

(Updated June 14, 2016, 5:41 p.m.)


Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and 
xiaojian zhou.


Changes
---

Updated based on review suggestions


Repository: geode


Description
---

This diff includes changes from changing the MultiFieldQueryParser to the 
StandardQueryParser

Propogates defaultField through the LuceneFunction

Currently does not handle null, we can check for null defaultFields at the 
query search level and use and internal default field?

Was not sure if we should put default field into the provider itself, then it 
would save having to pass the provider and the default field everywhere

Currently most of our tests do not rely on the default field.  The queries 
ended up specifying the field and not using the default field.

If anyone has preferences in parameter order for methods or any other 
suggestions, please let me know.


Diffs (updated)
-

  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryFactory.java
 198961a 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryProvider.java
 ef60158 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImpl.java
 385b226 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImpl.java
 a876b40 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
 62cb65c 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java
 26426ca 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java
 af8c51f 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
 92d8e8b 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
 c26997d 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java
 4bb67d2 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImplJUnitTest.java
 975b92f 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImplIntegrationTest.java
 262efaa 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
 cfd8c32 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContextJUnitTest.java
 39a4dde 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java
 c1a64ae 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesFunctionCollectorJUnitTest.java
 b7709bc 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java
 15ef449 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
 0cf8953 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 48699: GEODE-11: Support indexing values that are Strings or Numbers

[Jason Huynh]

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




geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java
 (line 71)
<https://reviews.apache.org/r/48699/#comment202689>

If they specify the REGION_VALUE_FIELD in a list of fields, that would 
create an indexed field out of all their searchable fields?

Should we check to see if the list is size of 1 also and prevent them from 
doing that?


- Jason Huynh


On June 14, 2016, 5:02 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48699/
> ---
> 
> (Updated June 14, 2016, 5:02 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Adding support to index values that are strings or numbers, by providing
> a special field name LuceneIndex.REGION_VALUE_FIELD that indicates the
> entire value should be indexed.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneService.java
>  462aa7e4010afdfed06cca23025638c0abbdeff3 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
>  47c4d76eabd893c8ce6ab352eb3eb6993be78fc4 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializer.java
>  d2b1db121fb4d626d71adaf850c33c968b8ad447 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PrimitiveSerializer.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/SerializerUtil.java
>  0ed9d5d9ff366baf0418f0b1fd7eb8e7971690e8 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
>  c26997d779e518c611de2693dad4c9799ce57691 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/HeterogeneousLuceneSerializerJUnitTest.java
>  3bce9045d44430cbb0c7b5c182cbc156d7d7700a 
> 
> Diff: https://reviews.apache.org/r/48699/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 48695: Allows passing in a defaultField into the query search

[Jason Huynh]

> On June 14, 2016, 4:55 p.m., Dan Smith wrote:
> > Hi Jason,
> > 
> > It looks good, but I think the LuceneQueryProvider should not be passed a 
> > default field. The APIs on LuceneQueryFactory should be:
> > 
> > create(String index, String region, String query, String defaultField)
> > create(String index, String region, LuceneQueryProvider) /*NO DEFAULT FIELD 
> > HERE*/
> > 
> > The reason for that is that most of the use cases for the 
> > LuceneQueryProvider don't really have a concept of a default field. A 
> > default field is really specific to this StandardQueryParser, but the user 
> > may be constructing their query programatically and have no need for one. 
> > If they do need a default field, they can embed it in their 
> > LuceneQueryProvider.
> 
> Jason Huynh wrote:
> I think that makes sense.  I added it due to the way we are 
> reusing/passing through that method.  Let me take a look and see what I can 
> do...

What I meant to say is, I think what you are suggesting makes sense ;-)


- Jason


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


On June 14, 2016, 4:01 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48695/
> ---
> 
> (Updated June 14, 2016, 4:01 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This diff includes changes from changing the MultiFieldQueryParser to the 
> StandardQueryParser
> 
> Propogates defaultField through the LuceneFunction
> 
> Currently does not handle null, we can check for null defaultFields at the 
> query search level and use and internal default field?
> 
> Was not sure if we should put default field into the provider itself, then it 
> would save having to pass the provider and the default field everywhere
> 
> Currently most of our tests do not rely on the default field.  The queries 
> ended up specifying the field and not using the default field.
> 
> If anyone has preferences in parameter order for methods or any other 
> suggestions, please let me know.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryFactory.java
>  198961a 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryProvider.java
>  ef60158 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImpl.java
>  385b226 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImpl.java
>  a876b40 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
>  62cb65c 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java
>  9567305 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContext.java
>  b0b2c60 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java
>  26426ca 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java
>  af8c51f 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  92d8e8b 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
>  c26997d 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java
>  4bb67d2 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImplJUnitTest.java
>  975b92f 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImplIntegrationTest.java
>  262efaa 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
>  cfd8c32 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContextJUnitTest.java
>  39a4dde 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java
>  c1a64ae 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntrie

Re: Review Request 48695: Allows passing in a defaultField into the query search

[Jason Huynh]

> On June 14, 2016, 4:55 p.m., Dan Smith wrote:
> > Hi Jason,
> > 
> > It looks good, but I think the LuceneQueryProvider should not be passed a 
> > default field. The APIs on LuceneQueryFactory should be:
> > 
> > create(String index, String region, String query, String defaultField)
> > create(String index, String region, LuceneQueryProvider) /*NO DEFAULT FIELD 
> > HERE*/
> > 
> > The reason for that is that most of the use cases for the 
> > LuceneQueryProvider don't really have a concept of a default field. A 
> > default field is really specific to this StandardQueryParser, but the user 
> > may be constructing their query programatically and have no need for one. 
> > If they do need a default field, they can embed it in their 
> > LuceneQueryProvider.

I think that makes sense.  I added it due to the way we are reusing/passing 
through that method.  Let me take a look and see what I can do...


- Jason


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


On June 14, 2016, 4:01 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48695/
> ---
> 
> (Updated June 14, 2016, 4:01 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This diff includes changes from changing the MultiFieldQueryParser to the 
> StandardQueryParser
> 
> Propogates defaultField through the LuceneFunction
> 
> Currently does not handle null, we can check for null defaultFields at the 
> query search level and use and internal default field?
> 
> Was not sure if we should put default field into the provider itself, then it 
> would save having to pass the provider and the default field everywhere
> 
> Currently most of our tests do not rely on the default field.  The queries 
> ended up specifying the field and not using the default field.
> 
> If anyone has preferences in parameter order for methods or any other 
> suggestions, please let me know.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryFactory.java
>  198961a 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryProvider.java
>  ef60158 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImpl.java
>  385b226 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImpl.java
>  a876b40 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
>  62cb65c 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java
>  9567305 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContext.java
>  b0b2c60 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java
>  26426ca 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java
>  af8c51f 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  92d8e8b 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
>  c26997d 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java
>  4bb67d2 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImplJUnitTest.java
>  975b92f 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImplIntegrationTest.java
>  262efaa 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
>  cfd8c32 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContextJUnitTest.java
>  39a4dde 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java
>  c1a64ae 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesFunctionCollectorJUnitTest.java
>  b7709bc 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java
>  15ef449 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
>  0cf8953 
> 
> Diff: https://reviews.apache.org/r/48695/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 48695: Allows passing in a defaultField into the query search

[Jason Huynh]

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

Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and 
xiaojian zhou.


Repository: geode


Description
---

This diff includes changes from changing the MultiFieldQueryParser to the 
StandardQueryParser

Propogates defaultField through the LuceneFunction

Currently does not handle null, we can check for null defaultFields at the 
query search level and use and internal default field?

Was not sure if we should put default field into the provider itself, then it 
would save having to pass the provider and the default field everywhere

Currently most of our tests do not rely on the default field.  The queries 
ended up specifying the field and not using the default field.

If anyone has preferences in parameter order for methods or any other 
suggestions, please let me know.


Diffs
-

  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryFactory.java
 198961a 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryProvider.java
 ef60158 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImpl.java
 385b226 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImpl.java
 a876b40 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
 62cb65c 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java
 9567305 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContext.java
 b0b2c60 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java
 26426ca 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java
 af8c51f 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
 92d8e8b 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
 c26997d 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java
 4bb67d2 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryFactoryImplJUnitTest.java
 975b92f 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImplIntegrationTest.java
 262efaa 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
 cfd8c32 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionContextJUnitTest.java
 39a4dde 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java
 c1a64ae 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesFunctionCollectorJUnitTest.java
 b7709bc 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java
 15ef449 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java
 0cf8953 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 48658: Change from MultiFieldQueryParser to StandardQueryParser

[Jason Huynh]

> On June 13, 2016, 6:08 p.m., Dan Smith wrote:
> > I don't think we want to use the first field as the default field long 
> > term. That's behavior that might be confusing to users, especially since 
> > the order of the fields is not really defined if you pass in a map of 
> > fields->analyzers.

Yeah, I agree, it was more of a place holder until we figure out what our 
default field will be.  I'll be working on adding api's to pass in the default 
field next and then we can figure out what the true default field should be...


- Jason


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


On June 13, 2016, 5:59 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48658/
> ---
> 
> (Updated June 13, 2016, 5:59 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Removed tests specific to multi field query parsing
> Currently using the first field as the default field.  Future changes will 
> allow user to pass in a default field.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
>  62cb65c 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
>  cfd8c32 
> 
> Diff: https://reviews.apache.org/r/48658/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 48658: Change from MultiFieldQueryParser to StandardQueryParser

[Jason Huynh]

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

Review request for geode, anilkumar gingade, nabarun nag, Dan Smith, and 
xiaojian zhou.


Repository: geode


Description
---

Removed tests specific to multi field query parsing
Currently using the first field as the default field.  Future changes will 
allow user to pass in a default field.


Diffs
-

  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProvider.java
 62cb65c 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/StringQueryProviderJUnitTest.java
 cfd8c32 

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


Testing
---


Thanks,

Jason Huynh

Review Request 48556: Fixing spark connector build issue due to ConfigurationProperties name change

[Jason Huynh]

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

Review request for geode, Udo Kohlmeyer and Dan Smith.


Repository: geode


Description
---

Modified JavaApiIntegrationTest as it was pointing to 
DistributedSystemConfigProperties to the renamed ConfigurationProperties


Diffs
-

  
geode-spark-connector/geode-spark-connector/src/it/java/ittest/io/pivotal/geode/spark/connector/JavaApiIntegrationTest.java
 1bae89b 

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


Testing
---


Thanks,

Jason Huynh

Re: Proposal to allow eviction and expiration operations/events with AsyncEventQueue.

[Jason Huynh]

*I will wait for other feedbacks/comments, if there is no objection i
willchange it to "forwardExpirationDestroy" with default value set to true.*

think you meant to say default value to set false :-)

On Fri, Jun 3, 2016 at 11:47 AM, Anilkumar Gingade 
wrote:

> Thanks Darrel...
>
> The name change was based on the review comments:
> https://reviews.apache.org/r/46243/
>
> I am fine with either one, ignoreXXX or forwardXXX.
>
> I will wait for other feedbacks/comments, if there is no objection i will
> change it to "forwardExpirationDestroy" with default value set to true.
>
> Thanks,
> -Anil.
>
>
>
>
>
>
>
>
> On Thu, Jun 2, 2016 at 2:14 PM, Darrel Schneider 
> wrote:
>
> > When did forwardXXX become ignoreXXX? I read through the email thread and
> > couldn't find why that happened. It is best for the default on a boolean
> > property to be false. That was the case when it was forwardXXX. But now
> > that it has changed to ignoreXXX the default has become true. I'd vote
> for
> > it being named something whose default can be false.
> >
> >
> > On Thu, Jun 2, 2016 at 11:10 AM, Anilkumar Gingade 
> > wrote:
> >
> > > Hi Team,
> > >
> > > As proposed here, we added support to propagate eviction and expiration
> > > (destroy) operation to AsyncEventQueue using single flag/attribute
> > > "ignoreEvictionAndExpiration" by default which is true (to keep the
> same
> > > behavior) and one could set (false) to receive eviction/expiration
> > event...
> > >
> > > But we come across a product issue, GEODE-1472, that cause data
> > > inconsistency (with eviction destroy)For this reason we are
> planning
> > to
> > > break the "ignoreEvictionAndExpiration" attribute to eviction and
> > > expiration specific:
> > > "ignoreEvictionDestroy", "ignoreExpirationDestroy"...
> > >
> > > Currently we are planning to support "ignoreExpirationDestroy",  and
> add
> > > "ignoreEvictionDestroy" once GEODE-1472 is fixed...
> > >
> > > Looking for comments on this...
> > >
> > > Thanks,
> > > -Anil.
> > >
> > > https://issues.apache.org/jira/browse/GEODE-1472
> > >
> > >
> > >
> > >
> > > On Tue, Apr 12, 2016 at 6:04 PM, Anilkumar Gingade <
> aging...@pivotal.io>
> > > wrote:
> > >
> > > > Kirk, We could not think of any such requirement...And with this
> > > > application will get all the update operation and can take
> appropriate
> > > > action (use or ignore)...
> > > >
> > > > -Anil.
> > > >
> > > >
> > > >
> > > > On Tue, Apr 12, 2016 at 5:46 PM, Kirk Lund  wrote:
> > > >
> > > >> Would any user ever have a reason to enable forwarding of one type
> but
> > > not
> > > >> the other? If so then I would separate them as
> forwardEvictionEvents()
> > > and
> > > >> forwardExpirationEvents().
> > > >>
> > > >>
> > > >> On Tue, Apr 12, 2016 at 5:44 PM, Kirk Lund 
> wrote:
> > > >>
> > > >> > +1 for being more explicit with the "And" conjunction
> > > >> >
> > > >> > -Kirk
> > > >> >
> > > >> >
> > > >> > On Tue, Apr 12, 2016 at 5:25 PM, Anthony Baker  >
> > > >> wrote:
> > > >> >
> > > >> >> I’d prefer to insert a conjunction to clarify the meaning:
> > > >> >>
> > > >> >> forwardEvictionAndExpirationEvents()
> > > >> >>
> > > >> >>
> > > >> >> $0.02,
> > > >> >> Anthony
> > > >> >>
> > > >> >> On Apr 12, 2016, at 5:11 PM, Anilkumar Gingade <
> > aging...@pivotal.io>
> > > >> >> wrote:
> > > >> >>
> > > >> >> *New attribute:* "forwardEvictionExpirationEvents()" (Any
> alternate
> > > >> >> names?).
> > > >> >>
> > > >> >>
> > > >> >>
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>

Review Request 48176: Changed cache xml property for spark tests to use new DistributionConfig property

[Jason Huynh]

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

Review request for geode, William Markito, Udo Kohlmeyer, and Dan Smith.


Repository: geode


Description
---

The spark build is currently broken due to an invalid property


Diffs
-

  
geode-spark-connector/geode-spark-connector/src/it/java/ittest/io/pivotal/geode/spark/connector/JavaApiIntegrationTest.java
 1c6a5a2 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 48145: GEODE-11: Adding a method to LuceneIndexImpl to dump indexes

[Jason Huynh]

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


Ship it!




Do the tests clean up the exported files?

- Jason Huynh


On June 1, 2016, 6:34 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48145/
> ---
> 
> (Updated June 1, 2016, 6:34 p.m.)
> 
> 
> Review request for geode and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Adding a method to LuceneIndexImpl to invoke the DumpDirectoryFiles
> function and dump all of the index files to disk.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/InternalLuceneIndex.java
>  951b0f9eb1fcef187dd57e3379a4f1065e12cef4 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
>  8f2cb10a0dc2d930362473f575042ec25f04c7f2 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForReplicatedRegion.java
>  c24cb9d8a8da1b6bb9e9f34ad2c8255f2057cdae 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java
>  0d7b859c51bc2b701b866f87d9ea496a013e1762 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java
>  4ae1efec926b495c99097a467ebcfdd3a2de802e 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java
>  9f02f2b780067e742635e416286969145c8c4862 
> 
> Diff: https://reviews.apache.org/r/48145/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Review Request 48151: Prevent lucene index from being created on region with eviction action local destroy

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Throws and UnsupportedOperationException when a lucene index is being created 
on a region with eviction action local-destroy.


Diffs
-

  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
 f9bb8ba 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationIntegrationTest.java
 d1cd8ac 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 48086: GEODE-11: Adding a tool to dump the lucene index files

[Jason Huynh]

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



Not sure how important it will be but maybe a test for an invalid function 
argument (such as a bad location or a null location?)


geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java
 (line 74)
<https://reviews.apache.org/r/48086/#comment200702>

Maybe check to see if index is null first?


- Jason Huynh


On May 31, 2016, 7:16 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48086/
> ---
> 
> (Updated May 31, 2016, 7:16 p.m.)
> 
> 
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Adding a function that will dump all of the files for the lucene index
> to disk, for examination with external tools like Luke.
> 
> 
> Diffs
> -
> 
>   
> geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java
>  184619fe2332038b80d4769e88968023f6c55d63 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
>  1f011a6b8650f98fd42fef71bd1593080fe91379 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java
>  2937af54631ed593ca9a5213dbcc6c964895731d 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java
>  fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java
>  f1e63e096208724b935649f102f37a0be3245822 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java
>  3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java
>  1d936ca216bf06550e6ce552bdefa999a3c25a31 
> 
> Diff: https://reviews.apache.org/r/48086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 47912: GEODE-11: Adding stats for lucene index updates and queries

[Jason Huynh]

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


Fix it, then Ship it!





geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java
 (line 116)
<https://reviews.apache.org/r/47912/#comment200031>

Not really an issue, but rather a test maintenance question; are we 
planning on adding to this test when a new stat is added or should we break 
this up into maybe the crud stats, and query?  If we break it up per stat, I 
can see why that would be a bit tedious...


- Jason Huynh


On May 26, 2016, 6:26 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47912/
> ---
> 
> (Updated May 26, 2016, 6:26 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, 
> nabarun nag, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Adding some initial stats for lucene indexes tracking updates and queries.
> 
> I haven't yet figured out how to update the documents stat, but I figured I 
> would get the easy stats checked in first. We're tracking updates, commits, 
> and query stats with this change.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
>  d22ca4a196df3b1a457b56c92da694bdbf792cc2 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImpl.java
>  c165085c83b38930bb970ac8f8e3f3fd8aa8808f 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStats.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/PartitionedRepositoryManager.java
>  57b8862d91cb0b825de27d67ff594c984fc8ca55 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java
>  065cc6a5c2b140a2fb383362d9e8d1316e903e8a 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexRecoveryHAIntegrationTest.java
>  194f3c7adfa32d0f2c8e9f18a27a3217d488560c 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStatsJUnitTest.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/PartitionedRepositoryManagerJUnitTest.java
>  4532d1651845c429bfa2b9f2f4eebf9e6e2bbdb2 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/DistributedScoringJUnitTest.java
>  7bcc7619198c360d0ec3a5d887bb0b2b4a483d8d 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java
>  53c41616042dfe71febe7bc192c74f26f3938203 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java
>  cec76ba0533093c88d653879323acba4e1901c9e 
> 
> Diff: https://reviews.apache.org/r/47912/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 47803: field value could be null, need to handle this case

[Jason Huynh]

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




geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionLuceneSerializer.java
 (line 73)
<https://reviews.apache.org/r/47803/#comment199714>

If we wanted to do something similar we could change SerializerUtil to 
detect a null value and stuff a token in, but do we want to do that?


- Jason Huynh


On May 25, 2016, 12:20 a.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47803/
> ---
> 
> (Updated May 25, 2016, 12:20 a.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: GEODE-1453
> https://issues.apache.org/jira/browse/GEODE-1453
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The field value could be null, when creating index, we need to handle it.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PdxLuceneSerializer.java
>  3990614 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionLuceneSerializer.java
>  a76478c 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationTest.java
>  4ebb9c4 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/PdxFieldMapperJUnitTest.java
>  278e818 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/ReflectionFieldMapperJUnitTest.java
>  c187022 
> 
> Diff: https://reviews.apache.org/r/47803/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that b

[Jason Huynh]

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


Ship it!




- Jason Huynh


On May 19, 2016, 9:47 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> ---
> 
> (Updated May 19, 2016, 9:47 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, 
> Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is 
> stopped. The test stops the CQ and verifies no events are recieved in stopped 
> state; it was doing that by checking operations/events that happend before 
> stop and not counting the changes happened during stop. In a slower 
> environment, it could so happen that the CQ may not have recieved all the 
> events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -
> 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java
>  130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> ---
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the 
> test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that b

[Jason Huynh]

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



Long term this test could probably be broken up into smaller chunks as it's 
testing multiple scenarios


geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java
 (line 432)
<https://reviews.apache.org/r/47615/#comment198694>

I think having the waitForUpdate (which happens a few lines below) will be 
enough for cq_1?  He won't get the create and update out of order correct?



geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java
 (line 433)
<https://reviews.apache.org/r/47615/#comment198693>

I don't think we need this one here (for cq_3), we have a wait for updated 
later and do not do any validation until after the update.


- Jason Huynh


On May 19, 2016, 9:47 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> ---
> 
> (Updated May 19, 2016, 9:47 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, 
> Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is 
> stopped. The test stops the CQ and verifies no events are recieved in stopped 
> state; it was doing that by checking operations/events that happend before 
> stop and not counting the changes happened during stop. In a slower 
> environment, it could so happen that the CQ may not have recieved all the 
> events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -
> 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java
>  130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> ---
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the 
> test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 47496: Adding transaction tests for lucene indexes

[Jason Huynh]

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

(Updated May 19, 2016, 6:32 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Changes
---

Updated based on review comments
Migrated a few tests from LuceneServiceImplIntegrationTest to better locations


Repository: geode


Description
---

Added integration tests to test three scenarios:
1.) the lucene indexes are not updated during an in flight transaction (begin 
but before rollback or commit)
2.) the lucene indexes are updated after a commit
3.) the lucene indexes are not changed after a rollback

todo:
Do we want to add a dunit test for multiple vms and threads doing transactions? 
 I avoided adding this test for now as I thought that was more of a "testing 
the geode transaction feature" rather than testing how the feature affects 
lucene.


Diffs (updated)
-

  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationIntegrationTest.java
 4c28938 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java
 23983cb 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java
 PRE-CREATION 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIntegrationTest.java
 c302460 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplIntegrationTest.java
 fa3392c 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 47496: Adding transaction tests for lucene indexes

[Jason Huynh]

> On May 18, 2016, 1 a.m., Dan Smith wrote:
> > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplIntegrationTest.java,
> >  line 352
> > <https://reviews.apache.org/r/47496/diff/1/?file=1386143#file1386143line352>
> >
> > I'm actually surprised this works, since the field names are "title" 
> > and "description", not "text".

Looked into this a bit, apparently I need to use description:"\hello world\" as 
the string, otherwise lucene will look for the word "world" in the default 
field which was the reason why it was working the way it was.  In our case the 
default I think are all fields or whatever the QueryParser is passed


- Jason


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


On May 17, 2016, 10:54 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47496/
> ---
> 
> (Updated May 17, 2016, 10:54 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added integration tests to test three scenarios:
> 1.) the lucene indexes are not updated during an in flight transaction (begin 
> but before rollback or commit)
> 2.) the lucene indexes are updated after a commit
> 3.) the lucene indexes are not changed after a rollback
> 
> todo:
> Do we want to add a dunit test for multiple vms and threads doing 
> transactions?  I avoided adding this test for now as I thought that was more 
> of a "testing the geode transaction feature" rather than testing how the 
> feature affects lucene.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplIntegrationTest.java
>  fa3392c 
> 
> Diff: https://reviews.apache.org/r/47496/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: Review Request 47496: Adding transaction tests for lucene indexes

[Jason Huynh]

> On May 18, 2016, 12:16 a.m., anilkumar gingade wrote:
> > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplIntegrationTest.java,
> >  line 321
> > <https://reviews.apache.org/r/47496/diff/1/?file=1386143#file1386143line321>
> >
> > How about moving these puts into region initialization method?

I left it this way for now because I think there will be cases where we want to 
create a region with different data and not necessarily have the same data 
every time.  Each test can specifically tune the data they want to use and 
expect.


- Jason


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


On May 17, 2016, 10:54 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47496/
> ---
> 
> (Updated May 17, 2016, 10:54 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added integration tests to test three scenarios:
> 1.) the lucene indexes are not updated during an in flight transaction (begin 
> but before rollback or commit)
> 2.) the lucene indexes are updated after a commit
> 3.) the lucene indexes are not changed after a rollback
> 
> todo:
> Do we want to add a dunit test for multiple vms and threads doing 
> transactions?  I avoided adding this test for now as I thought that was more 
> of a "testing the geode transaction feature" rather than testing how the 
> feature affects lucene.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplIntegrationTest.java
>  fa3392c 
> 
> Diff: https://reviews.apache.org/r/47496/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 47496: Adding transaction tests for lucene indexes

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Added integration tests to test three scenarios:
1.) the lucene indexes are not updated during an in flight transaction (begin 
but before rollback or commit)
2.) the lucene indexes are updated after a commit
3.) the lucene indexes are not changed after a rollback

todo:
Do we want to add a dunit test for multiple vms and threads doing transactions? 
 I avoided adding this test for now as I thought that was more of a "testing 
the geode transaction feature" rather than testing how the feature affects 
lucene.


Diffs
-

  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplIntegrationTest.java
 fa3392c 

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


Testing
---


Thanks,

Jason Huynh

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

[Jason Huynh]

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



We have some other tests that had work arounds for this issue.  Perhaps we can 
fix these to use the new flush method?  For example, I think 
LuceneIndexCreationIntegrationTest we added Awaitility.await().atMost(60, 
TimeUnit.SECONDS).until(() -> {
  assertEquals(Arrays.asList("field1"), field1Analyzer.analyzedfields);
  assertEquals(Arrays.asList("field2"), field2Analyzer.analyzedfields);
});

Or in LuceneIndexRecoveryHAIntegrationTest

 private void waitUntilQueueEmpty(final String aeqId) {
// TODO flush queue
AsyncEventQueue queue = cache.getAsyncEventQueue(aeqId);
Awaitility.waitAtMost(1000, TimeUnit.MILLISECONDS).until(() -> 
assertEquals(0, queue.size()));
  }


geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImpl.java
 (line 103)
<https://reviews.apache.org/r/47004/#comment197790>

Should we throw an exception if we never get to 0?


- Jason Huynh


On May 16, 2016, 5:39 a.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47004/
> ---
> 
> (Updated May 16, 2016, 5:39 a.m.)
> 
> 
> Review request for geode, anilkumar gingade 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/LuceneEventListener.java
>  9fdfd43 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImpl.java
>  0b5f8fa 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  c467a18 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImplJUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47004/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Re: Review Request 46918: GEODE-11: Refactoring the LuceneFunctionReadPathDUnitTest

[Jason Huynh]

> On May 3, 2016, 9:20 p.m., Jason Huynh wrote:
> > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java,
> >  line 60
> > <https://reviews.apache.org/r/46918/diff/2/?file=1369368#file1369368line60>
> >
> > Should we pull the "createIndex" into it's own method call and not have 
> > it be part of initDataStore?
> > 
> > The only reason why we might want to do that would be when we finally 
> > support creating indexes after the region has been created?
> 
> Dan Smith wrote:
> The reason why I made the createIndex code a callback is that I wanted 
> the test class to be able to decide when and if it should run it. I guess 
> maybe that made more sense for the accessor VM, because an accessor could be 
> a datastore or it could be a client depending on the topology we're testing. 
> So maybe I should take it out of the initDataStore but leave it in 
> initAccessor? I think maybe I'll leave them the same for the moment but we 
> can refactor if we need to add different tests for different orders?

Oh I just saw that the initDataStore for some of the classes would call 
createIndex.run() before creating the region but perhaps later on we would want 
tests in the same classes to created the index after the region was created.  
As you said, we can always refactor it later.


- Jason


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


On May 3, 2016, 12:12 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46918/
> -------
> 
> (Updated May 3, 2016, 12:12 a.m.)
> 
> 
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Refactoring this test into a framework for adding more tests with a
> bunch of subclasses.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPR.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflow.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java
>  f6fcf8a36ea73218918c93192a98c60eae83ac0b 
> 
> Diff: https://reviews.apache.org/r/46918/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 46918: GEODE-11: Refactoring the LuceneFunctionReadPathDUnitTest

[Jason Huynh]

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


Fix it, then Ship it!





geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
 (line 60)
<https://reviews.apache.org/r/46918/#comment195582>

Should we pull the "createIndex" into it's own method call and not have it 
be part of initDataStore?

The only reason why we might want to do that would be when we finally 
support creating indexes after the region has been created?


- Jason Huynh


On May 3, 2016, 12:12 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46918/
> ---
> 
> (Updated May 3, 2016, 12:12 a.m.)
> 
> 
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Refactoring this test into a framework for adding more tests with a
> bunch of subclasses.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPR.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflow.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java
>  f6fcf8a36ea73218918c93192a98c60eae83ac0b 
> 
> Diff: https://reviews.apache.org/r/46918/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 46917: GEODE-625: Adding tests for exception handling of functions

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On May 3, 2016, 1:03 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46917/
> ---
> 
> (Updated May 3, 2016, 1:03 a.m.)
> 
> 
> Review request for geode, anilkumar gingade and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Testing how functions propagate exceptions from within the function to
> the caller or the result collector.
> 
> Fixing a bug in exception propegation discovered while writing these tests.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegionDataStore.java
>  57b1e71e8a5591e74f11041c916ed5cec1a8fd29 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceBase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceLocalPRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceLocalRRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceMultipleOnMemberDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServicePeerAccessorPRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServicePeerAccessorRRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceSingleOnMemberDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46917/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 46918: GEODE-11: Refactoring the LuceneFunctionReadPathDUnitTest

[Jason Huynh]

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




geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
 (line 101)
<https://reviews.apache.org/r/46918/#comment195396>

For future reference, what happens if the region is a replicated region, 
does rebalance just get ignored?



geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflow.java
 (line 26)
<https://reviews.apache.org/r/46918/#comment195397>

Remove?


- Jason Huynh


On May 2, 2016, 11:43 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46918/
> ---
> 
> (Updated May 2, 2016, 11:43 p.m.)
> 
> 
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Refactoring this test into a framework for adding more tests with a
> bunch of subclasses.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPR.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflow.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java
>  f6fcf8a36ea73218918c93192a98c60eae83ac0b 
> 
> Diff: https://reviews.apache.org/r/46918/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 46917: GEODE-625: Adding tests for exception handling of functions

[Jason Huynh]

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




geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceBase.java
 (line 65)
<https://reviews.apache.org/r/46917/#comment195384>

Do we care about the message in the functionexception?

If so, instead of using the expected = FunctionException.class annotation, 
we can do:

//class variable
public ExpectedException expectedException = ExpectedException.none();

//in each test
expectedException.expect(SomeException.class);
expectedException.expectMessage("Some message");



geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceRemoteRRDUnitTest.java
 (line 18)
<https://reviews.apache.org/r/46917/#comment195388>

Should this be proxy RR instead of remote RR?

Is this a client or a peer proxy?


- Jason Huynh


On May 2, 2016, 10:57 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46917/
> ---
> 
> (Updated May 2, 2016, 10:57 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Testing how functions propagate exceptions from within the function to
> the caller or the result collector.
> 
> Fixing a bug in exception propegation discovered while writing these tests.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegionDataStore.java
>  57b1e71e8a5591e74f11041c916ed5cec1a8fd29 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceBase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceLocalPRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceLocalRRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceMultipleOnMemberDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceRemotePRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceRemoteRRDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceSingleOnMemberDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46917/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Upgrading Gradle to version 2.12

[Jason Huynh]

This work has been completed and checked into develop :
8e744982a1cc5da50d4eb5640a2dae8ed87dfd24

Thanks,
-Jason

On Thu, Apr 28, 2016 at 12:06 PM, Mark Bretl <asf.mbr...@gmail.com> wrote:

> +1 Thanks Jason!
>
> --Mark
>
> On Wed, Apr 27, 2016 at 5:22 PM, William Markito <wmark...@pivotal.io>
> wrote:
>
> > +1
> >
> > On Wed, Apr 27, 2016 at 10:41 AM, Jason Huynh <jhu...@pivotal.io> wrote:
> >
> > > Hello Geode dev list,
> > >
> > > As a heads up, I am about to check in a change to upgrade the Gradle
> > > version to 2.12.  This will address GEODE-1259, GEODE-1085, GEODE-1261.
> > >
> > > This change will now make the required minimum Gradle Version the same
> as
> > > the version used by the Gradle Wrapper Plugin.  This may affect you if
> > you
> > > have not been using the gradlew scripts to build Geode.
> > >
> > > The review request is at https://reviews.apache.org/r/46706/diff/1/
> > >
> > > Thanks,
> > > -Jason
> > >
> >
> >
> >
> > --
> >
> > ~/William
> >
>

Upgrading Gradle to version 2.12

[Jason Huynh]

Hello Geode dev list,

As a heads up, I am about to check in a change to upgrade the Gradle
version to 2.12.  This will address GEODE-1259, GEODE-1085, GEODE-1261.

This change will now make the required minimum Gradle Version the same as
the version used by the Gradle Wrapper Plugin.  This may affect you if you
have not been using the gradlew scripts to build Geode.

The review request is at https://reviews.apache.org/r/46706/diff/1/

Thanks,
-Jason

Re: Review Request 46706: GEODE-1259: Upgrade gradle version to 2.12

[Jason Huynh]

> On April 27, 2016, 1:46 p.m., Anthony Baker wrote:
> > settings.gradle, line 38
> > <https://reviews.apache.org/r/46706/diff/1/?file=1362457#file1362457line38>
> >
> > If you define this as 'ext.minimumGradleVersion' can we avoid setting 
> > it both here and in gradle.properties?

I moved the minimumGradleVersion to gradle.properties and removed the one in 
settings.gradle.  Was there a reason to keep it in settings.gradle instead?


On April 27, 2016, 1:46 p.m., Jason Huynh wrote:
> > After running `gradle wrapper` with the gradleVersion set, the 
> > `gradle/wrapper/gradle-wrapper.jar` is different, probably because it was 
> > generated with different gradle version.  Should that be updated for 
> > consistency?
> > 
> > Since this is a build change that will affect everyone, can you send a 
> > heads up to the dev list?  Thanks!

Added the gradle-wrapper.jar.  I'll send an email before checking in


- Jason


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


On April 26, 2016, 11:12 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46706/
> ---
> 
> (Updated April 26, 2016, 11:12 p.m.)
> 
> 
> Review request for geode, Anthony Baker, anilkumar gingade, Barry Oglesby, 
> Mark Bretl, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Addresses GEODE-1085
> Addresses GEODE-1261
> 
> 
> Diffs
> -
> 
>   BUILDING.md ababe2b 
>   build.gradle 4f79eae 
>   gradle.properties 669baed 
>   gradle/wrapper/gradle-wrapper.properties 7b0d17a 
>   settings.gradle c579dce 
> 
> Diff: https://reviews.apache.org/r/46706/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 46706: GEODE-1259: Upgrade gradle version to 2.12

[Jason Huynh]

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

Review request for geode, Anthony Baker, anilkumar gingade, Barry Oglesby, Mark 
Bretl, nabarun nag, Dan Smith, and xiaojian zhou.


Repository: geode


Description
---

Addresses GEODE-1085
Addresses GEODE-1261


Diffs
-

  BUILDING.md ababe2b 
  build.gradle 4f79eae 
  gradle.properties 669baed 
  gradle/wrapper/gradle-wrapper.properties 7b0d17a 
  settings.gradle c579dce 

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


Testing
---


Thanks,

Jason Huynh

Review Request 46239: UnitTest for LuceneIndexForPartitionedRegion

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Not sure if this is needed at this point, but with this new unit test it gets 
slightly more coverage in a fraction of the time (takes 2 seconds on my laptop 
for 18 tests) it takes to run LuceneServiceImplJUnitTest (9 seconds for 5 
tests).  None of the cache calls are on a real cache so that gives some speed 
savings, although as you can see, the difference is only in seconds at this 
point.

Allows ability to test that double initialize calls do not invoke multiple file 
region creates (which may have been hard to test before or is this case even 
possible?)

refactored LuceneIndexForPartitionRegion so that a unit test for it could test 
specific units, but also allowing tests for initialize() to be stubbed out.


Diffs
-

  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
 25b63a4 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java
 PRE-CREATION 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 46156: GEODE-679 Explore removing SocketIOWithTimeout and other classes related to FD soft leak

[Jason Huynh]

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



I think we can also delete SocketIOWithTimeout, SocketInputStream and 
SocketOutputStream along with these changes

- Jason Huynh


On April 13, 2016, 5:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46156/
> ---
> 
> (Updated April 13, 2016, 5:48 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jason Huynh, Jianxia Chen, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-679
> https://issues.apache.org/jira/browse/GEODE-679
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> SocketUtils was added to avoid a file descriptor "leak" caused by the use of 
> NIO socket channel selectors.  This was spurred by a HADOOP JIRA ticket that 
> claimed that sun.misc.Cleaners were being used to close selectors (see 
> https://issues.apache.org/jira/browse/HADOOP-4346).  I have verified that 
> Cleaners are no longer used to close selectors and that SocketUtils is not 
> making any difference in the number of file descriptors created in servers 
> using NIO selectors using the (recently deleted) FDDUnitTest with 
> modifications to force clients to close their connections to the server.
> 
> So, this change-set removes SocketUtils, reverting all made to introduce it 
> in GemFire 8.0 to use direct method invokations on sockets.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java
>  5016d67a7162268e2c793152f702d1f4d60f71d3 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 
> 2130a261a395b19a3b3aa197d9f97eab3cab2cc0 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java
>  5b20e8634328df490ac845c57fd35837890d5ae3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java
>  3178b8d1de4d649597d429e651a3b06877347e52 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientUpdater.java
>  3e43b69398725d32cd393c5da90c0f6969282ae3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java
>  2ea6ca06c38de54eb9ac1184995c7908424eaa55 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java
>  bfe382c0c5aac7e6286a4f7841f9b50724fe59db 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerConnection.java
>  1dd2562dbcda007e90908c7e21714b26f5b9bbe7 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ServerHandShakeProcessor.java
>  07185b8cfc6c985b856f497a79ccb37af239a23a 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java 
> bf44cd379f68d07edac78fe44b045a1e88f5c5f5 
> 
> Diff: https://reviews.apache.org/r/46156/diff/
> 
> 
> Testing
> ---
> 
> precheckin is underway
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 46065: clean up the lucene index support code for RR

[Jason Huynh]

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




geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java
 (line 220)
<https://reviews.apache.org/r/46065/#comment191870>

Instead of a try-catch block, unless we are checking the message, we can 
instead use a @Test (expected=UnsupportedOperationException.class) as the tag 
instead.


- Jason Huynh


On April 12, 2016, 6:30 a.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46065/
> ---
> 
> (Updated April 12, 2016, 6:30 a.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Bugs: geode-1212
> https://issues.apache.org/jira/browse/geode-1212
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The objective is:
> 1) If tried to create LuceneIndex on a replicated region, should throw 
> UnsupportedOperationException
> 2) No API to mislead user to create index on replicated region
> 3) add test case to verify
> 
> I did not remove the class LuceneIndexForReplicatedRegion, since it has 
> implemented to throw UnsupportedOperationException for all its methods. This 
> is also an option.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
>  7002567 
>   
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
>  1a5dd1a 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryImplJUnitTest.java
>  3975ac3 
>   
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java
>  159fd46 
> 
> Diff: https://reviews.apache.org/r/46065/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Review Request 45947: Update lucene version to 6.0.0

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Updated version to 6.0.0
Made modifications to use DoublePoint instead of LegacyDoubleField, IntPoint 
instead of LegacyIntField, etc
Added a temporary file to FileSystem.  This can be discussed as to how we want 
to handle this method.  Currently it creates a temporary, non distributed file.
SearcherManager creation now will pass in true for force deletes... This can 
also be discussed on what we want to pass in

Sorting the fileList due to new Lucene based test that requires the list 
returned to be sorted


Diffs
-

  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/RegionDirectory.java
 e25dc77 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java
 b84dc92 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java
 a9c463e 
  
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/serializer/SerializerUtil.java
 7ffc5db 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java
 74f3742 
  gradle/dependency-versions.properties 7a5054b 

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


Testing
---

geode-lucene:precheckin
running precheckin


Thanks,

Jason Huynh

Re: Review Request 45938: GEODE-1062: Undoing reduction in wait time

[Jason Huynh]

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

(Updated April 8, 2016, 8:26 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Changes
---

Updated diff to add a new method signature.  Any tests that are failing due to 
the defaulted 30 seconds can now be changed on a per test basis to wait for 
however long the test thinks it needs.


Repository: geode


Description
---

Some tests with overflow require the full 24 millisecond timeout...


Diffs (updated)
-

  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 130f960 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 45938: GEODE-1062: Undoing reduction in wait time

[Jason Huynh]

> On April 8, 2016, 8:02 p.m., Barry Oglesby wrote:
> > Maybe that timeout should be configurable? Most times 4 minutes might as 
> > well be forever.
> 
> Jason Huynh wrote:
> Yes, that would be a good idea.  I'm trying to revert this one change in 
> the big refactoring I did earlier for GEODE-1062.  I didn't think anything 
> would need to wait 4 minutes but Naba found a test that fails for him on his 
> machine.  Most of the properly passing tests will actually complete before 
> the 4 minutes due to this being a wait criterion.
> 
> Barry Oglesby wrote:
> Right, but if it isn't a "properly passing test," then you're stuck 
> waiting for 4 minutes for it to fail. I'm sure all our tests will pass from 
> now on, so we don't have to worry about this wait time.

Or maybe I can leave this change at 30 seconds and any test that fails from 
this can be rewritten to not rely on the full 4 minutes?


- Jason


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


On April 8, 2016, 6:37 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45938/
> ---
> 
> (Updated April 8, 2016, 6:37 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Some tests with overflow require the full 24 millisecond timeout...
> 
> 
> Diffs
> -
> 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
>  130f960 
> 
> Diff: https://reviews.apache.org/r/45938/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: Review Request 45938: GEODE-1062: Undoing reduction in wait time

[Jason Huynh]

> On April 8, 2016, 8:02 p.m., Barry Oglesby wrote:
> > Maybe that timeout should be configurable? Most times 4 minutes might as 
> > well be forever.

Yes, that would be a good idea.  I'm trying to revert this one change in the 
big refactoring I did earlier for GEODE-1062.  I didn't think anything would 
need to wait 4 minutes but Naba found a test that fails for him on his machine. 
 Most of the properly passing tests will actually complete before the 4 minutes 
due to this being a wait criterion.


- Jason


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


On April 8, 2016, 6:37 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45938/
> ---
> 
> (Updated April 8, 2016, 6:37 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Some tests with overflow require the full 24 millisecond timeout...
> 
> 
> Diffs
> -
> 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
>  130f960 
> 
> Diff: https://reviews.apache.org/r/45938/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 45938: GEODE-1062: Undoing reduction in wait time

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Some tests with overflow require the full 24 millisecond timeout...


Diffs
-

  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 130f960 

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


Testing
---


Thanks,

Jason Huynh

Review Request 45897: Update gemfire-spark project to geode-spark project and also renamed packages and files

[Jason Huynh]

/StructStreamingResultSenderAndCollectorTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/io/pivotal/geode/spark/connector/internal/oql/QueryParserTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/ConnectorImplicitsTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/GeodeConnectionConfTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/GeodeDStreamFunctionsTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/GeodeRDDFunctionsTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/LocatorHelperTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/rdd/GeodeRDDPartitionerTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-connector/src/test/scala/unittest/io/pivotal/geode/spark/connector/rdd/GeodeRegionRDDTest.scala
 PRE-CREATION 
  
geode-spark-connector/geode-spark-demos/basic-demos/src/main/java/demo/Emp.java 
PRE-CREATION 
  
geode-spark-connector/geode-spark-demos/basic-demos/src/main/java/demo/OQLJavaDemo.java
 PRE-CREATION 
  
geode-spark-connector/geode-spark-demos/basic-demos/src/main/java/demo/PairRDDSaveJavaDemo.java
 PRE-CREATION 
  
geode-spark-connector/geode-spark-demos/basic-demos/src/main/java/demo/RDDSaveJavaDemo.java
 PRE-CREATION 
  
geode-spark-connector/geode-spark-demos/basic-demos/src/main/java/demo/RegionToRDDJavaDemo.java
 PRE-CREATION 
  
geode-spark-connector/geode-spark-demos/basic-demos/src/main/scala/demo/NetworkWordCount.scala
 PRE-CREATION 
  geode-spark-connector/project/Dependencies.scala c77c158 
  geode-spark-connector/project/GemFireSparkBuild.scala 89d8e0b 
  geode-spark-connector/project/GeodeSparkBuild.scala PRE-CREATION 
  geode-spark-connector/project/Settings.scala 796541c 

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


Testing
---

sbt clean compile package test it:test


Thanks,

Jason Huynh

Review Request 45880: Added Lucene Index creation on RR expects exception test. Minor refactoring

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Added a test that expects unsupported exception when creating a lucene index on 
replicated region

Instead of a sleep/wait, instead will use awaitility to wait until async queue 
size is 0.  The todo to flush the queue instead is still there and should be 
easy to replace the awaitility call

Removed the createBasicCache code and merged it with getCache() (This will 
hopefully remove any chance to accidently createBasicCache and then call 
getCache() and end up with two caches).  

getCache() and getService() will now return the cache and luceneservice.

renamed a test to be slightly more descriptive.  createRegionFirst() to 
cannotCreateLuceneIndexAfterRegionHasBeenCreated()

removed dead code on else block where a root region was being created even 
though it was not supposed to be and it was also not being used.


Diffs
-

  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexRecoveryHAJUnitTest.java
 405c986 
  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImplJUnitTest.java
 159fd46 

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


Testing
---


Thanks,

Jason Huynh

Review Request 45820: Minor clarity changes for Lucene dunit test

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

Removed a system out that looks out of place
Renamed the test methods to hopefully fit better with the actual method

The test itself looks to only work for PartitionedRegions (the rebalance 
operation would fail for a replicated region?) and the destroy Region only 
works for a Partitioned Region at this point, but I didn't rename the 
e2eTextSearchForRegionType for now.


Diffs
-

  
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionReadPathDUnitTest.java
 901b363 

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


Testing
---


Thanks,

Jason Huynh

Review Request 45711: GEODE-1044: Not initializing query array for every test, code clean up

[Jason Huynh]

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

Review request for geode, anilkumar gingade and nabarun nag.


Repository: geode


Description
---

The query array is not needed for most tests that use QueryTestUtils.  Instead 
of tests that need it will need to invoke the method to initialize the array 
explicitly.

Removed code that was not being called by any other class


Diffs
-

  geode-core/src/test/java/com/gemstone/gemfire/cache/query/QueryTestUtils.java 
d53a85b 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/QueryTestUtilsJUnitTest.java
 15cae6a 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexDUnitTest.java
 86f629d 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/HashIndexDUnitTest.java
 4db6fc0 
  
geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/CompactRangeIndexJUnitTest.java
 a24f7bf 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 45463: GEODE-1062: CI failure: SerialWANPropogationOffHeapDUnitTest.testReplicatedSerialPropagation_withoutRemoteSite

[Jason Huynh]

/internal/cache/wan/parallel/ParallelWANPropagationLoopBackDUnitTest.java
 b921c88 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelWANStatsDUnitTest.java
 55fd5ad 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderEventListenerDUnitTest.java
 7a05644 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderOperationsDUnitTest.java
 ed6dc29 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderQueueDUnitTest.java
 c4ceb21 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPersistenceEnabledGatewaySenderDUnitTest.java
 4fac57f 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropagationLoopBackDUnitTest.java
 b1fb8d9 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropogationDUnitTest.java
 a272eeb 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropogation_PartitionedRegionDUnitTest.java
 126e737 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropogationsFeatureDUnitTest.java
 5846e75 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
 6602104 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/WANManagementDUnitTest.java
 1f0f3bd 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/pulse/TestRemoteClusterDUnitTest.java
 672cbf7 

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


Testing
---

precheckin


Thanks,

Jason Huynh

Review Request 45463: GEODE-1062: CI failure: SerialWANPropogationOffHeapDUnitTest.testReplicatedSerialPropagation_withoutRemoteSite

[Jason Huynh]

/SerialGatewaySenderOperationsDUnitTest.java
 ed6dc29 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderQueueDUnitTest.java
 c4ceb21 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPersistenceEnabledGatewaySenderDUnitTest.java
 4fac57f 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropagationLoopBackDUnitTest.java
 b1fb8d9 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropogationDUnitTest.java
 a272eeb 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropogation_PartitionedRegionDUnitTest.java
 126e737 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANPropogationsFeatureDUnitTest.java
 5846e75 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
 6602104 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/WANManagementDUnitTest.java
 1f0f3bd 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/pulse/TestRemoteClusterDUnitTest.java
 672cbf7 

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


Testing
---

precheckin


Thanks,

Jason Huynh

Review Request 45180: GEODE-1054: InternalResultSender.setException logging at info level instead of fatal

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Repository: geode


Description
---

When a function exception is thrown due to failing, the results senders are 
currently logging this at a fatal level.  This is not a "catastrophic" error 
and should be reduced from fatal to either error, warn or even info level.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/execute/DistributedRegionFunctionResultSender.java
 18106c1 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/execute/MemberFunctionResultSender.java
 30c652c 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/execute/PartitionedRegionFunctionResultSender.java
 813d833 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 45162: GEODE-911: Cleaning up SerialGatewaySender queues when stopping gateway

[Jason Huynh]

> On March 22, 2016, 6:36 p.m., Dan Smith wrote:
> > geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderOperationsDUnitTest.java,
> >  line 130
> > <https://reviews.apache.org/r/45162/diff/1/?file=1310595#file1310595line130>
> >
> > Remove debugging comments

Whoops, good catch!


- Jason


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


On March 22, 2016, 5 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45162/
> ---
> 
> (Updated March 22, 2016, 5 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Bugs: GEODE-911
> https://issues.apache.org/jira/browse/GEODE-911
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stop() method was setting the EventProcessor to null before cleaning up 
> the queues.  This led to no queues being cleaned up because the method 
> getQueues would return null if the eventProcessor was null.
> 
> This includes refactoring a few methods in WanTestBase that pretty much did 
> the same thing but was duplicated all over.
> 
> The Parallel queues do not have this problem because the EventProcessor is 
> not set to null "for test purposes."  It seems like this should also be set 
> to null as well and the tests that rely on it, should be fixed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderQueue.java
>  430409a 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderImpl.java
>  8f0070f 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
>  5da6b5c 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentSerialGatewaySenderOperationsOffHeapDUnitTest.java
>  e24f593 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderOperationsDUnitTest.java
>  c824eb6 
> 
> Diff: https://reviews.apache.org/r/45162/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: Review Request 45162: GEODE-911: Cleaning up SerialGatewaySender queues when stopping gateway

[Jason Huynh]

> On March 22, 2016, 6:31 p.m., Barry Oglesby wrote:
> > I'm not sure how this fixes the original bug, but it does fix the 
> > BatchRemovalThreads being left running.
> > 
> > Can you also change this code to not call getQueues 3 times:
> > 
> > From:
> > 
> > if (getQueues() != null && !getQueues().isEmpty()) {
> >   for (RegionQueue q : getQueues()) {
> > ((SerialGatewaySenderQueue)q).cleanUp();
> >   }
> > }
> > 
> > To:
> > 
> > Set queues = getQueues();
> > if (queues != null && !queues.isEmpty()) {
> >   for (RegionQueue q : queues) {
> > ((SerialGatewaySenderQueue)q).cleanUp();
> >   }
> > }
> > 
> > Also, SerialGatewaySenderOperationsDUnitTest validateQueueClosed isn't 
> > implemented. Is that intentional?

The original problem was that the thread leaks would eventually cause the 
system to run out of memory and not be able to create a native thread.  This 
caused the sender to fail and the reciever would just wait for events that 
would never be arriving.

I'll make the change to getQueues.

The validate queue closed was never implemented and I tried implementing to 
verify in this case but by the time that code gets called, the call to 
getQueues() would always return null (because eventProcessor is null).  I had 
to do the verification on the stopSender itself...
I tried to leave the eventProcessor around (like In ParallelGatewaySender case) 
but that caused an issue when restarting the sender. It would get into some 
code to remove listeners because the eventProcessor is no longer null... it's 
very fragile and we probably need to figure out what the actual behavior should 
be...


- Jason


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


On March 22, 2016, 5 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45162/
> ---
> 
> (Updated March 22, 2016, 5 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
> Smith, and xiaojian zhou.
> 
> 
> Bugs: GEODE-911
> https://issues.apache.org/jira/browse/GEODE-911
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stop() method was setting the EventProcessor to null before cleaning up 
> the queues.  This led to no queues being cleaned up because the method 
> getQueues would return null if the eventProcessor was null.
> 
> This includes refactoring a few methods in WanTestBase that pretty much did 
> the same thing but was duplicated all over.
> 
> The Parallel queues do not have this problem because the EventProcessor is 
> not set to null "for test purposes."  It seems like this should also be set 
> to null as well and the tests that rely on it, should be fixed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderQueue.java
>  430409a 
>   
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderImpl.java
>  8f0070f 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
>  5da6b5c 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentSerialGatewaySenderOperationsOffHeapDUnitTest.java
>  e24f593 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderOperationsDUnitTest.java
>  c824eb6 
> 
> Diff: https://reviews.apache.org/r/45162/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 45162: GEODE-911: Cleaning up SerialGatewaySender queues when stopping gateway

[Jason Huynh]

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

Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan 
Smith, and xiaojian zhou.


Bugs: GEODE-911
https://issues.apache.org/jira/browse/GEODE-911


Repository: geode


Description
---

The stop() method was setting the EventProcessor to null before cleaning up the 
queues.  This led to no queues being cleaned up because the method getQueues 
would return null if the eventProcessor was null.

This includes refactoring a few methods in WanTestBase that pretty much did the 
same thing but was duplicated all over.

The Parallel queues do not have this problem because the EventProcessor is not 
set to null "for test purposes."  It seems like this should also be set to null 
as well and the tests that rely on it, should be fixed.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderQueue.java
 430409a 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderImpl.java
 8f0070f 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 5da6b5c 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentSerialGatewaySenderOperationsOffHeapDUnitTest.java
 e24f593 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderOperationsDUnitTest.java
 c824eb6 

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


Testing
---


Thanks,

Jason Huynh

Re: Review Request 45114: GEODE-27: Adding spring-webmvc to pulse

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On March 21, 2016, 5:46 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45114/
> ---
> 
> (Updated March 21, 2016, 5:46 p.m.)
> 
> 
> Review request for geode, Jason Huynh and Jens Deppe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Removing spring-webmvc from pulse caused these integration tests to fail:
> 
> com.gemstone.gemfire.distributed.LocatorLauncherAssemblyJUnitTest.testLocatorStopsWhenJmxPortIsZero
> com.gemstone.gemfire.distributed.LocatorLauncherAssemblyJUnitTest.testLocatorStopsWhenJmxPortIsNonZero
> com.gemstone.gemfire.management.internal.configuration.SharedConfigurationEndToEndDUnitTest.testStartServerAndExecuteCommands
> 
> 
> Diffs
> -
> 
>   geode-pulse/build.gradle 045d9038356a9bb2e543591211231e51456488fc 
> 
> Diff: https://reviews.apache.org/r/45114/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 45116: GEODE-27: Adding spring-expression back in to geode-web

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On March 21, 2016, 6:07 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45116/
> ---
> 
> (Updated March 21, 2016, 6:07 p.m.)
> 
> 
> Review request for geode, Jason Huynh and Jens Deppe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Not tests are failing due to this, but I discovered in a manual test of
> launching a locator that we were seeing errors due to the fact that the
> geode-web war could not be launched due to a missing spring expression
> class.
> 
> 
> Diffs
> -
> 
>   geode-web/build.gradle 11737057dc5774ba26719072458caaf8d960fed9 
> 
> Diff: https://reviews.apache.org/r/45116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 44980: GEODE-857: Wait for watchdog threads to start in SystemFailureJUnitTest

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On March 17, 2016, 9:26 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44980/
> ---
> 
> (Updated March 17, 2016, 9:26 p.m.)
> 
> 
> Review request for geode and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> I can't reproduce this failure, but maybe it's possible the isAlive flag
> is not set immediately on all platforms. So we'll have the test wait for
> the threads to be alive, rather than assert.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/com/gemstone/gemfire/SystemFailureJUnitTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44980/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Broken: apache/incubator-geode#210 (develop - 54931a5)

[Jason Huynh]

Sorry, forgot to add license headers to newly added files.

On Fri, Mar 18, 2016 at 1:37 PM, Travis CI <bui...@travis-ci.org> wrote:

> Build Update for apache/incubator-geode
> -
>
> Build: #210
> Status: Broken
>
> Duration: 15 minutes and 40 seconds
> Commit: 54931a5 (develop)
> Author: Jason Huynh
> Message: GEODE-1044: InitialImagePut executes BEFORE_UPDATE_OP on indexes
> for updates
>
> When entries are recovered from persistence, the values are added to the
> index,
> however a gii can then occur where the initialImagePut needs to properly
> alert the index that an update is occuring.  The BEFORE_UPDATE_OP allows
> the index
> to retreive the oldKey and notifies the index that an update is occuring.
>
> Refactor QueryTestUtils and removed unused code
> Minor modification for QueryTestUtils to be more generic and not be
> specific to Portfolios
>
> View the changeset:
> https://github.com/apache/incubator-geode/compare/8c74d600bf9e...54931a52ed3f
>
> View the full build log and details:
> https://travis-ci.org/apache/incubator-geode/builds/116999390
>
> --
>
> You can configure recipients for build notifications in your .travis.yml
> file. See https://docs.travis-ci.com/user/notifications
>
>

Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

[Jason Huynh]

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


Ship it!




Ship It!

- Jason Huynh


On March 16, 2016, 9:34 p.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> ---
> 
> (Updated March 16, 2016, 9:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java 
> PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>