from:"Bruce Schuchardt"

Review Request 53743: CI Failure: GMSHealthMonitorJUnitTest.testHMNextNeighborAfterTimeout

2016-11-14 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


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


Repository: geode


Description
---

I modified the test to wait longer and not require that the "next neighbor" be 
an exact member but merely a different one than the initial "next neigbor".

Also, the diagnostic test in the exception thrown by this method was including 
the JoinLeave membership view but that is null in these tests.  Instead it 
needs to show the HealthMonitor's membership view.  I added a method to 
GMSHealthMonitor to allow access to its view.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
 b04c6a1d4ddd5fe258a8148e7b5154d258a9abdb 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
 547a2003d91d2eb46f214622eef1bd229546010d 

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


Testing
---

Thousands of runs of the test, which only takes 1 second to execute.  I also 
ran the whole set of tests in this class over 200 times.


Thanks,

Bruce Schuchardt

Re: GMSJoinLeaveJUnitTest format is broken?

2016-11-14 Thread Bruce Schuchardt

This is fixed now.  I will not apologize for breaking the build though.  
I formatted the code that I checked in with the provided formatter 
configuration for Idea and that should pass the spotlessCheck task.


We should turn off spotlessCheck until the IDE formatters conform to the 
spotlessCheck rules.


Le 11/14/2016 à 10:27 AM, Kirk Lund a écrit :

* What went wrong:
Execution failed for task ':geode-core:spotlessJavaCheck'.

Format violations were found. Run 'gradlew spotlessApply' to fix them.

../../geode/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or
--debug option to get more log output.

BUILD FAILED

Re: Review Request 53652: GEODE-2089 entry-idle-time setting on the client side cache is not working as expected

2016-11-10 Thread Bruce Schuchardt


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

(Updated Nov. 10, 2016, 9:47 p.m.)


Review request for geode, Darrel Schneider, Hitesh Khamesra, and Udo Kohlmeyer.


Changes
---

renamed setLastModified(long,long) to 
setLastModifiedAndAccessedTimes(long,long) and added javadocs


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


Repository: geode


Description
---

When we pull an entry in from another cache in LocalRegion.findObjectInSystem() 
we end up invoking updateStatsForPut and then updateStatsForGet.  The former 
method installs a lastModifiedTime from the version tag that came from the 
other cache and this is used in updateStatsForPut to schedule an expiry-task 
for the entry.  Then updateStatsForGet is invoked to set the lastAccessed time 
of the entry to the current time.  However, by the time this is done the entry 
may have already been removed by the expiry-task if the lastModified time was 
too far in the past.

The fix is to establish both the lastModified and lastAccessed times in 
updateStatsForPut.

I've included two of the "leaf" RegionEntry classes in the diff but all of them 
had to be modified in the same way.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
 41ca8d084405480c8e4e69bb99db74e19529bc71 
  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java 
de05b0d3991254834da94ed97ada9c9247aa69ab 
  geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
e307c86a0aed48c0304a6a1ef90bdf147a05c379 
  
geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java
 6de85a1ab20b653e1731e0476ffb559bd093a121 
  geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 
0233b0f32d697c15a1148c95d6549f20997f3cc8 
  geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 
b5a3719ed00a56096582f1adf0b21b61b86563b9 
  geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java 
3448d1fba101a14d421584beb91fe216246794ed 
  
geode-core/src/main/java/org/apache/geode/internal/cache/VMStatsDiskLRURegionEntryHeapIntKey.java
 41bd3346c7a275b5fe83fe1df35ac87cc611dd83 
  
geode-core/src/test/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java 
7cf632f850098c178d4cf23bf116292d89220738 

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


Testing
---

New unit test, precheckin


Thanks,

Bruce Schuchardt

Re: Review Request 53652: GEODE-2089 entry-idle-time setting on the client side cache is not working as expected

2016-11-10 Thread Bruce Schuchardt



> On Nov. 10, 2016, 8:21 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java,
> >  lines 170-172
> > <https://reviews.apache.org/r/53652/diff/1/?file=1560532#file1560532line170>
> >
> > I think this overloaded method is a little confusing. It is setting the 
> > lastModified but takes lastAccessed as well. Then we only use lastModified.

I'll change the name of the method to be more understandable.


> On Nov. 10, 2016, 8:21 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java,
> >  lines 178-180
> > <https://reviews.apache.org/r/53652/diff/1/?file=1560532#file1560532line178>
> >
> > I think this implementation should read
> > 
> > ```setLastModified(lastModified);
> > setLastAccessed(lastAccessed);``` 
> > 
> > Then the VMStatsDiskLRURegionEntryHeapIntKey can override 
> > `updateStatsForPut` with its custom implementation.

AbstractRegionEntry doesn't have a setLastAccessedTime nor a lastAccessed 
instance variable.  Other subclasses of AbstractRegionEntry also do not have 
lastAccessedTime.  I'll change the name of this method so it's clearer and post 
an update.


> On Nov. 10, 2016, 8:21 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/VMStatsDiskLRURegionEntryHeapIntKey.java,
> >  lines 306-312
> > <https://reviews.apache.org/r/53652/diff/1/?file=1560539#file1560539line306>
> >
> > maybe the !DISABLE_ACCESS_TIME_UPDATE_ON_PUT logic should reside in the 
> > updateStatsForPut method, or at least override the normal behaviour from 
> > AbstractRegionEntry.

setLastModified is also invoked directly from transaction code in 
AbstractRegionMap.  Moving this to updateStatsForPut would change behavior for 
transactions, so I don't think we should mess with that.


- Bruce


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


On Nov. 10, 2016, 4:47 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53652/
> ---
> 
> (Updated Nov. 10, 2016, 4:47 p.m.)
> 
> 
> Review request for geode, Darrel Schneider, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2089
> https://issues.apache.org/jira/browse/GEODE-2089
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When we pull an entry in from another cache in 
> LocalRegion.findObjectInSystem() we end up invoking updateStatsForPut and 
> then updateStatsForGet.  The former method installs a lastModifiedTime from 
> the version tag that came from the other cache and this is used in 
> updateStatsForPut to schedule an expiry-task for the entry.  Then 
> updateStatsForGet is invoked to set the lastAccessed time of the entry to the 
> current time.  However, by the time this is done the entry may have already 
> been removed by the expiry-task if the lastModified time was too far in the 
> past.
> 
> The fix is to establish both the lastModified and lastAccessed times in 
> updateStatsForPut.
> 
> I've included two of the "leaf" RegionEntry classes in the diff but all of 
> them had to be modified in the same way.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
>  41ca8d084405480c8e4e69bb99db74e19529bc71 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  de05b0d3991254834da94ed97ada9c9247aa69ab 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> e307c86a0aed48c0304a6a1ef90bdf147a05c379 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java
>  6de85a1ab20b653e1731e0476ffb559bd093a121 
>   geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 
> 0233b0f32d697c15a1148c95d6549f20997f3cc8 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 
> b5a3719ed00a56096582f1adf0b21b61b86563b9 
>   geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java 
> 3448d1fba101a14d421584beb91fe216246794ed 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/VMStatsDiskLRURegionEntryHeapIntKey.java
>  41bd3346c7a275b5fe83fe1df35ac87cc611dd83 
>   
> geode-core/src/test/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java
>  7cf632f850098c178d4cf23bf116292d89220738 
> 
> Diff: https://reviews.apache.org/r/53652/diff/
> 
> 
> Testing
> ---
> 
> New unit test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Review Request 53652: GEODE-2089 entry-idle-time setting on the client side cache is not working as expected

2016-11-10 Thread Bruce Schuchardt


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

Review request for geode, Darrel Schneider, Hitesh Khamesra, and Udo Kohlmeyer.


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


Repository: geode


Description
---

When we pull an entry in from another cache in LocalRegion.findObjectInSystem() 
we end up invoking updateStatsForPut and then updateStatsForGet.  The former 
method installs a lastModifiedTime from the version tag that came from the 
other cache and this is used in updateStatsForPut to schedule an expiry-task 
for the entry.  Then updateStatsForGet is invoked to set the lastAccessed time 
of the entry to the current time.  However, by the time this is done the entry 
may have already been removed by the expiry-task if the lastModified time was 
too far in the past.

The fix is to establish both the lastModified and lastAccessed times in 
updateStatsForPut.

I've included two of the "leaf" RegionEntry classes in the diff but all of them 
had to be modified in the same way.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
 41ca8d084405480c8e4e69bb99db74e19529bc71 
  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java 
de05b0d3991254834da94ed97ada9c9247aa69ab 
  geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
e307c86a0aed48c0304a6a1ef90bdf147a05c379 
  
geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java
 6de85a1ab20b653e1731e0476ffb559bd093a121 
  geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 
0233b0f32d697c15a1148c95d6549f20997f3cc8 
  geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 
b5a3719ed00a56096582f1adf0b21b61b86563b9 
  geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java 
3448d1fba101a14d421584beb91fe216246794ed 
  
geode-core/src/main/java/org/apache/geode/internal/cache/VMStatsDiskLRURegionEntryHeapIntKey.java
 41bd3346c7a275b5fe83fe1df35ac87cc611dd83 
  
geode-core/src/test/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java 
7cf632f850098c178d4cf23bf116292d89220738 

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


Testing
---

New unit test, precheckin


Thanks,

Bruce Schuchardt

Re: Review Request 53557: GEODE-2080 Rest POST put call not working with region valueConstrain

2016-11-09 Thread Bruce Schuchardt



> On Nov. 8, 2016, 1:10 a.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > lines 3311-3326
> > <https://reviews.apache.org/r/53557/diff/1/?file=1556244#file1556244line3311>
> >
> > Maybe this is better suited on the PDXInstance. Maybe a method like 
> > "isTypeOf(Class). Then at least we don't spread the PDX logic everywhere in 
> > the code.

I had it coded like that but then LocalRegion had to import and interact with 
PdxInstanceImpl for the first time and I didn't want to do that.  Also, since 
this constraint check is just for Rest objects it didn't make sense to me to 
further pollute PdxInstanceImpl with knowledge about Rest objects.  I would 
rather have put it in JSonFormatter but that's a public API and this is an 
internal implementation detail.  So, no I don't agree with putting it in 
PdxInstanceImpl.


- Bruce


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


On Nov. 7, 2016, 10:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53557/
> ---
> 
> (Updated Nov. 7, 2016, 10:13 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2080
> https://issues.apache.org/jira/browse/GEODE-2080
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If you set a value constraint on a cache Region you will be unable to store 
> objects in the region via the Rest API.  This change-set modifies 
> LocalRegion's constraint check to look for a Rest document and use its type 
> name in the constraint check
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 3873e6e159ebba4c1a288e9fccde5dbabd2a1140 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 
> 
> Diff: https://reviews.apache.org/r/53557/diff/
> 
> 
> Testing
> ---
> 
> new test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 53557: GEODE-2080 Rest POST put call not working with region valueConstrain

2016-11-08 Thread Bruce Schuchardt



> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > line 3323
> > <https://reviews.apache.org/r/53557/diff/1/?file=1556244#file1556244line3323>
> >
> > Is ".equals" too specific? You want something like instanceof. For 
> > example if the value-constraint was "Number" you would want it to be 
> > satisfied if "valueClassName" was "Integer", "Long", etc.
> > 
> > You could do this with Class.isAssignableFrom(this.valueConstraint) but 
> > in that case you need to load a class for "valueClassName". One of the 
> > goals of pdx is that you do not need to be able to load the domain classes 
> > on the server, so you might not want to do this check. If you leave it with 
> > just a simple equals check you should document that in the case of pdx the 
> > value-constraint needs to exactly match (i.e. no instanceof support).
> 
> Bruce Schuchardt wrote:
> In the case of Rest objects there are no classes to perform this kind of 
> check so equals() is the correct operation to use.  For non-Pdx objects we 
> already use instanceof checks.  For non-Pdx objects we perform no constraint 
> check at all because it's already been performed and we can't be assured that 
> we have classes for the objects held by the PdxInstance anyway.

Edited: For non-Pdx objects we already use instanceof checks.  For 
**PdxInstance objects** we perform no constraint check at all because it's 
already been performed and we can't be assured that we have classes for the 
objects held by the PdxInstance anyway.


- Bruce


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


On Nov. 7, 2016, 10:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53557/
> ---
> 
> (Updated Nov. 7, 2016, 10:13 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2080
> https://issues.apache.org/jira/browse/GEODE-2080
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If you set a value constraint on a cache Region you will be unable to store 
> objects in the region via the Rest API.  This change-set modifies 
> LocalRegion's constraint check to look for a Rest document and use its type 
> name in the constraint check
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 3873e6e159ebba4c1a288e9fccde5dbabd2a1140 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 
> 
> Diff: https://reviews.apache.org/r/53557/diff/
> 
> 
> Testing
> ---
> 
> new test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 53557: GEODE-2080 Rest POST put call not working with region valueConstrain

2016-11-08 Thread Bruce Schuchardt



> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > line 3315
> > <https://reviews.apache.org/r/53557/diff/1/?file=1556244#file1556244line3315>
> >
> > If pdx.getClassName() is not equals to JSON_CLASSNAME then would it 
> > make sense to set valueClassName to pdx.getClassName()? That way you could 
> > have "read-serialized" set to true and still have a meaningful value 
> > constraint with pdx

No, definitely not.  That would cause it to start rejecting PdxInstances of 
subclasses of the constraint class.

For example, if the region is constrained to hold Person objects and a client 
entered an Employee object (subclass of Person) into the region a server would 
reject the value if we checked that the PdxInstance's class name had to match 
Person.

It's enough that the original cache that entered the object into the system 
performed a constraint check on the original object before it was serialized.  
That's not the case for Json objects coming from the Rest API - they need to be 
checked and that is the reason for this change.


> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > line 3323
> > <https://reviews.apache.org/r/53557/diff/1/?file=1556244#file1556244line3323>
> >
> > Is ".equals" too specific? You want something like instanceof. For 
> > example if the value-constraint was "Number" you would want it to be 
> > satisfied if "valueClassName" was "Integer", "Long", etc.
> > 
> > You could do this with Class.isAssignableFrom(this.valueConstraint) but 
> > in that case you need to load a class for "valueClassName". One of the 
> > goals of pdx is that you do not need to be able to load the domain classes 
> > on the server, so you might not want to do this check. If you leave it with 
> > just a simple equals check you should document that in the case of pdx the 
> > value-constraint needs to exactly match (i.e. no instanceof support).

In the case of Rest objects there are no classes to perform this kind of check 
so equals() is the correct operation to use.  For non-Pdx objects we already 
use instanceof checks.  For non-Pdx objects we perform no constraint check at 
all because it's already been performed and we can't be assured that we have 
classes for the objects held by the PdxInstance anyway.


- Bruce


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


On Nov. 7, 2016, 10:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53557/
> ---
> 
> (Updated Nov. 7, 2016, 10:13 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2080
> https://issues.apache.org/jira/browse/GEODE-2080
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If you set a value constraint on a cache Region you will be unable to store 
> objects in the region via the Rest API.  This change-set modifies 
> LocalRegion's constraint check to look for a Rest document and use its type 
> name in the constraint check
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 3873e6e159ebba4c1a288e9fccde5dbabd2a1140 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 
> 
> Diff: https://reviews.apache.org/r/53557/diff/
> 
> 
> Testing
> ---
> 
> new test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Review Request 53557: GEODE-2080 Rest POST put call not working with region valueConstrain

2016-11-07 Thread Bruce Schuchardt


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

Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer.


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


Repository: geode


Description
---

If you set a value constraint on a cache Region you will be unable to store 
objects in the region via the Rest API.  This change-set modifies LocalRegion's 
constraint check to look for a Rest document and use its type name in the 
constraint check


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
3873e6e159ebba4c1a288e9fccde5dbabd2a1140 
  geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 

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


Testing
---

new test, precheckin


Thanks,

Bruce Schuchardt

Re: more spotless problems on Windows

2016-11-04 Thread Bruce Schuchardt

I don't believe we did #2

Le 11/4/2016 à 12:09 PM, Jared Stewart a écrit :

@Bruce

To be clear, are you saying you did both of the following?

1). Create a .gitattributes file with “ *test eol=lf” at the top level of Geode
2). Change ‘UNIX’ in build.gradle to ‘GIT_ATTRIBUTES'

On Nov 4, 2016, at 12:07 PM, Bruce Schuchardt wrote:

Udo and I tried that & it didn't work. Maybe you'll have better luck.

Le 11/4/2016 à 11:59 AM, Jared Stewart a écrit :

From the spotless devs:

Git is not a pure content store, it mucks with line endings. Regardless of what
you check-in, it will store your files with unix line endings in the repo.

Then, when you checkout, it will modify the line endings to suit your platform. Unless
you add a .gitattributes file to tell git "forget the platform, do what this file
says".

Remove lineEndings 'UNIX' in your build.gradle [Jared - we should put
‘GIT_ATTRIBUTES’ in its place], and add a .gitattributes file in your root
directory with the content * text eol=lf and your problem will be fixed.

On Nov 4, 2016, at 10:48 AM, Nabarun Nag wrote:

Thank you Jared. I wanted to confirm that this was not an isolated incident
specific to my machine.

Regards
Naba

On Fri, Nov 4, 2016 at 10:45 AM Jared Stewart wrote:

@Naba,

I filed a bug report with Spotless this morning. The Spotless devs have
been very responsive so far in my experience, hopefully this will be fixed
soon.

On Nov 4, 2016, at 10:35 AM, Nabarun Nag wrote:

@Udo, I confirmed that this is not limited to my windows 10 environment.

ran the steps on a Windows Server 2016 AMI instance and the same error
occurred in the AMI too. [source checkout time 4th Nov 10:00AM PST]

I wanted to know if there is a mandate on what the value of
core.autocrlf should
be set to on a windows machine for geode dev work. For my experiments

value

of core.autocrlf was set to true. [recommended for cross platform
development]

Regards
Naba

On Thu, Nov 3, 2016 at 10:18 PM Nabarun Nag wrote:

@Jared
I ran ./gradlew spotlessApply on the Windows 10 machine using git bash
this is what has happened.
NOTE: I started the below steps on a fresh git clone of the open side.
[Steps:
1. git clone

https://git-wip-us.apache.org/repos/asf/incubator-geode.git

open
2. cd open
3. git checkout -b develop origin/develop]

*Step 1. ./gradlew clean build -Dskip.tests=true*

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':geode-core:spotlessJavaCheck'.

Format violations were found. Run 'gradlew spotlessApply' to fix them.

geode-core\src\main\java\org\apache\geode\internal\statistics\StatArchiveReader.java
geode-core\src\test\java\org\apache\geode\cache\query\dunit\PdxLocalQueryVersionedClassDUnitTest.java
geode-core\src\test\java\org\apache\geode\internal\cache\execute\ClientServerFunctionExecutionDUnitTest.java
geode-core\src\test\java\org\apache\geode\internal\cache\functions\TestFunction.java
geode-core\src\test\java\org\apache\geode\internal\statistics\StatArchiveWithMissingResourceTypeRegressionTest.java

*Step 2: ./gradlew spotlessApply*

BUILD SUCCESSFUL

Total time: 12.728 secs

*Step 3: git status*

modified:

geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java

modified:

geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxLocalQueryVersionedClassDUnitTest.java

modified:

geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java

modified:

geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java

modified:

geode-core/src/test/java/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.java

*Step 4 : git add .*
warning: LF will be replaced by CRLF in

geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java.

The file will have its original line endings in your working directory.
warning: LF will be replaced by CRLF in

geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxLocalQueryVersionedClassDUnitTest.java.

The file will have its original line endings in your working directory.
warning: LF will be replaced by CRLF in

geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java.

The file will have its original line endings in your working directory.
warning: LF will be replaced by CRLF in

geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java.

The file will have its original line endings in your working directory.
warning: LF will be replaced by CRLF in

geode-core/src/test/java/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.java.

The file will have its original line endings in your working directory.

*Step 5: git status*
On branch develop
Your branch is up-to-date with &

Re: more spotless problems on Windows

2016-11-04 Thread Bruce Schuchardt

 spotlessApply and git add . the first time, the
spotless errors do not reoccur on subsequent builds.

I will try running this on other machines and check if this occurs in
other windows environments.


Regards
Naba

On Thu, Nov 3, 2016 at 4:25 PM Udo Kohlmeyer 
wrote:

@Jared, I mailed with the Spotless project devs and they recommend using
.gitattributes. But maybe @Naba's problem is Windows10 related... Who
knows..

--Udo


On 4/11/16 10:16 am, Jared Stewart wrote:

The only Windows machine I have is running Windows 8, and I am unable

to

reproduce this on that machine.  I don’t think .gitattributes would

affect

this, since we have already configured spotless to always use Unix line
endings.

Naba - Can you run ‘./gradlew spotlessApply’ and push the results to a

branch so I can see what Spotless was complaining about?

On Nov 3, 2016, at 4:09 PM, Udo Kohlmeyer  wrote:

I think we seriously have to look at using .gitattributes for this...

As I initially said, it should be a no brainer.. it should just

automatically just work.

--Udo


On 4/11/16 9:00 am, Bruce Schuchardt wrote:

It's been working on my Windows 7 machine under a cygwin shell.   I

just ran it again using "clean bulid -Dskip.tests=true" from the root

Geode

directory on the develop branch.

Run spotlessApply and let us know how it modified the files.


Le 11/3/2016 à 12:38 PM, Nabarun Nag a écrit :

I tested gradlew build on a windows 10 machine to test the spotless

feature.

Steps:
1.  git clone

https://git-wip-us.apache.org/repos/asf/incubator-geode.git

open
2. cd open
3. git checkout -b develop origin/develop
4.  ./gradlew clean build -Dskip.tests=true

The build failed with multiple formatting error on each file.

In my opinion the issue still exists. It will be awesome if someone

else

can verify if the issue still exists by running the build steps on a
different windows machine.

Regards
Nabarun

On Mon, Oct 24, 2016 at 3:50 PM Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:


The lineEndings setting works great. I've pushed the change to

develop

On Mon, Oct 24, 2016 at 3:47 PM, Dan Smith 

wrote:

I think we have a fix for the spotless line ending issue on

windows;

Bruce

will check it in shortly:

diff --git a/build.gradle b/build.gradle
index a734e05..6e82433 100755
--- a/build.gradle
+++ b/build.gradle
@@ -88,6 +88,7 @@ subprojects {

   apply plugin: "com.diffplug.gradle.spotless"
   spotless {
+lineEndings = 'unix';
 java {
   eclipseFormatFile
"${rootProject.projectDir}/etc/eclipse-java-google-style.xml"


On Mon, Oct 24, 2016 at 2:50 PM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:


Running geode-core:spotlessCheck complains that all of the .java

files

have format violations

* What went wrong:
Execution failed for task ':geode-core:spotlessJavaCheck'.

Format violations were found. Run 'gradlew spotlessApply' to fix

them.

geode-core\src\jca\java\org\apache\geode\internal\ra\GFConne
ctionFactoryImpl.java
geode-core\src\jca\java\org\apache\geode\internal\ra\

GFConnectionImpl.java

geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
LocalTransaction.java
geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
ManagedConnection.java
geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
ManagedConnectionFactory.java
geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
ManagedConnectionnMetaData.java
etc.

Until this is fixed I can't validate that the changes I check in

conform

to the formatting rules.

Re: Review Request 53409: GEODE-2017: Removal of nonSingleHopsCount stat in client

2016-11-03 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Nov. 3, 2016, 5:58 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53409/
> ---
> 
> (Updated Nov. 3, 2016, 5:58 p.m.)
> 
> 
> Review request for geode and Bruce Schuchardt.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Removal of nonSingleHopsCount stat in client as it serves little purpose in 
> what it inteded to display.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientMetadataService.java
>  915e1bf 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java 
> 395e8c7 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 360c6a9 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/SingleHopStatsDUnitTest.java
>  1b6a00c 
> 
> Diff: https://reviews.apache.org/r/53409/diff/
> 
> 
> Testing
> ---
> 
> precheckin - completed
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: more spotless problems on Windows

2016-11-03 Thread Bruce Schuchardt

It's been working on my Windows 7 machine under a cygwin shell.   I just 
ran it again using "clean bulid -Dskip.tests=true" from the root Geode 
directory on the develop branch.


Run spotlessApply and let us know how it modified the files.


Le 11/3/2016 à 12:38 PM, Nabarun Nag a écrit :

I tested gradlew build on a windows 10 machine to test the spotless feature.

Steps:
1.  git clone https://git-wip-us.apache.org/repos/asf/incubator-geode.git
open
2. cd open
3. git checkout -b develop origin/develop
4.  ./gradlew clean build -Dskip.tests=true

The build failed with multiple formatting error on each file.

In my opinion the issue still exists. It will be awesome if someone else
can verify if the issue still exists by running the build steps on a
different windows machine.

Regards
Nabarun

On Mon, Oct 24, 2016 at 3:50 PM Bruce Schuchardt 
wrote:


The lineEndings setting works great.  I've pushed the change to develop

On Mon, Oct 24, 2016 at 3:47 PM, Dan Smith  wrote:


I think we have a fix for the spotless line ending issue on windows;

Bruce

will check it in shortly:

diff --git a/build.gradle b/build.gradle
index a734e05..6e82433 100755
--- a/build.gradle
+++ b/build.gradle
@@ -88,6 +88,7 @@ subprojects {

apply plugin: "com.diffplug.gradle.spotless"
spotless {
+lineEndings = 'unix';
  java {
eclipseFormatFile
"${rootProject.projectDir}/etc/eclipse-java-google-style.xml"


On Mon, Oct 24, 2016 at 2:50 PM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:


Running geode-core:spotlessCheck complains that all of the .java files
have format violations

* What went wrong:
Execution failed for task ':geode-core:spotlessJavaCheck'.

Format violations were found. Run 'gradlew spotlessApply' to fix

them.

geode-core\src\jca\java\org\apache\geode\internal\ra\GFConne
ctionFactoryImpl.java
geode-core\src\jca\java\org\apache\geode\internal\ra\

GFConnectionImpl.java

geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
LocalTransaction.java
geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
ManagedConnection.java
geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
ManagedConnectionFactory.java
geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
ManagedConnectionnMetaData.java
etc.

Until this is fixed I can't validate that the changes I check in

conform

to the formatting rules.

Re: Review Request 53410: GEODE-2064 Added check for system shutdown while handlling connect exception.

2016-11-03 Thread Bruce Schuchardt


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



That looks okay but since it's changing shutdown behavior please run 
splitBrain/splitBrain.bt and splitBrain/networkPartition3Hosts.bt

- Bruce Schuchardt


On Nov. 2, 2016, 11:41 p.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53410/
> ---
> 
> (Updated Nov. 2, 2016, 11:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Eric Shu, Scott 
> Jewell, Ken Howe, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> While message send in progress, if the system gets shutdown (forced 
> disconnect), the send (message delivery to peers) reports connect exception 
> and ignores detecting/throwing SystemDisconnect exception. 
> 
> In "DirectChannel.getConnection()" it checks for "mgr.shutdownInProgress()" 
> and returns ConnectException to the caller 
> "GMSMembershipManager.directChannelSend()"
> 
> In client/server scenario, if the client is performing cache operation, the 
> cache operation may succeed in server that is getting down and failure to 
> deliver the message to other peers/servers. The client will see the operation 
> getting successfully completed.
> 
> The above scenario could result in client missing an event and resulting in 
> data mismatch between client and server.
> 
> Made changes to throw "DistributedSystemDisconnectedException" if system is 
> shutting down. This will result in caller to retry the operation.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  a4691f4 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManagerJUnitTest.java
>  bae1ddc 
> 
> Diff: https://reviews.apache.org/r/53410/diff/
> 
> 
> Testing
> ---
> 
> Added new unit test. Verified the test without my change and with the change. 
> With change test looks for DistributedSystemDisconnectedException to be 
> thrown.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: PMC nomination

2016-11-02 Thread Bruce Schuchardt


Great - thanks Greg

Le 11/2/2016 à 11:31 AM, Gregory Chase a écrit :

I saw your post to @private.

On Wed, Nov 2, 2016 at 11:27 AM, Bruce Schuchardt 
wrote:


I've been trying to join the private email list all morning &
self-nominate for the PMC if it's not too late

PMC nomination

2016-11-02 Thread Bruce Schuchardt

I've been trying to join the private email list all morning & 
self-nominate for the PMC if it's not too late

Review Request 53388: GEODE-2059 client SSL handshake attempts do not time out

2016-11-02 Thread Bruce Schuchardt


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

Review request for geode and Udo Kohlmeyer.


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


Repository: geode


Description
---

Client SSL connection attempts now time out during the SSL handshake


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java 
a3ba102f0680cacd10368d7bd4e43d9c47bc85f1 
  
geode-core/src/test/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
 d5a947cf40295328039159df2b5af5292cef724c 

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


Testing
---

precheckin, new unit test


Thanks,

Bruce Schuchardt

Re: Review Request 53360: GEODE-2050 Remove doc of statistics no longer present

2016-11-02 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Nov. 1, 2016, 11:55 p.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53360/
> ---
> 
> (Updated Nov. 1, 2016, 11:55 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Dave Barnes, and Joey McAllister.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> JGroups-related statistics were removed in the first commit. New membership 
> service-related statistics were added in the second commit.
> 
> 
> Diffs
> -
> 
>   geode-docs/reference/statistics/statistics_list.html.md.erb 
> 227f8dfda8747d00b2e9077f920646d68f3f8acf 
> 
> Diff: https://reviews.apache.org/r/53360/diff/
> 
> 
> Testing
> ---
> 
> Gradle rat check passes.
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Re: Backwards compatibility for 1.1

2016-11-01 Thread Bruce Schuchardt


+1

I still have the backward-compatibility distributedTest extensions that 
I could contribute.  The extension lets you spawn a VM running an older 
version and interact with it.  You can even run a unit test in the 
spawned VM.


I have one test that sets up a server using the current version and then 
spawns a client unit test running under an older version.  The client 
finds the server through the distributedTest locator and runs its tests 
against the server.



Le 11/1/2016 à 4:04 PM, Jianxia Chen a écrit :

+1

On Tue, Nov 1, 2016 at 4:00 PM, Dan Smith  wrote:


Hi,

We made a lot of changes in 1.0 that broke compatibility with old versions
of gemfire for various reasons (package renaming, changing membership
system). I just wanted to confirm that starting with 1.1, we're planning on
maintaining client/server, peer-to-peer, WAN and disk backwards
compatibility with older versions geode as outlined in this wiki page:

https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+
Compatibility
https://cwiki.apache.org/confluence/display/GEODE/
Criteria+for+Code+Submissions

Now that we have 1.0 out the door, we need to be more careful about
introducing changes that might break compatibility if we're going to stick
to these guidelines. We also probably should introduce some tests that
check compatibility with 1.0.


-Dan

Re: Review Request 53331: GEODE-2047 Document change to enable-network-partition-detection

2016-11-01 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Nov. 1, 2016, 4:02 p.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53331/
> ---
> 
> (Updated Nov. 1, 2016, 4:02 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Dave Barnes, and Joey McAllister.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> - This is a subtask of GEODE-762.
> - The default value of property enable-network-partition-detection
> changed from false to true, enabling partition detection by
> default, so all documentation that discusses partition detection
> also needs to change.
> - Fixed a minor typo or two encountered in the files that were
> being updated.
> 
> 
> Diffs
> -
> 
>   
> geode-docs/managing/troubleshooting/recovering_conflicting_data_exceptions.html.md.erb
>  38375aea1e5742d878d1e7dbaf3c92d67320d17f 
>   
> geode-docs/managing/troubleshooting/recovering_from_network_outages.html.md.erb
>  8c23beac365af0f8f917b13447c2ae6382ad2525 
>   geode-docs/reference/topics/gemfire_properties.html.md.erb 
> 988256840a084cac56a4e38723415a4c4ea2d99f 
> 
> Diff: https://reviews.apache.org/r/53331/diff/
> 
> 
> Testing
> ---
> 
> gradle rat check passes
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Re: Fwd: Build failed in Jenkins: Geode-nightly #636

2016-10-28 Thread Bruce Schuchardt

Someone should look into this test and the previously run Auth tests to 
see what caused this OOME


   
org.apache.geode.internal.statistics.DiskSpaceLimitIntegrationTest.sameKeepsOneFile
   




 Error Details
   

   java.lang.OutOfMemoryError: GC overhead limit exceeded


Previously run tests: [
PeerSecurityWithEmbeddedLocatorDUnitTest,
PostProcessorDUnitTest,
IntegratedClientContainsKeyAuthDistributedTest,
IntegratedClientGetEntryAuthDistributedTest,
IntegratedClientUnregisterInterestAuthDistributedTest,
PDXGfshPostProcessorOnRemoteServerTest,
IntegratedClientExecuteFunctionAuthDistributedTest,
IntegratedClientDestroyInvalidateAuthDistributedTest]


Le 10/28/2016 à 9:15 AM, Kirk Lund a écrit :

Anyone know why the job output didn't include the test failures like it
usually does?

-Kirk

-- Forwarded message --
From: Apache Jenkins Server 
Date: Fri, Oct 28, 2016 at 8:54 AM
Subject: Build failed in Jenkins: Geode-nightly #636
To: dev@geode.incubator.apache.org, n...@pivotal.io, bogle...@pivotal.io,
dschnei...@pivotal.io, gz...@pivotal.io, dbar...@pivotal.io,
jil...@pivotal.io, kl...@pivotal.io, bschucha...@pivotal.io,
aba...@pivotal.io, kmil...@pivotal.io, sbawas...@pivotal.io


See 

Changes:

[kmiller] GEODE-2040 Add to docs Tomcat 8.0 and 8.5 for HTTP Session Mgmt
module

[bschuchardt] GEODE-2000 client should see server-bind-address in event
memberId

[boglesby] GEODE-2027: ParallelQueueRemovalMessage processing removes
events from

[bschuchardt] fixing dumpStack/dumpAllStacks methods

[boglesby] GEODE-2017: Fixed formatting

[klund] GEODE-2012: always write stat types to archive

--
[...truncated 588 lines...]
:geode-cq:check
:geode-cq:build
:geode-cq:distributedTest
:geode-cq:flakyTest
:geode-cq:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources UP-TO-DATE
:geode-json:testClasses UP-TO-DATE
:geode-json:checkMissedTests UP-TO-DATE
:geode-json:spotlessJavaCheck
:geode-json:spotlessCheck
:geode-json:test UP-TO-DATE
:geode-json:check
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:flakyTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:flakyTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene: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-lucene:processTestResources
:geode-lucene:testClasses
:geode-lucene:checkMissedTests
:geode-lucene:spotlessJavaCheck
:geode-lucene:spotlessCheck
:geode-lucene:test
:geode-lucene:check
:geode-lucene:build
:geode-lucene:distributedTest
:geode-lucene:flakyTest
:geode-lucene:integrationTest
:geode-old-client-support:assemble
:geode-old-client-support:compileTestJava
:geode-old-client-support:processTestResources UP-TO-DATE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest
:geode-old-client-support:flakyTest
:geode-old-client-support:integrationTest
:geode-pulse:assemble
:geode-pulse:compileTestJavaNote: /x1/jenkins/jenkins-slave/
workspace/Geode-nightly/geode-pulse/src/test/java/org/
apache/geode/tools/pulse/tests/ui/PulseAbstractTest.java uses or overrides
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-pulse:processTestResources
:geode-pulse:testClasses
:geode-pulse:checkMissedTests
:geode-pulse:spotlessJavaCheck
:geode-pulse:spotlessCheck
:geode-pulse:test
:geode-pulse:check
:geode-pulse:build
:geode-pulse:distributedTest
:geode-pulse:flakyTest
:geode-pulse:integrationTest
:geode-rebalancer:assemble
:geode-rebalancer:compileTestJava
:geode-rebalancer:processTestResources UP-TO-DATE
:geode-rebalancer:testClasses
:geode-rebalancer:checkMissedTests
:geode-rebalancer:spotlessJavaCheck
:geode-rebalancer:spotlessCheck
:geode-rebalancer:test
:geode-rebalancer:check
:geode-rebalancer:build
:geode-rebalancer:distributedTest

Re: CI failures due to JMX client heartbeat thread

2016-10-27 Thread Bruce Schuchardt

I think the code is in Gfsh.notifyDisconnect().  I think the test that's 
polluting others is ConnectToLocatorSSLDUnitTest.  I had a failure with 
this suspect string in a test that ran just after it. I added a thread 
dump in the @After of that test and see that it's leaving one of these 
threads behind along with some Gfsh Launcher threads.


Le 10/27/2016 à 10:54 AM, Kirk Lund a écrit :

Bruce, can you please point me at the JDK code you think is generating the
fatal message? I looked in RMIConnector.java and can't find it in there.

Maybe we can fix ordering of tearDown or something else in order to "fix"
this message.

Thanks,
Kirk


On Wed, Oct 26, 2016 at 5:00 PM, Dan Smith  wrote:


Just looking at a couple of these bugs, these are fatal level errors. Do
you know what is causing them or what affect this might have?

[fatal 2016/09/29 12:12:03.891 PDT  tid=0x18d]
(tid=397 msgId=39) No longer connected to cc6-co6.gemstone.com[27162].

-Dan

On Wed, Oct 26, 2016 at 4:50 PM, Bruce Schuchardt 
wrote:


We have 23 CI failure tickets concerning suspect strings from "JMX client
heartbeat" threads.  I think we should add this to ExpectedStrings.java

and

close these tickets.  The suspect strings are coming from
javax.management.remote.rmi.RMIConnector and I don't think there's
anything we can do about them.  Most, if not all, are assigned to

security

or JMX components.


GEODE-1858
GEODE-1955
GEODE-1492
GEODE-1539
GEODE-1538
GEODE-1773
GEODE-1922
GEODE-1482
GEODE-1475
GEODE-1480
GEODE-1481
GEODE-1826
GEODE-1825
GEODE-1820
GEODE-1878
GEODE-1879
GEODE-1877
GEODE-1875
GEODE-1876
GEODE-1499
GEODE-1476
GEODE-1869
GEODE-1769

Re: CI failures due to JMX client heartbeat thread

2016-10-27 Thread Bruce Schuchardt

I spent some time digging into this more and it looks like these threads 
are probably left behind by tests using Gfsh.  It creates a JMX 
heartbeat thread that issues "severe" (i.e. "fatal") warnings with the 
message we're seeing



  public void notifyDisconnect(String endPoints) {
String message =
CliStrings.format(CliStrings.GFSH__MSG__NO_LONGER_CONNECTED_TO_0,
new Object[] {endPoints});
printAsSevere(LINE_SEPARATOR + message);
if (gfshFileLogger.severeEnabled()) {
  gfshFileLogger.severe(message);
}
setPromptPath(

org.apache.geode.management.internal.cli.converters.RegionPathConverter.DEFAULT_APP_CONTEXT_PATH);
  }


In a recent test run I can see that ConnectToLocatorSSLDUnitTest ran 
just before a test that logged this fatal-level message.  Adding an 
@After to ConnectToLocatorSSLDUnitTest that dumps threads I can see that 
it is leaving behind a "JMX client heartbeat 4" thread as well as 
several "Gfsh Launcher" threads.


We should fix these tests to properly terminate Gfsh threads during cleanup.


Le 10/26/2016 à 5:00 PM, Dan Smith a écrit :

Just looking at a couple of these bugs, these are fatal level errors. Do
you know what is causing them or what affect this might have?

[fatal 2016/09/29 12:12:03.891 PDT  tid=0x18d]
(tid=397 msgId=39) No longer connected to cc6-co6.gemstone.com[27162].

-Dan

On Wed, Oct 26, 2016 at 4:50 PM, Bruce Schuchardt 
wrote:


We have 23 CI failure tickets concerning suspect strings from "JMX client
heartbeat" threads.  I think we should add this to ExpectedStrings.java and
close these tickets.  The suspect strings are coming from
javax.management.remote.rmi.RMIConnector and I don't think there's
anything we can do about them.  Most, if not all, are assigned to security
or JMX components.


GEODE-1858
GEODE-1955
GEODE-1492
GEODE-1539
GEODE-1538
GEODE-1773
GEODE-1922
GEODE-1482
GEODE-1475
GEODE-1480
GEODE-1481
GEODE-1826
GEODE-1825
GEODE-1820
GEODE-1878
GEODE-1879
GEODE-1877
GEODE-1875
GEODE-1876
GEODE-1499
GEODE-1476
GEODE-1869
GEODE-1769

CI failures due to JMX client heartbeat thread

2016-10-26 Thread Bruce Schuchardt

We have 23 CI failure tickets concerning suspect strings from "JMX 
client heartbeat" threads.  I think we should add this to 
ExpectedStrings.java and close these tickets.  The suspect strings are 
coming from javax.management.remote.rmi.RMIConnector and I don't think 
there's anything we can do about them.  Most, if not all, are assigned 
to security or JMX components.



   GEODE-1858
   GEODE-1955
   GEODE-1492
   GEODE-1539
   GEODE-1538
   GEODE-1773
   GEODE-1922
   GEODE-1482
   GEODE-1475
   GEODE-1480
   GEODE-1481
   GEODE-1826
   GEODE-1825
   GEODE-1820
   GEODE-1878
   GEODE-1879
   GEODE-1877
   GEODE-1875
   GEODE-1876
   GEODE-1499
   GEODE-1476
   GEODE-1869
   GEODE-1769

Review Request 53199: GEODE-2000 ClientMemberShipListener at client should see server-bind-address in event memberId

2016-10-26 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


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


Repository: geode


Description
---

The previous fix for this caused confusion as it changed the server memberId 
that is used in other places and should remain unchanged.  This change set 
alters just the listener-invocation code in the client cache so that client 
events are based on the ServerLocation information returned by the Locator or 
added to the connection pool by applications.

Udo worked with me on this and we found the listener invocation code to be 
somewhat convoluted, mixing server-side notification about clients with 
client-side notification about servers in the same code.  This lead to a bit of 
refactoring in InternalClientMembership to separate the two.

A number of changes had to be made in test code.  Some tests were requiring 
that client-side listeners see the server's exact member ID which is no longer 
true since the ID being fabricated out of a ServerLocation doesn't have as much 
detail as the true member ID and so is not equal() to it.  Some other test code 
was creating ServerLocation objects with non-existent host names.  This is no 
longer allowed so we changed these tests to use a numeric IP address.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
 ec8a81833ba502fae4672529f755147d5639d42e 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
 ac8379bddab6582daf7661640e9f894633bfda67 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
 656f7ded38b175792f08255f8f916e89a704db2e 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientUpdater.java
 90cdedaaa847af956c088075df6ff42326712118 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
 5e13be091070e5c31d61f3288ca7008e64481f5f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
 47932d014538ec55e5781a451534715be2d82f25 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 63fc8d58ea2c728e0d8c633f8be44d057225c2fc 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerJUnitTest.java
 1017db2229b437837e6dfb10e00cdb12d709b5e8 
  
geode-core/src/test/java/org/apache/geode/cache/partition/PartitionRegionHelperDUnitTest.java
 a4494ca71b32bf97b0e65b6eb02036f4be4b45d1 
  
geode-core/src/test/java/org/apache/geode/cache30/ClientMembershipDUnitTest.java
 83b75d5f92915710a6be1eff903df79e741737ce 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
 1329c24fb1af808aa245e550f75e259bb9bb63c3 
  
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionQueryEvaluatorIntegrationTest.java
 4e8408c74a718ce2333e725ca83d8fed7f4b42c0 
  
geode-core/src/test/java/org/apache/geode/management/UniversalMembershipListenerAdapterDUnitTest.java
 019bd0f2acc76e2cffec342868f221bdf55b431c 
  geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java 
aaf3e281c7fba82f2ab437122af66373ce967494 

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


Testing
---

precheckin, new unit test


Thanks,

Bruce Schuchardt

Re: [DISCUSS] Graduation

2016-10-26 Thread Bruce Schuchardt


+1

Le 10/25/2016 à 5:25 PM, Roman Shaposhnik a écrit :

Hi!

with the 1.0.0-incubating release officially out (huge kudos
to the team!) I think it is time we officially start our graduation
discussion. Process-wise graduation consists of drafting
a board resolution, getting it approved by the IPMC and finally
submitting it to the ASF board's consideration. At the very minimum
your resolution will contain:
 1. A name of the project (I assume that'll be Geode)
 2. A list of proposed PMC
 3. A proposed PMC chair
A good example of a resolution can be found here:
 http://markmail.org/message/7ow2kdqa3rdx3x5k

On #2 my suggestion would be to have an opt-in system. Basically
we will kick off the thread off on private@geode asking current PPMC
members if they are willing to continue on the PMC.

On #3 I typically recommend podlings I mento to setup a rotating chair
policy. This is, in no way, an ASF requirement so feel free to ignore it,
but it worked well before. The chair will be expected up for rotation every
year. It will be more that ok for the same person to self-nominate once
the year is up -- but at the same time it'll be up to the same person to
actually kick off a thread asking if anybody else is interested in serving
as a chair for the next year. Of course, if there multiple candidates there
will have to be a vote.

Speaking of self-nomination -- the same thread that we're going to kick
off as part of solving for #2 will ask for folks to self-nominate as an initial
chair to be listed on the resolution.

Unless somebody objects strongly to my #2 and #3 proposals I'm going
to kick of this thread on private@.

With that in mind, lets make the rest of the discussion on dev@ to be about
collecting the datapoints to present to IPCM as part of us asking them to
vote YES on our graduation. You can see my initial list of these data points
below. Please, please add yours:

Project status:
  http://incubator.apache.org/projects/geode.html

Maturity assessment:
  
https://cwiki.apache.org/confluence/display/GEODE/Geode+Podling+Maturity+Assessment

4 releases

2734 commits on develop
269 PR”s
92 contributors across all branches

dev list averaged ~650 msgs/month in 2016
user list averaged ~90 msgs/month in 2016
267 unique posters

1900 issues created
1230 issues resolved

6 new committers / PPMC members added

Quick bump:  as far as committer diversity goes here’s the picture:

committer domains
  reasonably active
   pivotal.io
   kollective.com (Mark Bretl)
   ampool.io
   silverspringnet.com (Sai Boorlagadda)

  inactive
   snappydata.io
   newrelic.com (Qihong)
   google.com (Sourabh)
   microsoft.com (Ashvin)
   weave.works (Stuart)
   elastic.co (Catherine)
   dell.com (Lyndon, …)
   vmware.com
   brsg.io
   cobaltdigital.marketing
   cdkglobal.com
   dev9.com

Thanks,
Roman.

Review Request 53150: GEODE-2024 Deadlock creating a new distributed lock service Grantor while transactions are in progress

2016-10-25 Thread Bruce Schuchardt


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

Review request for geode, Darrel Schneider, Kirk Lund, and Udo Kohlmeyer.


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


Repository: geode


Description
---

This change-set causes the code in TXLockServiceImpl.release() to perform 
periodic checks to see if grantor recovery is being performed.  If so it skips 
releaseTryLocks, which requires a recovered grantor to function.  This is in 
line with the previous attempts to fix this problem.  The recovery message that 
is trying to obtain the recovery write-lock now sets the "recovering" state in 
TXLockServiceImpl prior to attempting to get the lock so that it is set when 
TXLockServiceImpl.release() checks its state.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRecoverGrantorProcessor.java
 37fbfbe978437db15ba1d3439b28fdcc7eb0cee8 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
 a859299a3581939740f6c62cc759179bfdd22ee1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLockServiceImpl.java
 f4ab02feb67490da03209d8953da30c0415e4677 
  
geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXRecoverGrantorMessageProcessor.java
 77dec946d920bf4f6d3f732c9d68f7c94bd7ea01 
  
geode-core/src/test/java/org/apache/geode/internal/cache/locks/TXLockServiceDUnitTest.java
 fb16ea9e50da205322b17a9f46d6153729bf3850 

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


Testing
---

TXLockServiceDUnitTest had a test for previous occurences of this problem but 
it was marked @Ignore.  I've fixed the timing issues with that test but it 
wasn't quite modeling what happened in the GEODE-2024 failure so I added 
another test for grantor fail-over.  The new test ensures that another member 
is the grantor prior to obtaining TX locks.  Then it initiates grantor failover 
and tells the TXLockService to release its locks.


Thanks,

Bruce Schuchardt

Re: more spotless problems on Windows

2016-10-24 Thread Bruce Schuchardt

The lineEndings setting works great.  I've pushed the change to develop

On Mon, Oct 24, 2016 at 3:47 PM, Dan Smith  wrote:

> I think we have a fix for the spotless line ending issue on windows; Bruce
> will check it in shortly:
>
> diff --git a/build.gradle b/build.gradle
> index a734e05..6e82433 100755
> --- a/build.gradle
> +++ b/build.gradle
> @@ -88,6 +88,7 @@ subprojects {
>
>apply plugin: "com.diffplug.gradle.spotless"
>spotless {
> +lineEndings = 'unix';
>  java {
>eclipseFormatFile
> "${rootProject.projectDir}/etc/eclipse-java-google-style.xml"
>
>
> On Mon, Oct 24, 2016 at 2:50 PM, Bruce Schuchardt 
> wrote:
>
> > Running geode-core:spotlessCheck complains that all of the .java files
> > have format violations
> >
> > * What went wrong:
> > Execution failed for task ':geode-core:spotlessJavaCheck'.
> > > Format violations were found. Run 'gradlew spotlessApply' to fix them.
> > geode-core\src\jca\java\org\apache\geode\internal\ra\GFConne
> > ctionFactoryImpl.java
> > geode-core\src\jca\java\org\apache\geode\internal\ra\
> GFConnectionImpl.java
> > geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
> > LocalTransaction.java
> > geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
> > ManagedConnection.java
> > geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
> > ManagedConnectionFactory.java
> > geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
> > ManagedConnectionnMetaData.java
> > etc.
> >
> > Until this is fixed I can't validate that the changes I check in conform
> > to the formatting rules.
> >
>

Re: Coding practices/standards

2016-10-24 Thread Bruce Schuchardt

I'm having trouble with Spotless.   spotlessApply on MS-Windows is 
changing all of the files it touches to have CRLF line separators. 
Adding endWithNewline() to the java config in build.gradle doesn't 
help.  If I run geode-core:spotlessApply I end up with over 5000 
modified files.



Le 10/24/2016 à 11:09 AM, Udo Kohlmeyer a écrit :

+1 - Nice work. Thx


On 24/10/16 10:40 am, Dan Smith wrote:

Doing a spotlessApply on my feature branch before rebasing didn't help
bring down the number of conflicts.

I came up with this sequence of steps to rebase a feature branch on 
develop

that avoids the need to manually resolve conflicts with the formatting
changes. The trick here is to pick up *just* the formatting changes 
in one
of the steps, and then reject any formatting changes that conflict 
with my

changes.

#Rebase onto the commit before the spotless change. Resolve conflicts 
if any

git rebase 56917a26a8916b83f0cec6e85285b5040ff66ee6

#Rebase onto the spotless change, automatically throwing away the
formatting changes if they conflict.
git rebase -Xtheirs c2319bb7a6201d5ae82ecb0fe23a1e3b8072c2e1

#Rebase onto the rest of develop. Resolve conflicts if any.
git rebase origin/develop

#Apply formatting
./gradlew geode-core:spotlessApply

-Dan

On Fri, Oct 21, 2016 at 4:34 PM, Jared Stewart  
wrote:



Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb
(from feature/spotlessPlugin).



On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:

FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the

Apache

git repo.

Is there a way to reformat a branch and then rebase on develop to

minimize

conflicts?

-Kirk


On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart 

wrote:
Fantastic, thanks for merging this in Mark.  For anyone with 
outstanding
work on branches made before this change, your life may be made 
easier

by

cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
Spotless) into your branch and then running ‘gradlew 
spotlessApply’ on

it

before attempting to merge into develop.

— Jared
On Oct 21, 2016, at 1:33 PM, Mark Bretl  
wrote:


Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does 
touch many

files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 

wrote:

Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl 

wrote:

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXDUnitTest.java

geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXWithVersionsDUnitTest.java

geode-core/src/test/java/org/apache/geode/internal/cache/execute/

PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 

wrote:

I just pulled and rebased onto develop, and force pushed into the

existing

pull request.  It should be clean to merge in now.

Thanks,
Jared

On Oct 21, 2016, at 11:57 AM, Mark Bretl 

wrote:

I believe there is enough consensus here to check this into

develop.
Jared, due to recent checkins into develop, can you update 
the pull

request

one more time? Trying to make this as clean as possible. I will

check

into

develop after the update, unless someone else gets to it first.

All, can we hold checkins on develop until the new formatter is

applied?

Thanks,

--Mark

On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 

wrote:

+1


On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the 
formatter,

then I

am

good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 


wrote:

+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <

jstew...@pivotal.io

wrote:


I have opened a pull request here <

https://github.com/apache/
incubator-geode/pull/268> to enable the Spotless plugin 
and

to

switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund 

wrote:

For reference TRAC #38741 was a bug with the summary

"EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <

jstew...@pivotal.io

wrote:

To give everyone an update, using the Google Java Style

eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java
even if you run it directly after spotlessApply. This 
needs

to

be

Re: Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

2016-10-21 Thread Bruce Schuchardt


That's a better comment.  You should replace the one you have with it.

Le 10/21/2016 à 4:03 PM, Hitesh Khamesra a écrit :
This is an automatically generated e-mail. To reply, visit: 
https://reviews.apache.org/r/53094/



On October 21st, 2016, 10:36 p.m. UTC, *Bruce Schuchardt* wrote:


geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java

<https://reviews.apache.org/r/53094/diff/1/?file=1543182#file1543182line1516>
(Diff revision 1)

public final boolean destroy(EntryEventImpl event,

1516

 owner.cancelExpiryTask(re,  event.getExpiryTask());

1516

 //we only want to cancel if concurrency-check is not 
enabled

I don't understand this comment. There doesn't seem to be a
check for concurrency-checks being enabled & you've removed
the check for a tombstone.

Right, I just added some more comment there. Basically regionEntry 
will be null if concurrency-check is enable. And in that case we 
cancel expiry from removeTomstone method.



//we only want to cancel if concurrency-check is not enabled 
//re(regionentry) will be null when concurrency-check is enable and 
removeTombstone method //will call cancelExpiryTask on regionEntry



- Hitesh


On October 21st, 2016, 6:41 p.m. UTC, Hitesh Khamesra wrote:

Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
Kohlmeyer.

By Hitesh Khamesra.

/Updated Oct. 21, 2016, 6:41 p.m./

*Repository: * geode


  Description

ExpirationManager tracks the regionEntry as reference to expiryTask. 
It assumes, it is unique for that key. But consistency check mechanism 
keeps that regionEntry around and that enforces re-use of that for new 
update. That introduces the race condition between expiry thread and 
user thread, it endup not scheduling the entry for expiration. As a 
fix, now "consistency check mechanism" unschedules the expiry task to 
avoid this race condition.



  Diffs

  * 
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
(e02c7e1)
  * geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
(d059aab)
  * 
geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
(0c20d32)
  * geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
(ac4c705)

View Diff <https://reviews.apache.org/r/53094/diff/>

Re: Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

2016-10-21 Thread Bruce Schuchardt


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




geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java 
(line 1516)
<https://reviews.apache.org/r/53094/#comment223014>

I don't understand this comment.  There doesn't seem to be a check for 
concurrency-checks being enabled & you've removed the check for a tombstone.


- Bruce Schuchardt


On Oct. 21, 2016, 6:41 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53094/
> ---
> 
> (Updated Oct. 21, 2016, 6:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ExpirationManager tracks the regionEntry as reference to expiryTask. It 
> assumes, it is unique for that key. But consistency check mechanism keeps 
> that regionEntry around and that enforces re-use of that for new update. That 
> introduces the race condition between expiry thread and user thread, it endup 
> not scheduling the entry for expiration. As a fix, now "consistency check 
> mechanism" unschedules the expiry task to avoid this race condition.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  e02c7e1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 
> d059aab 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java 
> 0c20d32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> ac4c705 
> 
> Diff: https://reviews.apache.org/r/53094/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 53092: GEODE-2011 Client clears pdx registry needs synchronization

2016-10-21 Thread Bruce Schuchardt


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



is it possible to unit test this change?  
PdxClientServerDUnitTest.testNonPersistentServerRestartAutoSerializer ran into 
the problem but seems to be marked Flaky.  Can you modify that test to always 
hit the problem and remove the Flaky label?

- Bruce Schuchardt


On Oct. 21, 2016, 6:52 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53092/
> ---
> 
> (Updated Oct. 21, 2016, 6:52 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> We clear pdx registry in client when we re-connect to cluster. But during 
> that time other thread may end up using old registry while other thread is 
> clearing this. Thus we need to synchronize that. In current code it happens 
> through listener events and I modified that notification to synchronize this.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
>  3f3d725 
> 
> Diff: https://reviews.apache.org/r/53092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Coding practices/standards

2016-10-21 Thread Bruce Schuchardt

On Oct 12, 2016, at 2:27 PM, Darrel Schneider <

dschnei...@pivotal.io

wrote:
I like Dan's idea of catching formatting issues earlier 
but

I

think

adding

5-10 minutes to "build" would be too much. Currently when

I'm

trying

to do
a quick build I use -xjavadoc. I'd probably do the same 
for

this

target if

it was part of build until I'm ready to do a precheckin.

Mark, wouldn't running the formatter on all our java files

and

checking

them in get these issues down to 0?

On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <

ukohlme...@pivotal.io

wrote:


+1 - adding checkstyle to precheckin.

If the developer uses the provided templates ( eclipse +

intellij)

then

most of the formatting issues should be handled before

precheckin.

Also, if

a developer has a questionable coding style, that should

lessen

as

that
developer will have resolve the issues before being 
able to

commit.

I also believe that this should not be an overbearing and

intrusive

process.

--Udo



On 13/10/16 6:36 am, Mark Bretl wrote:


Dan,

There is some extra amount of time, 5-10 minutes 
extra for

the

entire
project (depending on your CPU). I think the real 
issue to

adding

it

to

the
precheckin target and have it be 'effective' is it needs

to

run

successfully, otherwise it would turn into noise most of

the

time

I

think.
We need to get the issues down to 0 or manage to set 
a new

baseline

(not

the best idea), which is a lot of work, to make it run

successfully.

Right
now, if you run the target, it will fail every time 
since

there

outstanding
issues in the code and very hard to tell what issues 
were

introduced.


--Mark

On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith <

dsm...@pivotal.io>

wrote:
Seems like it should run as part of the build target. 
The

only

reason to

make it part of precheckin is if it takes a long time,

otherwise

people

should get fast feedback they need to change their code

before

they

push.

-Dan

On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <

jstew...@pivotal.io>

wrote:

+1 to running during the precheckin target as well as

Travis

CI

On Oct 12, 2016 11:20 AM, "Darrel Schneider" <

dschnei...@pivotal.io>

wrote:

If Travis CI is only run on pull requests then that is

not

enough

because
committers do not submit pull requests. Having it run

during

the

gradle
build or precheckin target is also needed. In 
addition

to

that

I

also

wanted PRs to be checked.


On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart <

jstew...@pivotal.io>

wrote:

It would certainly be necessary to make sure that the

code

style

to

be
enforced is sensible, e.g. doe not use wildcard 
imports.

We

would

also

want to make one large commit to format all existing

code

before

turning
this on.

Mark - Thank you for the information about the

existing

setup.

Mark, Darrel, Kevin - Given all of your comments, I

think

it

might

make

more sense to add the flag to enable it in Travis CI

rather

than

as

part
of  the build.  This way your build pass 
regardless of

whitespace,

but

the
CI job would fail on PRs if they did not adhere 
to the

standard

formatting.


Anthony - It doesn’t seem to me that turning this on

would

have

the

effect


of combining reformatting commits and logic changes.

Rather,

since

all

code would already be formatted, there would no longer

be

any

reformatting

commits except for single large commits when the 
code

style

file

was

updated.

On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
bschucha...@pivotal.io

wrote:

I like the idea of doing this but I don't think

Checkstyle

should

be

enabled until all of the code is reformatted.

Also, last time I checked there was still a problem

with

the

IntelliJ

auto-format settings.  It still used wildcard imports,

which I

believe

we


don't allow.  I've manually changed my settings in

Editor->Code

Style->Java


to "Use single class import" to correct that

problem.  I

couldn't see

how
to get Gradle to do it.

Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :

Source code with a consistent look-and-feel 
makes it

easier

for

people

to join the project community and contribute.
Let’s continue to keep reformatting commits 
separate

from

logic

changes—otherwise it’s too hard to review.
Anthony

On Oct 12, 2016, at 10:06 AM, Dan Smith <

dsm...@pivotal.io>

wrote:

+1

This might be a good time to reformat the code

since

I

don't

think

there
are too many long lived feature branches 
outstanding.

-Dan

On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart <


jstew...@pivotal.io

wrote:

I would like to advocate for adding a Checkstyle <

http://checkstyle

.


sourceforge.net/> or Spotless <

https://github.com/diffplug/

spotless

gradle task to our build process to ensure that all

code

checked

in

meets

the formatting standards described on the wiki <

https://cwiki.apache.org/

confluence/display/GEODE/Code+Style+Guide> (and in

the

intellij/eclipse

formatter xml files in our repository). This will

alleviate

difficulties

reviewing code when whitespace or formatting has

changed

since

all

code

checked in will already comply with standards.




--

~/William

Re: Removal of nonSingleHopCount stat from client

2016-10-19 Thread Bruce Schuchardt


+1 for removing it

It's redundant, we've rarely used client statistics and I hear that 
people generally have stats turned off in clients anyway.


Le 10/19/2016 à 12:55 PM, Udo Kohlmeyer a écrit :

Hi there Guys,

I've created https://issues.apache.org/jira/browse/GEODE-2017 to track 
the removal of the nonSingleHopCount stat from the client, as it is 
potentially a redundant stat.


From the implementation it only increments on "singleHop" enabled 
pools when an operation requires more than one hop to complete. This 
exact same metric is tracked in "metaDataRefreshCount", which tracks 
the amount of meta data refreshes on the client. Which is 
automatically  refreshed when an operation requires more than one hop.


Does anyone have any objection in the removal of this stat?

--Udo

Review Request 53034: GEODE-1927 backward compatibility support

2016-10-19 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


Repository: geode


Description
---

adding a new unit-test category for backward-compatibility tests


Diffs
-

  
geode-junit/src/main/java/org/apache/geode/test/junit/categories/BackwardCompatibilityTest.java
 PRE-CREATION 
  gradle/test.gradle 5b895ba07a37740165c2dca45179493615d613f5 

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


Testing
---


Thanks,

Bruce Schuchardt

Review Request 53001: GEODE-1927: more protection from seeing com.gemstone.gemfire packaged objects

2016-10-18 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra, Udo Kohlmeyer, and Dan Smith.


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


Repository: geode


Description
---

DeadlockDetector can read an archive of dependencies across the distributed 
system.  This adds a small ObjectInputStream modification to its method that 
reads these archives to let it handle those created before the package rename.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/deadlock/DeadlockDetector.java
 2c70418e53b25edd3ae7cfe801e8f4b92ca1c3ec 

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


Testing
---

manual testing with an old archive


Thanks,

Bruce Schuchardt

Re: Review Request 52956: GEODE-2011: add FlakyTest category to testNonPersistentServerRestartAutoSerializer

2016-10-17 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Oct. 17, 2016, 8:03 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52956/
> ---
> 
> (Updated Oct. 17, 2016, 8:03 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2011
> https://issues.apache.org/jira/browse/GEODE-2011
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2011: add FlakyTest category to 
> testNonPersistentServerRestartAutoSerializer
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 1afb1ad 
> 
> Diff: https://reviews.apache.org/r/52956/diff/
> 
> 
> Testing
> ---
> 
> build
> PdxClientServerDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52933: GEODE-1874: setNextNeighbor method allocates a HashSet on every p2p message received

2016-10-17 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Oct. 17, 2016, 5:18 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52933/
> ---
> 
> (Updated Oct. 17, 2016, 5:18 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Hitesh Khamesra.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Amended the logic in setNextNeighbor to not recreate HashSet every time. Also 
> amended logic in contactedBy to only call setNextNeighbor in a "state change" 
> rather than everytime the contactedBy method is called
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
>  aafb498 
> 
> Diff: https://reviews.apache.org/r/52933/diff/
> 
> 
> Testing
> ---
> 
> precheckin - Done
> comms regression - in progress
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: Review Request 52896: GEODE-706 Fixed race condition between expiry thread and put thread.

2016-10-17 Thread Bruce Schuchardt


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




geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java (line 
8721)
<https://reviews.apache.org/r/52896/#comment222071>

It would be a lot better to have two different methods, one that takes an 
expiryTask parameter and another that doesn't.


- Bruce Schuchardt


On Oct. 14, 2016, 9:05 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52896/
> ---
> 
> (Updated Oct. 14, 2016, 9:05 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There was possibility that expiry thread destroying the entry and
> another thread doing update on same key. In this case expiry thread
> cancel's existing task and update thread adds expiry task. But this
> tasks are refer by regionEntry, which is same for both the threads.
> So in this case if expiry thread cancel's task after update thread
> then that entry will never expire.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  5861e9a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 
> 6a964c0 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java 
> 816f32f 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> a6951de 
> 
> Diff: https://reviews.apache.org/r/52896/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Coding practices/standards

2016-10-14 Thread Bruce Schuchardt

to format all existing code

before

turning
this on.

Mark - Thank you for the information about the existing

setup.

Mark, Darrel, Kevin - Given all of your comments, I think

it

might

make

more sense to add the flag to enable it in Travis CI rather

than

as

part
of  the build.  This way your build pass regardless of

whitespace,

but

the

CI job would fail on PRs if they did not adhere to the

standard

formatting.


Anthony - It doesn’t seem to me that turning this on would

have

the

effect


of combining reformatting commits and logic changes.

Rather,

since

all

code would already be formatted, there would no longer be

any

reformatting


commits except for single large commits when the code

style

file

was

updated.

On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
bschucha...@pivotal.io

wrote:

I like the idea of doing this but I don't think

Checkstyle

should

be

enabled until all of the code is reformatted.

Also, last time I checked there was still a problem with

the

IntelliJ

auto-format settings.  It still used wildcard imports,

which I

believe

we


don't allow.  I've manually changed my settings in

Editor->Code

Style->Java


to "Use single class import" to correct that problem.  I

couldn't see

how
to get Gradle to do it.

Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :


Source code with a consistent look-and-feel makes it

easier

for

people

to join the project community and contribute.

Let’s continue to keep reformatting commits separate from

logic

changes—otherwise it’s too hard to review.
Anthony

On Oct 12, 2016, at 10:06 AM, Dan Smith <

dsm...@pivotal.io>

wrote:

+1

This might be a good time to reformat the code since I

don't

think

there

are too many long lived feature branches outstanding.

-Dan

On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart <


jstew...@pivotal.io

wrote:

I would like to advocate for adding a Checkstyle <

http://checkstyle

.


sourceforge.net/> or Spotless <

https://github.com/diffplug/

spotless

gradle task to our build process to ensure that all code

checked

in

meets

the formatting standards described on the wiki <

https://cwiki.apache.org/

confluence/display/GEODE/Code+Style+Guide> (and in the

intellij/eclipse

formatter xml files in our repository). This will

alleviate

difficulties

reviewing code when whitespace or formatting has changed

since

all

code

checked in will already comply with standards.




--

~/William

Re: Review Request 52836: GEODE-2000 ClientMemberShipListener at client should return hostname on which cacheserver is listening(i.e. server-bind-address)

2016-10-13 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Oct. 13, 2016, 4:26 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52836/
> ---
> 
> (Updated Oct. 13, 2016, 4:26 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ClientMembershipListener's callback method has clientMembershipEvent. This 
> returns distributedmember object. This object should return 
> server-bind-address at client.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
>  775fa24 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  d63dfa0 
> 
> Diff: https://reviews.apache.org/r/52836/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Coding practices/standards

2016-10-12 Thread Bruce Schuchardt

I like the idea of doing this but I don't think Checkstyle should be 
enabled until all of the code is reformatted.


Also, last time I checked there was still a problem with the IntelliJ 
auto-format settings.  It still used wildcard imports, which I believe 
we don't allow.  I've manually changed my settings in Editor->Code 
Style->Java to "Use single class import" to correct that problem.  I 
couldn't see how to get Gradle to do it.



Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :

Source code with a consistent look-and-feel makes it easier for people to join 
the project community and contribute.

Let’s continue to keep reformatting commits separate from logic 
changes—otherwise it’s too hard to review.

Anthony


On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:

+1

This might be a good time to reformat the code since I don't think there
are too many long lived feature branches outstanding.

-Dan

On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart  wrote:


I would like to advocate for adding a Checkstyle  or Spotless 
gradle task to our build process to ensure that all code checked in meets
the formatting standards described on the wiki  (and in the intellij/eclipse
formatter xml files in our repository). This will alleviate difficulties
reviewing code when whitespace or formatting has changed since all code
checked in will already comply with standards.

Re: Deprecate DynamicRegionFactory

2016-10-12 Thread Bruce Schuchardt


+1

Le 10/12/2016 à 6:36 AM, Jens Deppe a écrit :

+1 to remove it

On Tue, Oct 11, 2016 at 9:33 PM, John Blum  wrote:


+1 - forget deprecated; delete it.

On Tue, Oct 11, 2016 at 8:22 PM, Udo Kohlmeyer 
wrote:


+1 - I see no real benefit for this.



On 12/10/16 1:10 pm, William Markito wrote:


+1 -  We always discouraged people using dynamic regions

On Tue, Oct 11, 2016 at 4:33 PM, Dan Smith  wrote:

I'd like to reopen the discussion about deprecating

DynamicRegionFactory. This is some old crufty code I don't think we
want anyone to use. I see we had a ticket to deprecated it
(GEODE-388), but we chose not to do it because we don't have an
alternative API to create regions from the client (GEODE-215).

The thing is, there is a pretty easy workaround to use functions to
create regions from the client. Personally I'd recommend that over
DynamicRegionFactory. Can we just deprecate DynamicRegionFactory now
and implement GEODE-215 later?

-Dan






--
-John
503-504-8657
john.blum10101 (skype)

Re: Review Request 52641: GEODE-1914 removed old versions of dtds

2016-10-07 Thread Bruce Schuchardt

  This should be 7.0//EN



geode-core/src/test/resources/org/apache/geode/internal/cache/faultyDiskXMLsForTesting/mixed_diskstore_diskdir.xml
 (line 19)
<https://reviews.apache.org/r/52641/#comment220396>

This should be 7.0//EN



geode-core/src/test/resources/org/apache/geode/internal/cache/faultyDiskXMLsForTesting/mixed_diskstore_diskwriteattrs.xml
 (line 19)
<https://reviews.apache.org/r/52641/#comment220397>

This should be 7.0//EN



geode-core/src/test/resources/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.testDTDFallbackWithNonEnglishLocal.cache.xml
 (line 20)
<https://reviews.apache.org/r/52641/#comment220398>

This should be 7.0//EN



geode-cq/src/test/resources/org/apache/geode/internal/cache/tier/sockets/durablecq-client-cache.xml
 (line 19)
<https://reviews.apache.org/r/52641/#comment220400>

This should be 7.0//EN



geode-cq/src/test/resources/org/apache/geode/internal/cache/tier/sockets/durablecq-server-cache.xml
 (line 19)
<https://reviews.apache.org/r/52641/#comment220401>

This should be 7.0//EN



geode-spark-connector/geode-spark-connector/src/it/resources/test-regions.xml 
(line 20)
<https://reviews.apache.org/r/52641/#comment220402>

This should be 7.0//EN



geode-spark-connector/geode-spark-connector/src/it/resources/test-retrieve-regions.xml
 (line 20)
<https://reviews.apache.org/r/52641/#comment220403>

This should be 7.0//EN


- Bruce Schuchardt


On Oct. 7, 2016, 5:12 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52641/
> ---
> 
> (Updated Oct. 7, 2016, 5:12 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1914 Removed old dtds from geode source code(kept 7.0 and above)
> 
> Deteted old tests and updated test xmls to point 7.0 dtd
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/resources/javadoc-images/example-client-cache.xml 
> bddeb9f 
>   geode-core/src/main/resources/org/apache/geode/admin/doc-files/ds4_0.dtd 
> 6566af6 
>   geode-core/src/main/resources/org/apache/geode/admin/doc-files/ds5_0.dtd 
> d56fd4d 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache3_0.dtd 
> 44d42c2 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache4_0.dtd 
> d91d50e 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache4_1.dtd 
> d64968a 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache5_0.dtd 
> 34ca2a9 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache5_1.dtd 
> 4a93047 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache5_5.dtd 
> 090f7f9 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache5_7.dtd 
> 95e54fd 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache5_8.dtd 
> dc2ae73 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache6_0.dtd 
> 82bdc11 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache6_1.dtd 
> 3215fbc 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache6_5.dtd 
> 9798748 
>   geode-core/src/main/resources/org/apache/geode/cache/doc-files/cache6_6.dtd 
> 0a2c2f2 
>   geode-core/src/test/java/org/apache/geode/cache/RegionFactoryJUnitTest.java 
> 9f31eb1 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml30DUnitTest.java 
> 187be8a 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml40DUnitTest.java 
> 952efb2 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml41DUnitTest.java 
> ae1a98d 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml45DUnitTest.java 
> 7c10524 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml51DUnitTest.java 
> 30925e5 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml55DUnitTest.java 
> fd52580 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml57DUnitTest.java 
> 833093d 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml58DUnitTest.java 
> 4040b15 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml60DUnitTest.java 
> 35026ba 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml61DUnitTest.java 
> ea15c18 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml65DUnitTest.java 
> a12737b 
>   geode-core/src/test/java/org/apache/geode/cache30/CacheXml66DUnitTest.java 
> ededb8d 
>   
> geode-core/src/test/java/org/apache/geode/internal/JarClassLoaderJUnitTest.java
>

Re: backward compatibility testing against GemFire 8.x WAN/clients

2016-10-06 Thread Bruce Schuchardt

Thanks everyone.  I'm going to keep the framework out of Geode, at least 
for now.   We can pick up the discussion again when we have a 1.0.0 
Geode release to test against.


Le 10/6/2016 à 1:38 PM, Udo Kohlmeyer a écrit :
I believe that a backwards compatibility framework is required. Even 
if just to confirm the compatibility between supported Geode versions.


If this framework were to be used/extended by any user of the Geode 
ecosystem, then it becomes their responsibility to maintain that 
extension.


--Udo


On 7/10/2016 3:31 AM, Mark Bretl wrote:

-1 for this

I do understand the intent and reason behind this, however, in 
addition to
the reasons Anthony provided, I do not believe this community should 
carry
the burden of testing compatibility with proprietary/commercial 
software.
Even though Geode has its history in GemFire, I can only see the 
community
supporting official Geode releases. It would set a bad precedent to 
allow

for any part of the Geode code ( source or test ) to depend on any
commercial code.

If any company wants to make sure their software works with Geode, it is
their responsibility.

My $.02,

--Mark

On Thu, Oct 6, 2016 at 9:12 AM, Bruce Schuchardt 


wrote:


Yes, dist.gemstone.com.


Le 10/6/2016 à 9:08 AM, Dick Cavender a écrit :


Only on initial setup/access to repo.pivotal.io is the EULA acceptance
required. After that you should be able to pull from any repo there 
without
interaction but you will need to provide the creds as part of the 
pull.


Are you instead using dist.gemstone.com for the older jars which is 
the

original repo that lives in the spring s3 repo


On 10/6/2016 7:48 AM, Bruce Schuchardt wrote:


Thanks Anthony.

I thought there was some interest in supporting old GemFire 
clients and
WAN.  Is there no way to download it without a login/EULA?  I'm 
currently
using a different repo but maybe that's only available in 
Pivotal's network.


As far as M3 goes, do you think there would be any value in testing
against it?  I don't want to introduce new tests unless they are 
helpful.


Le 10/5/2016 à 5:31 PM, Anthony Baker a écrit :

Given that the official Pivotal maven repo [1] is locked behind a 
login
and a EULA I think this might not work so well.  The license 
compatibility

issues would need to be explored as well.

How would you feel about testing backwards compatibility against
1.0.0-incubating.M3?

Anthony

[1] http://commercial-repo.pivotal.io

On Oct 5, 2016, at 4:17 PM, Bruce Schuchardt 


wrote:

We now have a backward-compatibility module but it's not well 
tested.

I'd
like to add tests to this module that run against GemFire jars
downloaded
from the Pivotal maven repository.  I've already implemented the
framework
and some smoke tests but want to know how people feel about the 
tests
downloading something proprietary to test against and whether 
failures

would be something the community cares about.

If we don't want this I'll keep it out of the repo until we have a
1.0.0
release to test against.

Re: backward compatibility testing against GemFire 8.x WAN/clients

2016-10-06 Thread Bruce Schuchardt


Yes, dist.gemstone.com.

Le 10/6/2016 à 9:08 AM, Dick Cavender a écrit :
Only on initial setup/access to repo.pivotal.io is the EULA acceptance 
required. After that you should be able to pull from any repo there 
without interaction but you will need to provide the creds as part of 
the pull.


Are you instead using dist.gemstone.com for the older jars which is 
the original repo that lives in the spring s3 repo



On 10/6/2016 7:48 AM, Bruce Schuchardt wrote:

Thanks Anthony.

I thought there was some interest in supporting old GemFire clients 
and WAN.  Is there no way to download it without a login/EULA?  I'm 
currently using a different repo but maybe that's only available in 
Pivotal's network.


As far as M3 goes, do you think there would be any value in testing 
against it?  I don't want to introduce new tests unless they are 
helpful.


Le 10/5/2016 à 5:31 PM, Anthony Baker a écrit :
Given that the official Pivotal maven repo [1] is locked behind a 
login and a EULA I think this might not work so well.  The license 
compatibility issues would need to be explored as well.


How would you feel about testing backwards compatibility against 
1.0.0-incubating.M3?


Anthony

[1] http://commercial-repo.pivotal.io

On Oct 5, 2016, at 4:17 PM, Bruce Schuchardt 
 wrote:


We now have a backward-compatibility module but it's not well 
tested.  I'd
like to add tests to this module that run against GemFire jars 
downloaded
from the Pivotal maven repository.  I've already implemented the 
framework

and some smoke tests but want to know how people feel about the tests
downloading something proprietary to test against and whether failures
would be something the community cares about.

If we don't want this I'll keep it out of the repo until we have a 
1.0.0

release to test against.

Re: backward compatibility testing against GemFire 8.x WAN/clients

2016-10-06 Thread Bruce Schuchardt


Thanks Anthony.

I thought there was some interest in supporting old GemFire clients and 
WAN.  Is there no way to download it without a login/EULA?  I'm 
currently using a different repo but maybe that's only available in 
Pivotal's network.


As far as M3 goes, do you think there would be any value in testing 
against it?  I don't want to introduce new tests unless they are helpful.


Le 10/5/2016 à 5:31 PM, Anthony Baker a écrit :

Given that the official Pivotal maven repo [1] is locked behind a login and a 
EULA I think this might not work so well.  The license compatibility issues 
would need to be explored as well.

How would you feel about testing backwards compatibility against 
1.0.0-incubating.M3?

Anthony

[1] http://commercial-repo.pivotal.io


On Oct 5, 2016, at 4:17 PM, Bruce Schuchardt  wrote:

We now have a backward-compatibility module but it's not well tested.  I'd
like to add tests to this module that run against GemFire jars downloaded
from the Pivotal maven repository.  I've already implemented the framework
and some smoke tests but want to know how people feel about the tests
downloading something proprietary to test against and whether failures
would be something the community cares about.

If we don't want this I'll keep it out of the repo until we have a 1.0.0
release to test against.

backward compatibility testing against GemFire 8.x WAN/clients

2016-10-05 Thread Bruce Schuchardt

We now have a backward-compatibility module but it's not well tested.  I'd
like to add tests to this module that run against GemFire jars downloaded
from the Pivotal maven repository.  I've already implemented the framework
and some smoke tests but want to know how people feel about the tests
downloading something proprietary to test against and whether failures
would be something the community cares about.

If we don't want this I'll keep it out of the repo until we have a 1.0.0
release to test against.

Re: Review Request 52271: GEODE-1938: Big Snapshot File Read Exception via SnapshotReader API

2016-10-05 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Oct. 5, 2016, 12:36 a.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52271/
> ---
> 
> (Updated Oct. 5, 2016, 12:36 a.m.)
> 
> 
> Review request for geode, Anthony Baker, Bruce Schuchardt, and Hitesh 
> Khamesra.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When reading a GemFire/Geode snapshot from a client the ClientTypeRegistry 
> will fail due code that never allowed for a success case. Even if the type 
> registry was successfull the client would "fail" due to incorrect failure 
> logic.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/pdx/internal/ClientTypeRegistration.java
>  8957e7f 
> 
> Diff: https://reviews.apache.org/r/52271/diff/
> 
> 
> Testing
> ---
> 
> precheckin - Done
> regression - snapshot/snapshot.bt - Done
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Review Request 52524: GEODE-1927 backward compatibility support

2016-10-04 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


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


Repository: geode


Description
---

This fixes a problem in BaseCommand encountered during integration testing.  
The integration tests aren't checked in yet due to some Gradle issues I need to 
work out dealing with downloading old versions of GemFire to use in 
distributedTest runs.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
 743632c6173eae5192b8805601c6b08d5e267f5c 
  geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
b073d87df37617418336a77ad3f0df240e950417 
  
geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
 df5e0e095874c181fa72ec5ec7629aa7cea7f52e 
  
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java
 b8770b94087ed5bb7995e1f7076ec78fd0ddfc29 

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


Testing
---

Integration tests (pending checkin)


Thanks,

Bruce Schuchardt

Re: Review Request 52363: GEODE-1947: Renaming SSL_HTTP_SERVICE_REQUIRE_AUTHENTICATION to SSL_WEB_SERVICE_REQUIRE_AUTHENTICATION

2016-09-28 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Sept. 28, 2016, 7:16 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52363/
> ---
> 
> (Updated Sept. 28, 2016, 7:16 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Hitesh Khamesra.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is a property that was missed due to the HTTP -> WEB rename from 
> GEODE-420.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsWithSSLDUnitTest.java
>  cd6b590 
>   
> geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
>  66b1472 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
>  31fa4f6 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
>  9da08da 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
>  4d3d751 
>   
> geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
>  4261248 
>   geode-core/src/main/java/org/apache/geode/management/GemFireProperties.java 
> 2b2c1a6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
>  92d2f62 
>   
> geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
>  8b760df 
> 
> Diff: https://reviews.apache.org/r/52363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: security properties in the cluster config

2016-09-28 Thread Bruce Schuchardt

Okay, so that won't work for any communications settings.  They all need to
be in place long before a Cache is built and cluster configuration
information is available.

On Tue, Sep 27, 2016 at 4:56 PM, Jinmei Liao  wrote:

> By "putting a property in the cluster config", I meant this property will
> be passed down to the joining members if enable-cluster-configuration is
> set true in the locator and use-cluster-configuration is true in the
> member. This is what we are doing for security-manager and
> security-post-processor. You still specify those properties in your normal
> properties file.
>
> On Tue, Sep 27, 2016 at 4:47 PM, Bruce Schuchardt 
> wrote:
>
> > Isn't cluster-configuration optional?  If I restart a locator and it
> > doesn't have a persistent cluster configuration it is going to need to
> know
> > the DH algorithm in order to communicate with other members and join the
> > system.
> >
> > If it's going to stop being optional then I think we could conceivably
> > move this setting to the cluster-configuration.  It might be odd to have
> > that in cluster-config and SSL settings in the properties file though.
> > Both concern secure communications.
> >
> >
> > Le 9/27/2016 à 4:30 PM, Jinmei Liao a écrit :
> >
> >> for "security-udp-dhalgo" property, If "Each member needs to define this
> >> property with the same algorithm", it would make sense to put that in
> the
> >> cluster configuration.
> >>
> >> On Tue, Sep 27, 2016 at 3:09 PM, Bruce Schuchardt <
> bschucha...@pivotal.io
> >> >
> >> wrote:
> >>
> >> security-udp-dhalgo is new and is described here:
> >>> https://cwiki.apache.org/confluence/display/GEODE/Secure+
> >>> UDP+Communication+in+Geode
> >>>
> >>>
> >>> Le 9/26/2016 à 11:23 AM, Swapnil Bawaskar a écrit :
> >>>
> >>> Hi John,
> >>>> security-manager and security-post-processor are discussed here:
> >>>> https://cwiki.apache.org/confluence/display/GEODE/Geode+
> >>>> Integrated+Security
> >>>>
> >>>> On Mon, Sep 26, 2016 at 11:01 AM, Joey McAllister <
> >>>> jmcallis...@pivotal.io
> >>>> wrote:
> >>>>
> >>>> Hi John,
> >>>>
> >>>>> They are documented in the docs dev branch and will be published with
> >>>>> the
> >>>>> next Geode release. Also, we're scheduled to donate the docs code to
> >>>>> the
> >>>>> project later this week, so you'll be able to see the work in dev.
> >>>>>
> >>>>> Best,
> >>>>> Joey
> >>>>>
> >>>>> On Mon, Sep 26, 2016 at 10:41 AM John Blum  wrote:
> >>>>>
> >>>>> Jinmei-
> >>>>>
> >>>>>> Where are the following security-* properties documented?
> >>>>>>
> >>>>>> security-udp-dhalgo
> >>>>>>
> >>>>>> security-manager
> >>>>>>
> >>>>>> security-post-processor
> >>>>>>
> >>>>>> They certainly are not documented in the (Geode) User Docs, here
> >>>>>> <
> >>>>>> http://geode.docs.pivotal.io/docs/reference/topics/gemfire_
> >>>>>>
> >>>>>> properties.html
> >>>>>
> >>>>>[1].
> >>>>>>
> >>>>>> Thanks!
> >>>>>> John
> >>>>>>
> >>>>>> [1]
> >>>>>> http://geode.docs.pivotal.io/docs/reference/topics/gemfire_
> >>>>>>
> >>>>>> properties.html
> >>>>>
> >>>>>
> >>>>>> On Mon, Sep 26, 2016 at 8:42 AM, Jinmei Liao 
> >>>>>> wrote:
> >>>>>>
> >>>>>> Actually, I looked into the the config settings, these are the list
> of
> >>>>>>
> >>>>>>> settings that begin with security-. SSL settings are not there. The
> >>>>>>> security-client-* and security-peer-* are deprecated, so they don't
> >>>>>>>
> >>>>>>> need
> >>>>>> to
> >>>>>>
> >>>>>> be in the cluster config. W

Re: Review Request 52172: GEODE-1927: add support for old GemFire remote sites (WAN)

2016-09-28 Thread Bruce Schuchardt

ATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/GemFireException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/cache/execute/EmtpyRegionFunctionException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/cache/execute/FunctionException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/cache/execute/FunctionInvocationTargetException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/resources/META-INF/services/org.apache.geode.internal.cache.CacheService
 PRE-CREATION 
  
geode-old-client-support/src/test/java/com/gemstone/gemfire/ClientDataSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/com/gemstone/gemfire/ClientPDXSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/com/gemstone/gemfire/ClientSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/ClientDataSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/ClientPDXSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/ClientSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
 PRE-CREATION 
  settings.gradle 95c15f2855d25938e551371b6f93192b7b953fba 

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


Testing
---

precheckin.  New tests are being developed for gemfire<->geode WAN and 
client/server interactions but these won't be part of the Geode repo.


Thanks,

Bruce Schuchardt

Re: security properties in the cluster config

2016-09-27 Thread Bruce Schuchardt

Isn't cluster-configuration optional?  If I restart a locator and it 
doesn't have a persistent cluster configuration it is going to need to 
know the DH algorithm in order to communicate with other members and 
join the system.


If it's going to stop being optional then I think we could conceivably 
move this setting to the cluster-configuration.  It might be odd to have 
that in cluster-config and SSL settings in the properties file though.  
Both concern secure communications.


Le 9/27/2016 à 4:30 PM, Jinmei Liao a écrit :

for "security-udp-dhalgo" property, If "Each member needs to define this
property with the same algorithm", it would make sense to put that in the
cluster configuration.

On Tue, Sep 27, 2016 at 3:09 PM, Bruce Schuchardt 
wrote:


security-udp-dhalgo is new and is described here:
https://cwiki.apache.org/confluence/display/GEODE/Secure+
UDP+Communication+in+Geode


Le 9/26/2016 à 11:23 AM, Swapnil Bawaskar a écrit :


Hi John,
security-manager and security-post-processor are discussed here:
https://cwiki.apache.org/confluence/display/GEODE/Geode+
Integrated+Security

On Mon, Sep 26, 2016 at 11:01 AM, Joey McAllister 
They are documented in the docs dev branch and will be published with the
next Geode release. Also, we're scheduled to donate the docs code to the
project later this week, so you'll be able to see the work in dev.

Best,
Joey

On Mon, Sep 26, 2016 at 10:41 AM John Blum  wrote:

Jinmei-

Where are the following security-* properties documented?

security-udp-dhalgo

security-manager

security-post-processor

They certainly are not documented in the (Geode) User Docs, here
<
http://geode.docs.pivotal.io/docs/reference/topics/gemfire_


properties.html


   [1].

Thanks!
John

[1]
http://geode.docs.pivotal.io/docs/reference/topics/gemfire_


properties.html



On Mon, Sep 26, 2016 at 8:42 AM, Jinmei Liao  wrote:

Actually, I looked into the the config settings, these are the list of

settings that begin with security-. SSL settings are not there. The
security-client-* and security-peer-* are deprecated, so they don't


need
to


be in the cluster config. What about the udp-dhalgo and log-file and
log-level? Does it hurt to put them in the cluster-config?

"security-client-authenticator";

"security-client-accessor";

"security-client-accessor-pp";

"security-client-auth-init";

"security-client-dhalgo";

"security-peer-auth-init";

"security-peer-authenticator";

"security-peer-verifymember-timeout";

"security-udp-dhalgo";

"security-log-file";

"security-log-level";

"security-manager";

"security-post-processor";







On Fri, Sep 23, 2016 at 12:41 PM, Bruce Schuchardt <


bschucha...@pivotal.io


wrote:

SSL settings and the new UDP dhAlgo setting can't be in the cluster

config.  The cluster config is received over TCP/IP so you would have


to
use unsecured information to retrieve the settings, and you'd have to
do
it


before the cache is created.

Does the security-manager have any role to play prior to the cache


being
created?   For instance, is it involved in authenticating the receipt
of
a


new membership view or a join request in GMSAuthenticator?  If so you


can't


store it in the cluster config, which is only retrieved later on


during

cache creation.



Le 9/23/2016 à 11:57 AM, Michael Stolz a écrit :

I am in favor of keeping the SSL thoughts separate from the RBAC
thoughts,
but I don't see any reason they couldn't share the same repository.

That said though, does putting it all into the Cluster Configuration
Manager (CCM) make it so that you can only have security if you are


using
CCM for configuration?


--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771

On Fri, Sep 23, 2016 at 1:48 PM, Jinmei Liao 


wrote:

Hi, All,

I am working on this ticket:
https://issues.apache.org/jira/browse/GEODE-1659. Basically,


currently,

any

member(locator or server) needs to specify its own security-manager


in

order to protect its data which could leads to misconfiguration and

data

leak. So we would like to put it into the cluster configuration so

any

member who wants to join the cluster will need to apply the same

security

measures.

Now Here is my question, should we only put the "security-manager"


and

"security-post-processor" in the cluster config or any "security-*"

settings, which include SSL settings as well.

Thanks!

--
Cheers

Jinmei




--
Cheers

Jinmei



--
-John
503-504-8657
john.blum10101 (skype)

Re: Review Request 52172: GEODE-1927: add support for old GemFire remote sites (WAN)

2016-09-27 Thread Bruce Schuchardt



> On Sept. 22, 2016, 9:07 p.m., Dan Smith wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java,
> >  line 188
> > <https://reviews.apache.org/r/52172/diff/1/?file=1508461#file1508461line188>
> >
> > What is this for?
> 
> Bruce Schuchardt wrote:
> This will be used for backward-compatibility testing.  It will point to 
> the test output directory so we can find builds of old clients.
> 
> Dan Smith wrote:
> Long term, I think we want to separate out the dunit framework from geode 
> so it can be used generically for lots of projects. If possible, we should 
> avoid having a bunch of special case code for specific geode tests in the 
> famework. Rather we should focus on having a generic, extensible framework. 
> Is it possible to pass this test specific property to the child VMs as part 
> of the test itself?

The test using this will be migrated to the framework the Jens is working on.  
We can remove the JTESTS property at that time.


- Bruce


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


On Sept. 27, 2016, 11:18 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52172/
> ---
> 
> (Updated Sept. 27, 2016, 11:18 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1927
> https://issues.apache.org/jira/browse/GEODE-1927
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This adds a check in InternalDataSerializer for com.gemstone.gemfire packages 
> and transforms them to org.apache.geode.
> 
> I've also added a hook for translating org.apache.geode exceptions into 
> com.gemstone.gemfire exceptions.  We've decided to keep com.gemstone.gemfire 
> exceptions out of the Geode repository to avoid the confusion it would cause 
> to have an org.apache.geode implementation throw com.gemstone.gemfire 
> exceptions.
> 
> The latter change required changes to some tests using mocks to represent 
> ServerConnections.  These mocks were returning null from getClientVersion(), 
> causing NPEs in client/server code.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle a83b7a97016836b70a44d80c6a2221f4b4c8a5d9 
>   geode-core/src/main/java/org/apache/geode/CancelException.java 
> 94fd8b556dc04466b7fbae78bc6aaa48d3603c5d 
>   geode-core/src/main/java/org/apache/geode/DataSerializer.java 
> 8e2bad09adef9a6b4e7b5a5a71ff6b97fd515137 
>   geode-core/src/main/java/org/apache/geode/GemFireException.java 
> 142a97c9aa721cea82f7627a24da2517f3f97a24 
>   geode-core/src/main/java/org/apache/geode/cache/CacheException.java 
> 9b631a16b69bef63f9450b3c621764451ea743c4 
>   geode-core/src/main/java/org/apache/geode/cache/CacheRuntimeException.java 
> 9040596635c4ec6177f8e39183ab48025fa74658 
>   
> geode-core/src/main/java/org/apache/geode/cache/OperationAbortedException.java
>  340d1844b3d3827b6ff34c08a1ca814fb4c94be9 
>   geode-core/src/main/java/org/apache/geode/cache/RegionExistsException.java 
> 288dfe3c116ab7cd0cd3aec70473ad7c7ac918a8 
>   
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
>  6e7aa55bd8a8b99c63f39965daa0222f42f64d30 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
>  9a78772c466996f83383f4e63cb3d1d6654172a0 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/OldClientSupportService.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/BatchException70.java
>  b795156c324f4a0a17a4a0009f5816dec46c8f4c 
>   geode-core/src/main/java/org/apache/geode/pdx/internal/PdxType.java 
> 2e5b19f8b4f2980bb16bf17249ea894fca0652b0 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ContainsKey66Test.java
>  6728a377816e521a9419e8dd7576397f8548c14e 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CreateRegionTest.java
>  389399191a3a69a1f591d029ecf3ea3981f845db 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Destroy65Test.java
>  ed76cb6a965208b315ebb004cef39ea763d4b686 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/DestroyRegionTest.java
>  49a95a0bdae5f4fb51c0d6fcd8d5513805b45d26 
>   
> geode-core/src/test/java/org/apache

Re: Review Request 52172: GEODE-1927: add support for old GemFire remote sites (WAN)

2016-09-27 Thread Bruce Schuchardt

5b55d77a 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/PutTest.java
 a9c3af43de1b5b23a958d22b577091e6ccf0bca5 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RequestTest.java
 b6997bdef1b51e10780caf1eee6e3a7c46424f98 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/UnregisterInterestTest.java
 2da6f19a2708b839264c14b9c27e3ff6702a2b52 
  
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java
 1c87b1a0346445da95c9d510ecc8b8e6e996bac2 
  geode-old-client-support/build.gradle PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/GemFireException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/cache/execute/EmtpyRegionFunctionException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/cache/execute/FunctionException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/java/com/gemstone/gemfire/cache/execute/FunctionInvocationTargetException.java
 PRE-CREATION 
  
geode-old-client-support/src/main/resources/META-INF/services/org.apache.geode.internal.cache.CacheService
 PRE-CREATION 
  
geode-old-client-support/src/test/java/com/gemstone/gemfire/ClientDataSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/com/gemstone/gemfire/ClientPDXSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/com/gemstone/gemfire/ClientSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/ClientDataSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/ClientPDXSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/ClientSerializableObject.java
 PRE-CREATION 
  
geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
 PRE-CREATION 
  settings.gradle 95c15f2855d25938e551371b6f93192b7b953fba 

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


Testing
---

precheckin.  New tests are being developed for gemfire<->geode WAN and 
client/server interactions but these won't be part of the Geode repo.


Thanks,

Bruce Schuchardt

Re: security properties in the cluster config

2016-09-27 Thread Bruce Schuchardt

security-udp-dhalgo is new and is described here: 
https://cwiki.apache.org/confluence/display/GEODE/Secure+UDP+Communication+in+Geode


Le 9/26/2016 à 11:23 AM, Swapnil Bawaskar a écrit :

Hi John,
security-manager and security-post-processor are discussed here:
https://cwiki.apache.org/confluence/display/GEODE/Geode+Integrated+Security

On Mon, Sep 26, 2016 at 11:01 AM, Joey McAllister 
wrote:


Hi John,

They are documented in the docs dev branch and will be published with the
next Geode release. Also, we're scheduled to donate the docs code to the
project later this week, so you'll be able to see the work in dev.

Best,
Joey

On Mon, Sep 26, 2016 at 10:41 AM John Blum  wrote:


Jinmei-

Where are the following security-* properties documented?

security-udp-dhalgo

security-manager

security-post-processor

They certainly are not documented in the (Geode) User Docs, here
<
http://geode.docs.pivotal.io/docs/reference/topics/gemfire_

properties.html

  [1].

Thanks!
John

[1]
http://geode.docs.pivotal.io/docs/reference/topics/gemfire_

properties.html



On Mon, Sep 26, 2016 at 8:42 AM, Jinmei Liao  wrote:


Actually, I looked into the the config settings, these are the list of
settings that begin with security-. SSL settings are not there. The
security-client-* and security-peer-* are deprecated, so they don't

need

to

be in the cluster config. What about the udp-dhalgo and log-file and
log-level? Does it hurt to put them in the cluster-config?

"security-client-authenticator";

"security-client-accessor";

"security-client-accessor-pp";

"security-client-auth-init";

"security-client-dhalgo";

"security-peer-auth-init";

"security-peer-authenticator";

"security-peer-verifymember-timeout";

"security-udp-dhalgo";

"security-log-file";

"security-log-level";

"security-manager";

"security-post-processor";







On Fri, Sep 23, 2016 at 12:41 PM, Bruce Schuchardt <

bschucha...@pivotal.io

wrote:


SSL settings and the new UDP dhAlgo setting can't be in the cluster
config.  The cluster config is received over TCP/IP so you would have

to

use unsecured information to retrieve the settings, and you'd have to

do

it

before the cache is created.

Does the security-manager have any role to play prior to the cache

being

created?   For instance, is it involved in authenticating the receipt

of

a

new membership view or a join request in GMSAuthenticator?  If so you

can't

store it in the cluster config, which is only retrieved later on

during

cache creation.



Le 9/23/2016 à 11:57 AM, Michael Stolz a écrit :


I am in favor of keeping the SSL thoughts separate from the RBAC

thoughts,

but I don't see any reason they couldn't share the same repository.

That said though, does putting it all into the Cluster Configuration
Manager (CCM) make it so that you can only have security if you are

using

CCM for configuration?


--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771

On Fri, Sep 23, 2016 at 1:48 PM, Jinmei Liao 

wrote:

Hi, All,

I am working on this ticket:
https://issues.apache.org/jira/browse/GEODE-1659. Basically,

currently,

any
member(locator or server) needs to specify its own security-manager

in

order to protect its data which could leads to misconfiguration and

data

leak. So we would like to put it into the cluster configuration so

any

member who wants to join the cluster will need to apply the same

security

measures.

Now Here is my question, should we only put the "security-manager"

and

"security-post-processor" in the cluster config or any "security-*"
settings, which include SSL settings as well.

Thanks!

--
Cheers

Jinmei




--
Cheers

Jinmei




--
-John
503-504-8657
john.blum10101 (skype)

support for old clients

2016-09-23 Thread Bruce Schuchardt

I'm creating a new subproject for supporting old GemFire clients and WAN 
sites and need a name for it.  All of the current ones are prefixed with 
"geode-".


How about geode-gemfire-support?

Re: security properties in the cluster config

2016-09-23 Thread Bruce Schuchardt

SSL settings and the new UDP dhAlgo setting can't be in the cluster 
config.  The cluster config is received over TCP/IP so you would have to 
use unsecured information to retrieve the settings, and you'd have to do 
it before the cache is created.


Does the security-manager have any role to play prior to the cache being 
created?   For instance, is it involved in authenticating the receipt of 
a new membership view or a join request in GMSAuthenticator?  If so you 
can't store it in the cluster config, which is only retrieved later on 
during cache creation.



Le 9/23/2016 à 11:57 AM, Michael Stolz a écrit :

I am in favor of keeping the SSL thoughts separate from the RBAC thoughts,
but I don't see any reason they couldn't share the same repository.

That said though, does putting it all into the Cluster Configuration
Manager (CCM) make it so that you can only have security if you are using
CCM for configuration?


--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771

On Fri, Sep 23, 2016 at 1:48 PM, Jinmei Liao  wrote:


Hi, All,

I am working on this ticket:
https://issues.apache.org/jira/browse/GEODE-1659. Basically, currently,
any
member(locator or server) needs to specify its own security-manager in
order to protect its data which could leads to misconfiguration and data
leak. So we would like to put it into the cluster configuration so any
member who wants to join the cluster will need to apply the same security
measures.

Now Here is my question, should we only put the "security-manager" and
"security-post-processor" in the cluster config or any "security-*"
settings, which include SSL settings as well.

Thanks!

--
Cheers

Jinmei

Re: Review Request 52172: GEODE-1927: add support for old GemFire remote sites (WAN)

2016-09-22 Thread Bruce Schuchardt



> On Sept. 22, 2016, 9:07 p.m., Dan Smith wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/OldClientSupport.java,
> >  line 33
> > <https://reviews.apache.org/r/52172/diff/1/?file=1508448#file1508448line33>
> >
> > How is this extensible? This is a static method in a class?

A jar will go on the server's classpath before geode jars.  It will contain an 
OldClientSupport implementation that will translate the exception to a 
com.gemstone.gemfire exception for old clients.


> On Sept. 22, 2016, 9:07 p.m., Dan Smith wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java,
> >  line 171
> > <https://reviews.apache.org/r/52172/diff/1/?file=1508461#file1508461line171>
> >
> > What is this for?

There's no reason to have JRE jars in the classpath.


> On Sept. 22, 2016, 9:07 p.m., Dan Smith wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java,
> >  line 188
> > <https://reviews.apache.org/r/52172/diff/1/?file=1508461#file1508461line188>
> >
> > What is this for?

This will be used for backward-compatibility testing.  It will point to the 
test output directory so we can find builds of old clients.


- Bruce


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


On Sept. 22, 2016, 7:31 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52172/
> ---
> 
> (Updated Sept. 22, 2016, 7:31 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1927
> https://issues.apache.org/jira/browse/GEODE-1927
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This adds a check in InternalDataSerializer for com.gemstone.gemfire packages 
> and transforms them to org.apache.geode.
> 
> I've also added a hook for translating org.apache.geode exceptions into 
> com.gemstone.gemfire exceptions.  We've decided to keep com.gemstone.gemfire 
> exceptions out of the Geode repository to avoid the confusion it would cause 
> to have an org.apache.geode implementation throw com.gemstone.gemfire 
> exceptions.
> 
> The latter change required changes to some tests using mocks to represent 
> ServerConnections.  These mocks were returning null from getClientVersion(), 
> causing NPEs in client/server code.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
>  6e7aa55bd8a8b99c63f39965daa0222f42f64d30 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
>  9a78772c466996f83383f4e63cb3d1d6654172a0 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/OldClientSupport.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ContainsKey66Test.java
>  6728a377816e521a9419e8dd7576397f8548c14e 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CreateRegionTest.java
>  389399191a3a69a1f591d029ecf3ea3981f845db 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Destroy65Test.java
>  ed76cb6a965208b315ebb004cef39ea763d4b686 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/DestroyRegionTest.java
>  49a95a0bdae5f4fb51c0d6fcd8d5513805b45d26 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/DestroyTest.java
>  422733e4f5e958add74b4b7586bb123703776bc7 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Get70Test.java
>  4b63a072370746e950ef3697358c7be7a125a213 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/InvalidateTest.java
>  2dcdd0413c292acd3a9ef40f9809f23f5e95ca29 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put61Test.java
>  c368ba89435303f498a212defdc2508426366093 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java
>  830884c858ae3c6fd4121ed60343e4195b55d77a 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/PutTest.java
>  a9c3af43de1b5b23a958d22b577091e6ccf0bca5 
>   
> geode-core/src/test/java/org/apache/geode/internal/cach

Review Request 52172: GEODE-1927: add support for old GemFire remote sites (WAN)

2016-09-22 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


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


Repository: geode


Description
---

This adds a check in InternalDataSerializer for com.gemstone.gemfire packages 
and transforms them to org.apache.geode.

I've also added a hook for translating org.apache.geode exceptions into 
com.gemstone.gemfire exceptions.  We've decided to keep com.gemstone.gemfire 
exceptions out of the Geode repository to avoid the confusion it would cause to 
have an org.apache.geode implementation throw com.gemstone.gemfire exceptions.

The latter change required changes to some tests using mocks to represent 
ServerConnections.  These mocks were returning null from getClientVersion(), 
causing NPEs in client/server code.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java 
6e7aa55bd8a8b99c63f39965daa0222f42f64d30 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
 9a78772c466996f83383f4e63cb3d1d6654172a0 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/OldClientSupport.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ContainsKey66Test.java
 6728a377816e521a9419e8dd7576397f8548c14e 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CreateRegionTest.java
 389399191a3a69a1f591d029ecf3ea3981f845db 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Destroy65Test.java
 ed76cb6a965208b315ebb004cef39ea763d4b686 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/DestroyRegionTest.java
 49a95a0bdae5f4fb51c0d6fcd8d5513805b45d26 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/DestroyTest.java
 422733e4f5e958add74b4b7586bb123703776bc7 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Get70Test.java
 4b63a072370746e950ef3697358c7be7a125a213 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/InvalidateTest.java
 2dcdd0413c292acd3a9ef40f9809f23f5e95ca29 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put61Test.java
 c368ba89435303f498a212defdc2508426366093 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java
 830884c858ae3c6fd4121ed60343e4195b55d77a 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/PutTest.java
 a9c3af43de1b5b23a958d22b577091e6ccf0bca5 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RequestTest.java
 b6997bdef1b51e10780caf1eee6e3a7c46424f98 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/UnregisterInterestTest.java
 2da6f19a2708b839264c14b9c27e3ff6702a2b52 
  
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java
 1c87b1a0346445da95c9d510ecc8b8e6e996bac2 

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


Testing
---

precheckin.  New tests are being developed for gemfire<->geode WAN and 
client/server interactions but these won't be part of the Geode repo.


Thanks,

Bruce Schuchardt

Re: Error loading cluster config serialized prior to repackage

2016-09-20 Thread Bruce Schuchardt

The repackage broke those two methods.  The oldPackage needs to replace 
"org.apache" with "com.gemstone".  It allows interaction with a locator 
from WAN sites and clients running GemFire.


I'll fix that problem.

Le 9/20/2016 à 11:53 AM, Kirk Lund a écrit :

If current develop attempts to read a cluster config that was persisted
prior to the repackage, our code now throws ClassNotFoundException. Turns
out Cluster Config is implemented by the
class org.apache.geode.management.internal.configuration.domain.Configuration
which is a DataSerializable. Unfortunately, a DataSerializableFixedID was
never added for this class, so the code resorts to using the fully
qualified class name.

While brainstorming possible workarounds, we noticed a jgroups related
method in DataSerializer called swizzleClassNameForRead which is called
from readObject(DataInput):

   /**
* For backward compatibility we must swizzle the package of
* some classes that had to be moved when GemFire was open-
* sourced.  This preserves backward-compatibility.
*
* @param name the fully qualified class name
* @return the name of the class in this implementation
*/
   private static String swizzleClassNameForRead(String name) {
 String oldPackage = "org.apache.org.jgroups.stack.tcpserver";
 String newPackage = "org.apache.geode.distributed.internal.tcpserver";
 String result = name;
 if (name.startsWith(oldPackage)) {
   result = newPackage + name.substring(oldPackage.length());
 }
 return result;
   }

The package in red above never existed. I now have two questions: a) did
the repackage break this so that JGroups now need two swizzles (one for the
jgroups upgrade and now a 2nd for the apache repackage)? b) can cluster
Configuration piggy back on this technique for handling the serialized
Configuration for cluster config?

1) jgroups upgrade
 String oldPackage = "com.gemstone.jgroups.stack.tcpserver";
 String newPackage = "org.apache.geode.distributed.internal.tcpserver";

2) apache repackage for jgroups
 String oldPackage = "com.gemstone.gemfire.distributed.internal.tcpserver
";
 String newPackage = "org.apache.geode.distributed.internal.tcpserver";

3) apache repackage for cluster config
 String oldPackage = "com.gemstone.gemfire.management.internal.
configuration.domain";
 String newPackage = "org.apache.geode.management.
internal.configuration.domain";

Can anyone else think of something similar that might be broken? We could
scan the source code for DataSerializables that don't have a
corresponding DataSerializableFixedID.
Should we also scan for Serializable classes and try to determine if they
might be similarly persisted in a way that might break a feature?

Is this the best way to handle this? Should we reorganize
swizzleClassNameForRead to be a series of registered DataSwizzlers?

Thanks,
Kirk

jgroups 3.6.11 released

2016-09-19 Thread Bruce Schuchardt

There is a new version of jgroups available but I don't see any bugfixes 
that are relevant to Geode.  I think we should stick with 3.6.10 for the 
1.0.0 release.


https://issues.jboss.org/projects/JGRP/versions/12330930

Re: securing geode components

2016-09-13 Thread Bruce Schuchardt


+1

Le 9/12/2016 à 11:31 AM, Udo Kohlmeyer a écrit :

So it seems that we are all in agreement.

The communication channel security (via SSL) is to be separated from 
the RBAC.


I suggest the enum that deals with the securing of the communication 
channels is renamed to: SecurableCommunicationChannels. This internal 
enum will not be shared with RBAC anymore nor exposed as a public API.


In addition to this http will be renamed to web, which is non-protocol 
specific.


--Udo


On 10/09/2016 3:17 AM, Michael Stolz wrote:

Cool! I agree with this completely. Wasn't clear to me in the earlier
thread that we were keeping the thoughts separate.

--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771

On Fri, Sep 9, 2016 at 1:06 PM, John Blum  wrote:

I agree with Udo here.  Securing the channel between 
component/services has

really very little to do with Authentication/Authorization, and by
"Authentication", I mean in the user-centric sense, not the SSL 
"trusted"

sense (with "trusted" keys and such).

Whether a user can or cannot do something (in other words, is 
"authorized",
or allowed to invoke an action, given an ACL) is a function of the 
action

being performed and the privileges/rights that a user has been granted.
That is independent of whether or not the channel has been secured.

I also agree with Kirk that HTTP should perhaps be renamed to WEB.

Finally, while SSL is usually a cause to reboot the system, Auth has no
such restrictions.  I could easily change Auth credentials of a user 
(e.g.

revoke privileges) at runtime.

My $0.02,
-John


On Fri, Sep 9, 2016 at 10:00 AM, Michael Stolz  
wrote:


In a pure world I would go for all or nothing, but I always worry 
about

the

upgrade path. If I have to redeploy and restart EVERYTHING around the

whole

world simultaneously, it's a non-starter.

--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771

On Fri, Sep 9, 2016 at 12:51 PM, Anthony Baker 

wrote:

Mike, I was suggesting ON | OFF only for RBAC security, not for SSL
configuration.  Any thoughts on that?

Anthony


On Sep 9, 2016, at 9:44 AM, Michael Stolz  wrote:

I think a reason that we might need to be less than 
all-or-nothing is

for

at least these two situations:

1. a user who started out with SSL disabled, and now wants to enable

it,

but can't take a full global outage, so needs to get it enabled for

the

WAN

first, and then for server-to-server and then for client/server.

2. SSL enabled over the WAN because that is not a trusted network,

but

they

can live without SSL for the server/server and client/server

connections

because they ARE in a trusted network and they don't need to pay the
overhead for SSL on those links.

There are probably other scenarios as well, but these are the two

that

come

to mind quickly.



--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771

On Fri, Sep 9, 2016 at 12:05 PM, Jinmei Liao 

wrote:

That is my original thought as well. If we are protecting resources
(CLUSTER and DATA), it should be protected no matter which way user

is

trying to access it. I guess I'll leave this to the PMs to decide.

On Thu, Sep 8, 2016 at 7:37 PM, Anthony Baker 

wrote:

Udo, Kirk - this makes sense and thanks for the discussion to help

clarify

the issue.

Regarding GEODE-1648, does it make sense to do at all?  That is,

what

if

role-based access control is either ON | OFF instead of per

component.

If

we allow disabling RBAC for certain components, wouldn’t that make

it
possible to create a backdoor?  Could we start with a binary 
option

and

enable more granular control when requested by users?

Anthony


On Sep 8, 2016, at 4:11 PM, Kirk Lund  wrote:

+1 overall with some feedback...

1) I think the list is reasonable with a few nitpicks below

2) If these are Channels and not Components, then I would 
probably

name

it

SecurableChannels or SecurableCommunicationChannels or whatever.

3) I'd prefer HTTP be renamed to Web or other non-protocol 
word --

HTTP

is

a protocol and the names of the other channels are conceptual

channels

rather than a protocol (the others use TCP/IP or RMI but we don't

label

them as such). Or am I missing something here?

4) JMX is probably ok. Currently we are using (and securing) JMX

over

RMI

(javax.management.remote.rmi.RMIConnectorServer). There are other
connectors for JMX including HTTP (ex: mx4j.tools.adaptor.http.

HttpAdaptor)

and SNMP (ex: com.sun.jmx.snmp.daemon.SnmpAdaptorServer). We only

need

JMX

over RMI for now, but would we add those others as new enums to
SecurableChannels later if we add anything like that to Geode? Or

would

we
try to group those all together under the name JMX? Or decide 
when

the

time

comes?

I think we should try to steer away from being overly controlled

by

specs

especially for reasonable changes. We all follow agile process,

so a

decision made one iteration could easily be undone or c

Hitesh would like to freeze develop for a couple of days

2016-09-09 Thread Bruce Schuchardt

Hitesh has been working on package renaming (com.gemstone.gemfire -> 
org.apache.geode) and would like to schedule a time when the develop 
branch is frozen for a couple of days.  That will let him create a 
feature branch, perform the renaming, test the renamed system and merge 
it back to develop.


I know that Udo has some SSL changes he needs to merge to develop. Who 
else has a large change set they're working on?

Review Request 51696: GEODE-1777 CI failure: RestAPIsOnMembersFunctionExecutionDUnitTest.testFunctionExecutionEOnSelectedMembers[

2016-09-07 Thread Bruce Schuchardt


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

Review request for geode, Anthony Baker, Hitesh Khamesra, Udo Kohlmeyer, and 
Dan Smith.


Repository: geode


Description
---

Looking through our distributedTest output I see a large number of full GCs 
initiated by Metaspace cleanup.  Some of these GCs are taking as long as 70 
seconds and cause nodes to be kicked out of the distributed system.  For Java 8 
it's recommended that you estimate requirements for Metaspace and set 
XXMetaspaceSize to avoid these full GCs.  Our REST tests seem to need nearly 
half a gigabyte of metaspace so I've set it accordingly.  An AWS precheckin run 
with this change showed only one GC initiated by metaspace management, in the 
geode-web distributedTest suite lasting 1/2 second.


Diffs
-

  
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/standalone/ProcessManager.java
 f938c1a88cfec459efe4c87b8be41a6fa5c291e4 

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


Testing
---

precheckin


Thanks,

Bruce Schuchardt

Re: Review Request 51666: GEODE-1729: Configuration consistency in SSL configuration

2016-09-06 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On Sept. 6, 2016, 8:43 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51666/
> ---
> 
> (Updated Sept. 6, 2016, 8:43 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Hitesh Khamesra.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ssl-ciphers & ssl-protocols are defined as space-delimited multi-valued. 
> Whereas ssl-enabled-components is comma-delimited. It is more logical that 
> multi-values are comma-delimited rather than space-delimited. To have some 
> consistency in the new ssl configuration ssl-ciphers and ssl-protocols will 
> be comma-delimited.
> This is contradictory to the already configured *-ssl-ciphers and 
> *-ssl-protocols legacy settings which are space-delimited.
> In order not to have a time-costly refactor just to handle the ssl-ciphers 
> and ssl-protocols, the comma-delimited properties will internally be 
> converted to space-delimited. This is safer as all testing has been done with 
> space-delimited.
> A cleaner solution is proposed in GEODE-1865.
> 
> In this diff JMX_MANAGER_SSL_ALIAS is also renamed to JMX_SSL_ALIAS.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/ConfigurationProperties.java
>  df85aca 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java
>  f92511e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java
>  4ad95c6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java
>  c86b0e7 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/net/SSLConfigurationFactory.java
>  df4f49c 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/net/SocketCreator.java 
> 0a2bfa3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/GemFireProperties.java
>  410d658 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/beans/BeanUtilFuncs.java
>  9d62fbf 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
>  b68841a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/net/SSLConfigurationFactoryTest.java
>  d890457 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/net/SocketCreatorFactoryJUnitTest.java
>  c55c661 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/ConnectToLocatorSSLDUnitTest.java
>  75a0e82 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/JMXMBeanDUnitTest.java
>  f08c172 
> 
> Diff: https://reviews.apache.org/r/51666/diff/
> 
> 
> Testing
> ---
> 
> Precheckin currently running.
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: Nightly Build still failing with BindExceptions

2016-09-06 Thread Bruce Schuchardt

This test is not using AvailablePort.  There are two test cases in this 
class that alway use port .


Le 9/6/2016 à 8:00 AM, Anthony Baker a écrit :

How could we fix AvailablePort so we don’t try to use in-use ports?

Anthony


On Sep 3, 2016, at 10:29 PM, Kirk Lund  wrote:

We're still hitting BindExceptions in the nightly build, so I'll go ahead
and propose this again: any test that uses AvailablePort to find a random
port could be altered to automatically Retry if it encounters and fails
because of java.net.BindException. Opinions?

-Kirk

:geode-core:integrationTest

com.gemstone.gemfire.internal.cache.DiskRegionJUnitTest >
testBridgeServerRunningInSynchPersistOnlyForIOExceptionCase FAILED
java.net.BindException: Failed to create server socket on  null[5,555]
at com.gemstone.gemfire.internal.SocketCreator.createServerSocket(
SocketCreator.java:814)
at com.gemstone.gemfire.internal.SocketCreator.createServerSocket(
SocketCreator.java:774)
at com.gemstone.gemfire.internal.SocketCreator.createServerSocket(
SocketCreator.java:738)
at com.gemstone.gemfire.internal.cache.tier.sockets.
AcceptorImpl.(AcceptorImpl.java:470)
at com.gemstone.gemfire.internal.cache.CacheServerImpl.start(
CacheServerImpl.java:323)
at com.gemstone.gemfire.internal.cache.DiskRegionJUnitTest.
testBridgeServerRunningInSynchPersistOnlyForIOExceptionCase(
DiskRegionJUnitTest.java:2215)

Caused by:
java.net.BindException: Address already in use
at java.net.PlainSocketImpl.socketBind(Native Method)
at java.net.AbstractPlainSocketImpl.bind(
AbstractPlainSocketImpl.java:387)
at java.net.ServerSocket.bind(ServerSocket.java:375)
at com.gemstone.gemfire.internal.SocketCreator.
createServerSocket(SocketCreator.java:811)
... 5 more

com.gemstone.gemfire.internal.cache.DiskRegionJUnitTest >
testBridgeServerStoppingInSynchPersistOnlyForIOExceptionCase FAILED
java.net.BindException: Failed to create server socket on  null[5,555]
at com.gemstone.gemfire.internal.SocketCreator.createServerSocket(
SocketCreator.java:814)
at com.gemstone.gemfire.internal.SocketCreator.createServerSocket(
SocketCreator.java:774)
at com.gemstone.gemfire.internal.SocketCreator.createServerSocket(
SocketCreator.java:738)
at com.gemstone.gemfire.internal.cache.tier.sockets.
AcceptorImpl.(AcceptorImpl.java:470)
at com.gemstone.gemfire.internal.cache.CacheServerImpl.start(
CacheServerImpl.java:323)
at com.gemstone.gemfire.internal.cache.DiskRegionJUnitTest.
testBridgeServerStoppingInSynchPersistOnlyForIOExceptionCase
(DiskRegionJUnitTest.java:2103)

Caused by:
java.net.BindException: Address already in use
at java.net.PlainSocketImpl.socketBind(Native Method)
at java.net.AbstractPlainSocketImpl.bind(
AbstractPlainSocketImpl.java:387)
at java.net.ServerSocket.bind(ServerSocket.java:375)
at com.gemstone.gemfire.internal.SocketCreator.
createServerSocket(SocketCreator.java:811)
... 5 more

3247 tests completed, 2 failed, 175 skipped

please close your submitted and discarded RB reviews

2016-08-26 Thread Bruce Schuchardt

My dashboard has 105 items from other engineers on it and I know they're 
mostly old reviews that are no longer needed.


 It just takes a minute to close them.  Go to the dashboard, click on 
"Outgoing - Open" to bring up your reviews.   Click on the check-boxes 
of the ones you're done with and hit closed-submitted or closed-discarded.


Thanks

Re: Review Request 51461: GEODE-1803 Inefficient code in ClientMetadataService.getServerToFilterMap()

2016-08-26 Thread Bruce Schuchardt


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




geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ClientPartitionAdvisor.java
 (line 120)
<https://reviews.apache.org/r/51461/#comment213915>

I'm going to change this to use the new "random" instance variable.


- Bruce Schuchardt


On Aug. 26, 2016, 4:21 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51461/
> ---
> 
> (Updated Aug. 26, 2016, 4:21 p.m.)
> 
> 
> Review request for geode and Hitesh Khamesra.
> 
> 
> Bugs: GEODE-1803
> https://issues.apache.org/jira/browse/GEODE-1803
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> avoid using Collections.shuffle() to find a random server location
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ClientPartitionAdvisor.java
>  d3b0cf3204b2d295b362544425a3f08a83090974 
> 
> Diff: https://reviews.apache.org/r/51461/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Review Request 51461: GEODE-1803 Inefficient code in ClientMetadataService.getServerToFilterMap()

2016-08-26 Thread Bruce Schuchardt


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

Review request for geode and Hitesh Khamesra.


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


Repository: geode


Description
---

avoid using Collections.shuffle() to find a random server location


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ClientPartitionAdvisor.java
 d3b0cf3204b2d295b362544425a3f08a83090974 

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


Testing
---

precheckin


Thanks,

Bruce Schuchardt

Re: Review Request 51394: GEODE-1372 Geode UDP communications are not secure when SSL is configured

2016-08-24 Thread Bruce Schuchardt


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



I have a few review comments, and I think we need you to walk us through the 
algorithm and GMSEncrypt.  We need to update documentation based on the current 
design.


geode-core/src/main/java/com/gemstone/gemfire/distributed/ConfigurationProperties.java
 (line 1207)
<https://reviews.apache.org/r/51394/#comment213304>

I realize that SECURITY_CLIENT_DHALGO doesn't have much of a javadoc and 
you probably copied it to make this one, but you need to add a description of 
the property.  This is the primary javadoc location for distribution config 
properties.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetView.java
 (line 47)
<https://reviews.apache.org/r/51394/#comment213308>

How about fixing this TODO before you're done?  Otherwise we're going to 
have another TODO story to deal with.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java
 (line 23)
<https://reviews.apache.org/r/51394/#comment213309>

remove reference to GMSMember from the interface



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/Messenger.java
 (line 90)
<https://reviews.apache.org/r/51394/#comment213339>

These methods all need good javadocs and getPublickey should be 
getPublicKey with a capital "K".



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java
 (line 257)
<https://reviews.apache.org/r/51394/#comment213313>

GMSLocator shouldn't directly refer to Messenger implementation classes.  
Can you add this to the Messenger interface and have JGroupsMessenger invoke 
the GMSEncrypt method?



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 878)
<https://reviews.apache.org/r/51394/#comment213317>

The "K" should be capitalized in this method name



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 879)
<https://reviews.apache.org/r/51394/#comment213316>

fix the TODO



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 897)
<https://reviews.apache.org/r/51394/#comment213319>

Please add a comment describing why a View from a still-valid member is 
being rejected here.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 1134)
<https://reviews.apache.org/r/51394/#comment213321>

I don't see anything that changes in this message for each target member.  
Why aren't you just setting multiple recipients and sending it once?  If it's 
due to some JGroupsMessenger requirement related to handshaking can't the 
messenger figure that out?



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 1232)
<https://reviews.apache.org/r/51394/#comment213323>

getPublickey should have a capital "K": getPublicKey



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 2083)
<https://reviews.apache.org/r/51394/#comment213325>

Make this debug level?



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 2229)
<https://reviews.apache.org/r/51394/#comment213327>

delete dead code



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 (line 2358)
<https://reviews.apache.org/r/51394/#comment213328>

delete dead code



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncrypt.java
 (line 1)
<https://reviews.apache.org/r/51394/#comment213329>

Let's walk through this together



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
 (line 725)
<https://reviews.apache.org/r/51394/#comment21>

why is this using an Iterator?  That's taking a step backward and I can't 
see why it's needed.



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
 (line 743)
<https://reviews.apache.org/r/51394/#comment213334>

unnecessary cast



geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
 (line 898)
<https://reviews.apache.org/r/51394/#comment213336&g

Re: Review Request 51385: GEODE-900 test endup uding old cache

2016-08-24 Thread Bruce Schuchardt



> On Aug. 24, 2016, 8:27 p.m., Bruce Schuchardt wrote:
> > While it's okay to synchronize like that in checkExistingCache() I think it 
> > might be more appropriate to synchronize on CacheFactory.class in 
> > basicCreate() and fold the checkExistingClass() method into that method 
> > since that's the only place it's used.  I'm concerned that the static 
> > "instance" variable is being set in GemFireCacheImpl.initialize() outside 
> > of synchronization.  It used to be that there was only one path to 
> > basicCreate() and that method synchronized on CacheFactory.class, but now 
> > there appear to be other paths to it, like client-cache creation, that 
> > don't synchronize.
> 
> Hitesh Khamesra wrote:
> >>I'm concerned that the static "instance" variable is being set in 
> GemFireCacheImpl.initialize() outside of synchronization. 
> This is now part of synchronization
> 
>synchronized (GemFireCacheImpl.class) {
> GemFireCacheImpl instance = checkExistingCache(existingOk, 
> cacheConfig);
> if (instance == null) {
>   instance = new GemFireCacheImpl(isClient, pf, system, 
> cacheConfig, asyncEventListeners, typeRegistry);
>   instance.initialize();
> }
> return instance;
>   }

Yes, like that


- Bruce


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


On Aug. 24, 2016, 6:30 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51385/
> ---
> 
> (Updated Aug. 24, 2016, 6:30 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Darrel Schneider.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> 1. When cache is closed need to return null from "checkExistingCache"
> 2. Need to synchronize cache creation
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
>  76a7bad 
> 
> Diff: https://reviews.apache.org/r/51385/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 51385: GEODE-900 test endup uding old cache

2016-08-24 Thread Bruce Schuchardt

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

While it's okay to synchronize like that in checkExistingCache() I think it 
might be more appropriate to synchronize on CacheFactory.class in basicCreate() 
and fold the checkExistingClass() method into that method since that's the only 
place it's used.  I'm concerned that the static "instance" variable is being 
set in GemFireCacheImpl.initialize() outside of synchronization.  It used to be 
that there was only one path to basicCreate() and that method synchronized on 
CacheFactory.class, but now there appear to be other paths to it, like 
client-cache creation, that don't synchronize.

- Bruce Schuchardt

On Aug. 24, 2016, 6:30 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51385/
> ---
> 
> (Updated Aug. 24, 2016, 6:30 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Darrel Schneider.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> 1. When cache is closed need to return null from "checkExistingCache"
> 2. Need to synchronize cache creation
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
>  76a7bad 
> 
> Diff: https://reviews.apache.org/r/51385/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 51340: Added vmkind field in InterbalDistributedMember' wrtieEssentialData/readEssentialData method

2016-08-23 Thread Bruce Schuchardt


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



Wouldn't it be better to just set a VmKind in the ID created by 
JGroupsMessenger if it can't find a corresponding ID in the membership views?  
Either that or get rid of the assertion in InternalDistributedMember.  Adding 
an extra byte to every message transmitted seems like a big price to pay just 
to avoid this assertion failure.

- Bruce Schuchardt


On Aug. 23, 2016, 4:25 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51340/
> ---
> 
> (Updated Aug. 23, 2016, 4:25 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Fix for GEM-946
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java
>  b7754ce 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  f43c98d 
> 
> Diff: https://reviews.apache.org/r/51340/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 51340: Added vmkind field in InterbalDistributedMember' wrtieEssentialData/readEssentialData method

2016-08-23 Thread Bruce Schuchardt


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



You need to create an Apache JIRA ticket describing this problem.

- Bruce Schuchardt


On Aug. 23, 2016, 4:25 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51340/
> ---
> 
> (Updated Aug. 23, 2016, 4:25 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Fix for GEM-946
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java
>  b7754ce 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  f43c98d 
> 
> Diff: https://reviews.apache.org/r/51340/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 49962: Removed extra fields from distributedmember while serialization/de

2016-08-22 Thread Bruce Schuchardt


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




geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
 (line 895)
<https://reviews.apache.org/r/49962/#comment212757>

I don't think this can be transparent, but at the same time do we really 
need static methods on GMSMember for this when there are already instance 
methods?  I also prefer the method names in InternalDistributedMember 
(write/readEssentialData) because "shallow" to me means not recursing into 
contained objects, such as "shallow copy".


- Bruce Schuchardt


On Aug. 10, 2016, 9:25 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> ---
> 
> (Updated Aug. 10, 2016, 9:25 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Removed extra fields from distributedmember while serialization/de. Planning 
> to do this in udp-security branch.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java
>  d5d0b8e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java
>  87409c5 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  080bdb3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  5c0a327 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java
>  7eef594 
> 
> Diff: https://reviews.apache.org/r/49962/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 51227: GEODE-420: Locator SSL

2016-08-19 Thread Bruce Schuchardt


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




geode-core/src/main/java/com/gemstone/gemfire/distributed/ConfigurationProperties.java
 (line 158)
<https://reviews.apache.org/r/51227/#comment212592>

There are a number of deprecated SSL properties in this file that say to 
use other properties, but those properties are also deprecated an point to the 
new properties you've added.  In order to avoid confusion they should all point 
to your new properties.  See SERVER_SSL_CIPHERS, for instance.


ReviewBoard has really made a mess of your diffs.  There are also a large 
number of formatting changes to wade through.  Can you do a walkthrough for us?

- Bruce Schuchardt


On Aug. 18, 2016, 11:30 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51227/
> ---
> 
> (Updated Aug. 18, 2016, 11:30 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Hitesh Khamesra.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> As per the specification: 
> https://cwiki.apache.org/confluence/display/GEODE/Revised+SSL+properties
> 
> * Removal of legacy/deprecate previous ssl-* properties
> * Deprecation of current -ssl-* properties
> * Moving of SocketCreator and SocketCloser to the 
> com.gemstone.gemfire.internal.net package
> * Addition of an ExtendedAliasKeyManager to manage keystores with many keys 
> and aliases
> * Addition of locator ssl properties
> * Addition of SocketCreatorFactory
> * Addition of SSLConfigFactory
> * Addition of JMXMBeanDUnitTest
> * Addition of SSLEnabledComponent Enum
> * Ciphers and Protocols are stored as String[] instead of previous legacy 
> String
> * TCPClient is now instance rather than Singleton
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationEndToEndDUnitTest.java
>  3408717 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/rest/internal/web/controllers/RestAPIsQueryAndFEJUnitTest.java
>  8321caf 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/rest/internal/web/controllers/RestAPIsWithSSLDUnitTest.java
>  daa781d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/DistributedSystemConfig.java
>  5ef389f 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/GemFireMemberStatus.java 
> 1b544a8 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/DistributedSystemConfigImpl.java
>  517f5a6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/DistributedSystemHealthMonitor.java
>  54e7de7 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/DistributionLocatorConfigImpl.java
>  88d939a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/EnabledManagedEntityController.java
>  ff139c5 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/InetAddressUtil.java
>  43d8e44 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/ManagedEntityConfigImpl.java
>  69751e9 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java
>  a71c479 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentLauncher.java
>  dfa9ac3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/MX4JServerSocketFactory.java
>  649038e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/AutoConnectionSourceImpl.java
>  844a775 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionFactoryImpl.java
>  56449db 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionImpl.java
>  6ed2912 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/ConfigurationProperties.java
>  5c3a282 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/Locator.java 
> 8795b71 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/LocatorLauncher.java
>  81d874d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/SSLEnabledComponents.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
> cce482a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java
>  cc544f6 
>   
> geode-core/src/main/java/com/gemstone/g

Review Request 51249: GEODE-1800 StoppableCondition has faulty code in await()

2016-08-19 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.


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


Repository: geode


Description
---

The faulty methods in StoppableCondition have been removed.  I considered 
removing StoppableCondition entirely and just using a Condition on the lock 
wrapped by the Stoppable lock class but doing so would require additional work 
to make the CancelCriterion available to the code that uses these 
StoppableConditions, so other than removing the methods I've left the class 
alone.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
 1ea5611fcc197a04dbd35f6386e66fc2f0ac997d 
  
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/locks/GrantorRequestProcessor.java
 e22a8c06ce73d5cc5d5aa08a57eb8aac66cb6965 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HARegionQueue.java
 6ac56ffa89b6cde47319ac9e9bb99e234b8d485a 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java
 1b5c11f3d00a256df8128eb1cd9b80b2989d990a 
  
geode-core/src/main/java/com/gemstone/gemfire/internal/util/concurrent/StoppableCondition.java
 5c33498ab3c5de6ae1843449e4c44f3d8eabd2d0 

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


Testing
---

precheckin


Thanks,

Bruce Schuchardt

Re: Review Request 51208: GEODE-1761 Clients don't fail back when servers are bounced

2016-08-18 Thread Bruce Schuchardt


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




geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ClientMetadataService.java
 (line 625)
<https://reviews.apache.org/r/51208/#comment212466>

I'm missing a statistics update here - I'll add it.


- Bruce Schuchardt


On Aug. 18, 2016, 3:54 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51208/
> ---
> 
> (Updated Aug. 18, 2016, 3:54 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1761
> https://issues.apache.org/jira/browse/GEODE-1761
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Servers will send a refresh hint to clients if they detect that a request had 
> to be send to a different server who owned the primary bucket affected by the 
> operation.  Clients should always refresh when this happens unless they have 
> connection-pool size constraints that force them to use non-optimal servers.
> 
> Client-side operation classes have been modified to initiate the refresh.  
> I've added code in the meta-data service class to avoid performing multiple 
> concurrent refreshes on the same region.
> 
> On the server-side I've cleaned up some of the network-hop detection code to 
> stop using hard-coded integers and to consolidate some of the code that 
> resets the ThreadLocals being used to record network-hops detected.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/AbstractOp.java
>  1eb0db2d24d01b77fea3315a6ce26b6f959c 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ClientMetadataService.java
>  6e255c478e1d87bdf58d52934647a4acea2c655c 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/DestroyOp.java
>  e8ce1a7f88ed1bb7a8d7235f69c018611358364a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/GetClientPRMetaDataOp.java
>  9a467f7793a7d20c8f3a3ae6945261054f79d079 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/GetOp.java
>  6864306e0884d9b531660e78834a6bc007ece884 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PutAllOp.java
>  16104563be74f45074faae62042d68d5e44dc0e1 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PutOp.java
>  072ec4e1ddcf79c1a5e5030e6e94be780f98791a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/RemoveAllOp.java
>  1ab1ed31b9033bb89ba08e15592563dd243c0354 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/SingleHopOperationCallable.java
>  6047a501134734c3e2ef070777b0fdbf4f445c91 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegion.java
>  9ac95a1f9c0ab880d6cab8a350bd9c4842797d5d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java
>  f3485d255cb641c926bc8f0333426ccc41b3ae78 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java
>  4947e202296897b01efcb75075f5056f2442bba0 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Destroy.java
>  bc47e2a3ee93427dc2f251c3298c38dfdf5a54cb 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Destroy65.java
>  a571f713d4232ff94085f32c5e9a10ac35b3e98b 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Destroy70.java
>  82d9c1a0f65dd2e00a1454cff50d680d94d1fe11 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Get70.java
>  f5d9937ec06a85eacc3aa8f473b7824b51d3fd6e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Invalidate.java
>  166b11a3d385a0f8d21f66158603788746f17256 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Invalidate70.java
>  6200438428a67d49c005cddc25caeacbd81c8e14 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Put61.java
>  4529b2d035b8ff50e75fc46fc04fe365b345188a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/Put65.java
>  48d923cce194e7ae7829401b910300a809bcbf38 
>   
> geode-core/src/main/java/co

Review Request 51208: GEODE-1761 Clients don't fail back when servers are bounced

2016-08-18 Thread Bruce Schuchardt

8bd7faf9fe 

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


Testing
---


Thanks,

Bruce Schuchardt

Re: git commit messages

2016-08-18 Thread Bruce Schuchardt

I've found that a lot of CI failure JIRA titles don't conform to the width
limit, and I like to use those in my commit message title.  Otherwise I
have no problem with this.

Le jeudi 18 août 2016, Kenneth Howe  a écrit :

> +1 to Kirk’s proposal
>
> > On Aug 17, 2016, at 6:09 PM, Kevin Duling  > wrote:
> >
> > +1 for that
> >
> >
> > On Wed, Aug 17, 2016 at 5:25 PM, Kirk Lund  > wrote:
> >
> >> The guidelines written by Chris Beams are designed to fit well within
> the
> >> gui for github. He specifically says try for 50 but then try to keep 69
> >> chars as a hard-limit because the github gui will truncate at 69 chars
> and
> >> append ellipse "..." at the end. This mostly affects non-apache
> committers
> >> creating pull requests.
> >>
> >> I favor loose adherence to these guidelines:
> >>
> >> 1st line should be limited to 50-69 chars if possible. Subsequent lines
> >> should be wrapped at 72 chars.
> >>
> >> -Kirk
> >>
> >>
> >> On Wed, Aug 17, 2016 at 4:47 PM, Dan Smith  > wrote:
> >>
> >>> Yeah, 50 chars seems a bit short. I think I've been aiming for 72
> >>> personally.
> >>>
> >>> -Dan
> >>>
> >>> On Wed, Aug 17, 2016 at 4:37 PM, Kirk Lund  > wrote:
> >>>
>  Some examples from feature/GEODE-1781-02 which are my latest unmerged
>  commits...
> 
>  1) 1st line is 62 chars. To shorten to 50: "GEODE-1799: fix javadocs
> on
>  CacheServerStats" or "GEODE-1799: change javadocs from Bridge to
> Cache"
> 
>  commit 0a07db189c8b928976ed6554499f15b6a64e1633
>  Author: Kirk Lund >
>  Date:   Wed Aug 17 16:27:52 2016 -0700
> 
> GEODE-1799: change javadocs from Bridge Server to Cache Server
> 
>  2) 1st line is 80 chars. To shorten to 50: "GEODE-1781: exclude class
> >>> from
>  AnaylzeSerializableJUnitTest"
> 
>  commit 55c5df5e4cd6228be1617fb1e92d8d955a703b08
>  Author: Kirk Lund >
>  Date:   Tue Aug 16 09:25:33 2016 -0700
> 
> GEODE-1781: exclude LinuxProcFsStatistics$CPU from
>  AnaylzeSerializableJUnitTest
> 
>  3) 1st line is 59 chars. To shorten to 50: "GEODE-1782: fix stat
> >> resource
>  equals"
>  (the later lines meet our guidelines)
> 
>  commit 7dc1ce68a483f993adeb613893073d8a7c88a9b7
>  Author: Kirk Lund >
>  Date:   Mon Aug 15 18:35:45 2016 -0700
> 
> GEODE-1782: include start timestamp in stat resource equals
> 
> * stat resources with different time stamps should not be equal
> 
> * StatArchiveWithConsecutiveResourceInstGenerator generates gfs
> >> with
> multiple stat resources of same name but different times
> 
> * StatArchiveWithConsecutiveResourceInstIntegrationTest confirms
> existence of bug GEODE-1782: StatArchiveReader ignores later stats
> resource with same name as closed stats resource
> 
> * ResourceInstTest verifies the underlying issue and fix in
> StatArchiveReader.ResourceInst.equals and the fix
> 
>  4) I got this one down to 48 chars!
>  (the later lines meet our guidelines)
> 
>  commit 115070123ec15638ca1189a7349938c35e0d51e0
>  Author: Kirk Lund 
>  Date:   Mon Aug 15 18:26:16 2016 -0700
> 
> GEODE-1781: refactor internal statistics classes
> 
> * move internal statistics classes into
> >>> com.gemstone.gemfire.internal.
> statistics
> 
> * move statistics tests into com.gemstone.gemfire.internal.
> >>> statistics
> 
> * modify tests to include integration and distributed in names
> 
> * modify tests to use TemporaryFolder and TestName rules
> 
>  On Wed, Aug 17, 2016 at 4:22 PM, Kenneth Howe  >
> >> wrote:
> 
> > Agree with Kirk, 50 chars is really short by the time you use up the
>  first
> > 12 characters for the Jira tag. If we’re going to have a guideline,
> >> I’d
> > rather be longer - somewhat arbitrarily I’d probably make it 20-30
> >>> chars
> > more. It’s been a long time since text listings were intended to fit
> >>> on a
> > 80x24 dumb terminal, so I don’t see a need to restrict the commit
> >>> message
> > headers so severely.
> >
> > I do use the —online option embedded in a local alias I use to look
> >> at
> >>> a
> > history list of my local repo.
> >
> > Ken
> >
> >> On Aug 17, 2016, at 3:45 PM, Kevin Duling  >
> >>> wrote:
> >>
> >> The format is very similar to the one most other git shops I've
> >>> worked
>  in
> >> before use.  I don't believe we ever had formal length limits.
> > Typically,
> >> it was:
> >>
> >> -: 
> >>>
> >> blank line
> >>
> >>  > ticket>
> >>
> >>
> >> The Atlassian plugin for IDEA automates a lot of this.  There are
>  limits
> > on
> >> the length of a jira ticket summary, but I'm not sure what that is.
> >>> I
> > ran
> >> in to it when I did my round of CI.
>

Re: Flaky tests failing with BindException

2016-08-17 Thread Bruce Schuchardt


The membership-port-range changes have been checked in for a while now.

Le 8/17/2016 à 10:51 AM, Kirk Lund a écrit :

Have we made all of the changes that we think will help prevent
BindException failures?

Last night's nightly build failed with one again:

:geode-core:flakyTest

com.gemstone.gemfire.security.ClientAuthenticationDUnitTest >
testCredentialsForNotifications FAILED
 com.gemstone.gemfire.test.dunit.RMIException: While invoking
com.gemstone.gemfire.security.ClientAuthenticationTestCase$$Lambda$26/1964608307.call
in VM 0 running on Host asf902.gq1.ygridcore.net with 4 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:320)
 at
com.gemstone.gemfire.security.ClientAuthenticationTestCase.doTestCredentialsForNotifications(ClientAuthenticationTestCase.java:456)
 at
com.gemstone.gemfire.security.ClientAuthenticationDUnitTest.testCredentialsForNotifications(ClientAuthenticationDUnitTest.java:82)

 Caused by:
 java.lang.AssertionError: Got unexpected exception when starting
server

 Caused by:
 java.net.BindException: Failed to create server socket on
  null[60,026]

 Caused by:
 java.net.BindException: Address already in use

193 tests completed, 1 failed, 6 skipped

On Thu, Aug 4, 2016 at 10:38 AM, Bruce Schuchardt 
wrote:


I've pushed the port-range changes that I described in my last email on
this subject.


Le 8/1/2016 à 5:33 PM, Kirk Lund a écrit :


I think that the changes mentioned by Jens and Bruce obviate the need to
do
what I was proposing.

-Kirk


On Fri, Jul 29, 2016 at 3:41 PM, Bruce Schuchardt 
One of the problems with these tests is that they will choose a random
port for a Cache Server or some other component and only use the port
after
opening a cache.  Doing that allows the communications/membership
component
to grab two ports. AvailablePort restricts the ports it hands out to the
range [2, 3], so if we restrict the communications/membership
component to use ports outside of that range it will help avoid
collisions.


Le 7/29/2016 à 3:23 PM, Nabarun Nag a écrit :

+1 for the retry.

In my opinion, maintaining available port lists maybe hard as we move
towards running test modules in parallel. Also maybe some non-geode
entity
may come up and pick up a port hence we will need to constantly
refresh/update the list before/after each test run. (1 ports needs
to
be checked as per geode getRandomWildcardBindPortNumber)


Also for GEODE-1600 fix, DUnitLauncher now passes 0 as the port number
while creating a locator. The system assigns it an available port number
while staring the server rather than getting a random available port
number
first then asking things to be started on that port. (race conditions
ensues )

On Fri, Jul 29, 2016 at 2:36 PM William Markito 
wrote:

Why not create a JUnit rule that returns available ports and keep track
of


ports being used ?

I've cloned this gist from somewhere (don't remember now) but I've
planning
to send it for discussion...

https://gist.github.com/markito/b5be3fc570c7c7c84e6d09e064a6e898

Still talking about rules, I've played a bit with the TemporaryFolder
rule
and that's very useful as well, specially to clean up after test runs
and
to avoid conflicts.

http://junit.org/junit4/javadoc/4.12/org/junit/rules/Tempora
ryFolder.html

Just my 2c

On Fri, Jul 29, 2016 at 1:54 PM, Hitesh Khamesra <
hitesh...@yahoo.com.invalid> wrote:

Is there any possibility of running multiple test same time on that


machine?

-Hitesh


 From: Kirk Lund 
To: geode 
Sent: Friday, July 29, 2016 1:21 PM
Subject: Flaky tests failing with BindException

Many of our flaky tests are flaky because they use AvailablePort or
AvailablePortHelper to find randomly available ports. They then later

fail

with a BindException because the port is already in use by the time the

test uses it.

Here's a proposal for a temporary fix:

The module geode-junit contains a JUnit 4 rule called RetryRule. We
could
modify RetryRule to only retry if a BindException (or configurable
exception/s) is detected. This rule would then be dropped into every
test
that uses AvailablePort or AvailablePortHelper. Then if the test fails

with

a BindException, it would automatically retry (once or twice or

whatever

we

decide to configure RetryRule with). If the test fails without any

detected

BindException, then it would just fail without retrying.

Opinions on this?

Thanks,
Kirk





--

~/William

Re: IntelliJ project dependencies

2016-08-16 Thread Bruce Schuchardt

Yes, that's what I did as well

Le mardi 16 août 2016, Kirk Lund  a écrit :

> All I know of is upgrading Gradle version to 2.14.1 on Aug 2.
>
> I just built a new IntelliJ project for Geode myself without any problems.
> I don't use the "gradle idea" target (is that what you're using?). That
> produces a project that doesn't work for me. Instead I create a new
> "Project from Existing Sources" like this:
>
> 1) File | New | "Project from Existing Sources..."
> 2) select your Geode checkout and hit OK
> 3) select "Import project from external model", select Gradle and hit Next
>
> -Kirk
>
>
> On Tue, Aug 16, 2016 at 10:37 AM, Bruce Schuchardt  >
> wrote:
>
> > Has something changed in project dependencies recently?
> >
> > I rebuilt my IntelliJ project last Friday and found that there were
> > missing dependencies that broke my build.  The modules geode-web_test,
> > geode-cq_test, geode-lucene_test, etc, did not have a dependency on
> > geode-core_test.  That gave IntelliJ all sorts of grief until I manually
> > added them.
> >
> >
>

Re: IntelliJ project dependencies

2016-08-16 Thread Bruce Schuchardt

I'm not sure what I had this morning but I'm now using 2016.2.2 
community edition with the same JDK you're using.  Maybe it's a problem 
unique to MS-Windows.



Le 8/16/2016 à 3:43 PM, Kirk Lund a écrit :

The project I'm using was created/imported today. My modules view for
geode-core shows the tools.jar in the list of the Project Settings
| Modules | Dependencies for geode-core:

geode-core_jca
geode-core_legacyDUnit
geode-core_main
geode-core_test

Here's my IntelliJ version and info (on Mac) according to the About:

IntelliJ IDEA 2016.1.2
Build #IC-145.971, built on April 29, 2016
JRE: 1.8.0_76-release-b162 x86_64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o

My Project Settings | Modules | Dependencies also shows that I'm using
"jdk1.8.0_66" -- I'm not sure why that's different from the info in the
About window.

-Kirk


On Tue, Aug 16, 2016 at 3:28 PM, Bruce Schuchardt 
wrote:


After pulling down the latest patches to IntelliJ this worked for me as
well, though it still doesn't configure geode-core to require tools.jar.
Thanks Kirk


Le 8/16/2016 à 12:21 PM, Kirk Lund a écrit :


All I know of is upgrading Gradle version to 2.14.1 on Aug 2.

I just built a new IntelliJ project for Geode myself without any problems.
I don't use the "gradle idea" target (is that what you're using?). That
produces a project that doesn't work for me. Instead I create a new
"Project from Existing Sources" like this:

1) File | New | "Project from Existing Sources..."
2) select your Geode checkout and hit OK
3) select "Import project from external model", select Gradle and hit Next

-Kirk


On Tue, Aug 16, 2016 at 10:37 AM, Bruce Schuchardt <
bschucha...@pivotal.io>
wrote:

Has something changed in project dependencies recently?

I rebuilt my IntelliJ project last Friday and found that there were
missing dependencies that broke my build.  The modules geode-web_test,
geode-cq_test, geode-lucene_test, etc, did not have a dependency on
geode-core_test.  That gave IntelliJ all sorts of grief until I manually
added them.

Re: IntelliJ project dependencies

2016-08-16 Thread Bruce Schuchardt

After pulling down the latest patches to IntelliJ this worked for me as 
well, though it still doesn't configure geode-core to require 
tools.jar.  Thanks Kirk


Le 8/16/2016 à 12:21 PM, Kirk Lund a écrit :

All I know of is upgrading Gradle version to 2.14.1 on Aug 2.

I just built a new IntelliJ project for Geode myself without any problems.
I don't use the "gradle idea" target (is that what you're using?). That
produces a project that doesn't work for me. Instead I create a new
"Project from Existing Sources" like this:

1) File | New | "Project from Existing Sources..."
2) select your Geode checkout and hit OK
3) select "Import project from external model", select Gradle and hit Next

-Kirk


On Tue, Aug 16, 2016 at 10:37 AM, Bruce Schuchardt 
wrote:


Has something changed in project dependencies recently?

I rebuilt my IntelliJ project last Friday and found that there were
missing dependencies that broke my build.  The modules geode-web_test,
geode-cq_test, geode-lucene_test, etc, did not have a dependency on
geode-core_test.  That gave IntelliJ all sorts of grief until I manually
added them.

IntelliJ project dependencies

2016-08-16 Thread Bruce Schuchardt


Has something changed in project dependencies recently?

I rebuilt my IntelliJ project last Friday and found that there were 
missing dependencies that broke my build.  The modules geode-web_test, 
geode-cq_test, geode-lucene_test, etc, did not have a dependency on 
geode-core_test.  That gave IntelliJ all sorts of grief until I manually 
added them.

Re: Revised SSL properties failure scenario advice.

2016-08-10 Thread Bruce Schuchardt


+1 for ssl-default-alias and failing

Le 8/10/2016 à 3:36 PM, Udo Kohlmeyer a écrit :

Hi there guys,

As per the proposal for the revision of the SSL configuration 
, 
I'm nearing the completion of this feature.


What I have come across is some scenarios where a system is configured 
with a multi-key keystore and all Geode components are marked to use 
SSL. As the ssl configuration factory would not know what key to use, 
it might fail to correctly configure SSL comms.


In this scenario, would it make sense to introduce another property 
"ssl-default-alias" which specifies the default certificate alias to 
be used in a multi-key keystore?


Also, in the scenario where a single component specifies a different 
alias to be used, should we fail if the "ssl-default-alias" has not 
been set for a multi-key keystore?


Any advice or opinions would be appreciated.

--Udo

Re: Review Request 49962: Removed extra fields from distributedmember while serialization/de

2016-08-10 Thread Bruce Schuchardt


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



It looks good but you need to change the GMSMember reference in JoinLeave to 
NetMember so that we don't tie this interface to the implementation classes.

Fix it, then ship it!

- Bruce Schuchardt


On Aug. 10, 2016, 9:25 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> ---
> 
> (Updated Aug. 10, 2016, 9:25 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Removed extra fields from distributedmember while serialization/de. Planning 
> to do this in udp-security branch.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java
>  d5d0b8e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/JoinLeave.java
>  87409c5 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  080bdb3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  5c0a327 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java
>  7eef594 
> 
> Diff: https://reviews.apache.org/r/49962/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 49962: Removed extra fields from distributedmember while serialization/de

2016-08-08 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On août 5, 2016, 4:52 après-midi, Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49962/
> ---
> 
> (Updated août 5, 2016, 4:52 après-midi)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Removed extra fields from distributedmember while serialization/de. Planning 
> to do this in udp-security branch.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/InternalDistributedMember.java
>  067b71b 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java
>  d5d0b8e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  5c0a327 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMemberJUnitTest.java
>  7eef594 
> 
> Diff: https://reviews.apache.org/r/49962/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Review Request 50856: GEODE-225 excessive CPU utilization and garbage collection strain for JSON processing

2016-08-05 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.


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


Repository: geode


Description
---

Use of a static ObjectMapper can be enabled by setting the system property

PdxInstance.use-static-mapper=true

We can switch to always using a static ObjectMapper once multithreaded 
performance is determined to not be affected.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/pdx/internal/PdxInstanceImpl.java 
d759c49155f2c098b0d7ad48129045c4eb819ea8 

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


Testing
---

precheckin


Thanks,

Bruce Schuchardt

Re: Flaky tests failing with BindException

2016-08-04 Thread Bruce Schuchardt

I've pushed the port-range changes that I described in my last email on 
this subject.


Le 8/1/2016 à 5:33 PM, Kirk Lund a écrit :

I think that the changes mentioned by Jens and Bruce obviate the need to do
what I was proposing.

-Kirk


On Fri, Jul 29, 2016 at 3:41 PM, Bruce Schuchardt 
wrote:


I'm making another change that will help.

One of the problems with these tests is that they will choose a random
port for a Cache Server or some other component and only use the port after
opening a cache.  Doing that allows the communications/membership component
to grab two ports. AvailablePort restricts the ports it hands out to the
range [2, 3], so if we restrict the communications/membership
component to use ports outside of that range it will help avoid collisions.


Le 7/29/2016 à 3:23 PM, Nabarun Nag a écrit :


+1 for the retry.

In my opinion, maintaining available port lists maybe hard as we move
towards running test modules in parallel. Also maybe some non-geode entity
may come up and pick up a port hence we will need to constantly
refresh/update the list before/after each test run. (1 ports needs to
be checked as per geode getRandomWildcardBindPortNumber)


Also for GEODE-1600 fix, DUnitLauncher now passes 0 as the port number
while creating a locator. The system assigns it an available port number
while staring the server rather than getting a random available port
number
first then asking things to be started on that port. (race conditions
ensues )

On Fri, Jul 29, 2016 at 2:36 PM William Markito 
wrote:

Why not create a JUnit rule that returns available ports and keep track of

ports being used ?

I've cloned this gist from somewhere (don't remember now) but I've
planning
to send it for discussion...

https://gist.github.com/markito/b5be3fc570c7c7c84e6d09e064a6e898

Still talking about rules, I've played a bit with the TemporaryFolder
rule
and that's very useful as well, specially to clean up after test runs and
to avoid conflicts.

http://junit.org/junit4/javadoc/4.12/org/junit/rules/TemporaryFolder.html

Just my 2c

On Fri, Jul 29, 2016 at 1:54 PM, Hitesh Khamesra <
hitesh...@yahoo.com.invalid> wrote:

Is there any possibility of running multiple test same time on that

machine?

-Hitesh


From: Kirk Lund 
   To: geode 
   Sent: Friday, July 29, 2016 1:21 PM
   Subject: Flaky tests failing with BindException

Many of our flaky tests are flaky because they use AvailablePort or
AvailablePortHelper to find randomly available ports. They then later


fail


with a BindException because the port is already in use by the time the
test uses it.

Here's a proposal for a temporary fix:

The module geode-junit contains a JUnit 4 rule called RetryRule. We
could
modify RetryRule to only retry if a BindException (or configurable
exception/s) is detected. This rule would then be dropped into every
test
that uses AvailablePort or AvailablePortHelper. Then if the test fails


with


a BindException, it would automatically retry (once or twice or whatever


we


decide to configure RetryRule with). If the test fails without any


detected


BindException, then it would just fail without retrying.

Opinions on this?

Thanks,
Kirk






--

~/William

Review Request 50798: GEODE-1727 NPE in JGroups during shutdown

2016-08-04 Thread Bruce Schuchardt


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

Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.


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


Repository: geode


Description
---

We added a fix for shutting down the transport protocol's Timer but it was 
being done after stopping the protocol.  JGroups shutdown code has always 
tended toward nulling out instance variables for some reason and this lead to 
an NPE when a timer task fired and tried to retransmit a message using the null 
TP.bundler variable.


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/Transport.java
 2ba319de69df0e82f996c212adbf6ae200e36fe8 

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


Testing
---


Thanks,

Bruce Schuchardt

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

2016-08-03 Thread Bruce Schuchardt

The port-range test failure is due to my checkin yesterday.  I'm looking 
into it.


Le 8/3/2016 à 7:28 AM, Apache Jenkins Server a écrit :

See 

Changes:

[jdeppe] GEODE-1666: Bump Gradle from 2.12 to 2.14.1

[klund] GEODE-1682: Adding options for starting Geode REST API using gfsh

[klund] GEODE-1682: Added test for the REST arguments and Use correct

[klund] GEODE-1682: Fixing test failure HelpCommandsIntegrationTest >

[bschuchardt] restricting membership-port-range for tests to be outside of

[bschuchardt] GC thread names have changed.  This fixes the expected names in

[bschuchardt] GEODE-1645 forceUDPMessagingForCurrentThread should be re-enabled

--
[...truncated 566 lines...]
:geode-common:build
:geode-common:distributedTest
:geode-common:flakyTest
:geode-common:integrationTest
:geode-core:assemble
:geode-core:checkMissedTests
:geode-core:test
:geode-core:check
:geode-core:build
:geode-core:distributedTest

com.gemstone.gemfire.distributed.DistributedSystemDUnitTest > 
testConflictingUDPPort FAILED
 java.lang.IllegalArgumentException: Could not set "membership-port-range" to 
"21,017" because its value can not be less than "32,769".
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.minMaxCheck(AbstractDistributionConfig.java:100)
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.checkMembershipPortRange(AbstractDistributionConfig.java:383)
 at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
 at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
 at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 at java.lang.reflect.Method.invoke(Method.java:498)
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.checkAttribute(AbstractDistributionConfig.java:84)
 at 
com.gemstone.gemfire.distributed.internal.DistributionConfigImpl.setMembershipPortRange(DistributionConfigImpl.java:3172)
 at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
 at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
 at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 at java.lang.reflect.Method.invoke(Method.java:498)
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.setAttributeObject(AbstractDistributionConfig.java:564)
 at 
com.gemstone.gemfire.internal.AbstractConfig.setAttribute(AbstractConfig.java:394)
 at 
com.gemstone.gemfire.distributed.internal.DistributionConfigImpl.initialize(DistributionConfigImpl.java:1322)
 at 
com.gemstone.gemfire.distributed.internal.DistributionConfigImpl.(DistributionConfigImpl.java:734)
 at 
com.gemstone.gemfire.distributed.internal.DistributionConfigImpl.(DistributionConfigImpl.java:623)
 at 
com.gemstone.gemfire.distributed.internal.InternalDistributedSystem.(InternalDistributedSystem.java:409)
 at 
com.gemstone.gemfire.distributed.internal.InternalDistributedSystem.newInstance(InternalDistributedSystem.java:240)
 at 
com.gemstone.gemfire.distributed.DistributedSystem.connect(DistributedSystem.java:238)
 at 
com.gemstone.gemfire.distributed.DistributedSystemDUnitTest.testConflictingUDPPort(DistributedSystemDUnitTest.java:368)

com.gemstone.gemfire.distributed.DistributedSystemDUnitTest > testUDPPortRange 
FAILED
 java.lang.IllegalArgumentException: Could not set "membership-port-range" to 
"20,046" because its value can not be less than "32,769".
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.minMaxCheck(AbstractDistributionConfig.java:100)
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.checkMembershipPortRange(AbstractDistributionConfig.java:383)
 at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
 at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
 at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 at java.lang.reflect.Method.invoke(Method.java:498)
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistributionConfig.checkAttribute(AbstractDistributionConfig.java:84)
 at 
com.gemstone.gemfire.distributed.internal.DistributionConfigImpl.setMembershipPortRange(DistributionConfigImpl.java:3172)
 at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
 at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
 at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 at java.lang.reflect.Method.invoke(Method.java:498)
 at 
com.gemstone.gemfire.distributed.internal.AbstractDistri

Re: Flaky tests failing with BindException

2016-07-29 Thread Bruce Schuchardt


I'm making another change that will help.

One of the problems with these tests is that they will choose a random 
port for a Cache Server or some other component and only use the port 
after opening a cache.  Doing that allows the communications/membership 
component to grab two ports. AvailablePort restricts the ports it hands 
out to the range [2, 3], so if we restrict the 
communications/membership component to use ports outside of that range 
it will help avoid collisions.


Le 7/29/2016 à 3:23 PM, Nabarun Nag a écrit :

+1 for the retry.

In my opinion, maintaining available port lists maybe hard as we move
towards running test modules in parallel. Also maybe some non-geode entity
may come up and pick up a port hence we will need to constantly
refresh/update the list before/after each test run. (1 ports needs to
be checked as per geode getRandomWildcardBindPortNumber)


Also for GEODE-1600 fix, DUnitLauncher now passes 0 as the port number
while creating a locator. The system assigns it an available port number
while staring the server rather than getting a random available port number
first then asking things to be started on that port. (race conditions
ensues )

On Fri, Jul 29, 2016 at 2:36 PM William Markito  wrote:


Why not create a JUnit rule that returns available ports and keep track of
ports being used ?

I've cloned this gist from somewhere (don't remember now) but I've planning
to send it for discussion...

https://gist.github.com/markito/b5be3fc570c7c7c84e6d09e064a6e898

Still talking about rules, I've played a bit with the TemporaryFolder rule
and that's very useful as well, specially to clean up after test runs and
to avoid conflicts.

http://junit.org/junit4/javadoc/4.12/org/junit/rules/TemporaryFolder.html

Just my 2c

On Fri, Jul 29, 2016 at 1:54 PM, Hitesh Khamesra <
hitesh...@yahoo.com.invalid> wrote:


Is there any possibility of running multiple test same time on that
machine?

-Hitesh


   From: Kirk Lund 
  To: geode 
  Sent: Friday, July 29, 2016 1:21 PM
  Subject: Flaky tests failing with BindException

Many of our flaky tests are flaky because they use AvailablePort or
AvailablePortHelper to find randomly available ports. They then later

fail

with a BindException because the port is already in use by the time the
test uses it.

Here's a proposal for a temporary fix:

The module geode-junit contains a JUnit 4 rule called RetryRule. We could
modify RetryRule to only retry if a BindException (or configurable
exception/s) is detected. This rule would then be dropped into every test
that uses AvailablePort or AvailablePortHelper. Then if the test fails

with

a BindException, it would automatically retry (once or twice or whatever

we

decide to configure RetryRule with). If the test fails without any

detected

BindException, then it would just fail without retrying.

Opinions on this?

Thanks,
Kirk







--

~/William

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

2016-07-29 Thread Bruce Schuchardt


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




geode-core/src/main/java/com/gemstone/gemfire/internal/cache/BucketRegion.java 
(line 425)
<https://reviews.apache.org/r/50587/#comment210136>

This "if" needs braces



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HARegionQueue.java
 (line 1604)
<https://reviews.apache.org/r/50587/#comment210137>

remove this TODO



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HARegionQueue.java
 (line 3722)
<https://reviews.apache.org/r/50587/#comment210139>

I wish this code wasn't duplicated in AbstractRegionMap.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/LockObject.java
 (line 23)
<https://reviews.apache.org/r/50587/#comment210142>

Since this is never decremented it could be a boolean instead of an int.  
It looks like you started coding this & thought you would decrement the 
variable but then decided you didn't need to.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java
 (line 341)
<https://reviews.apache.org/r/50587/#comment210150>

I know you're just merging stuff that other people did but this code really 
should be in serializeAndAddPart instead of addObjPart.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java
 (line 190)
<https://reviews.apache.org/r/50587/#comment210151>

Remove customer name from this comment.  I think they were all changed to 
"performance"



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java
 (line 191)
<https://reviews.apache.org/r/50587/#comment210155>

We should change this to limit it to 16-bit integers.  I don't see any uses 
of it that would cause memory bloat in the current code base but that could 
change in the future.


- Bruce Schuchardt


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

geode-core distributedTest statistics

2016-07-29 Thread Bruce Schuchardt


Here are some stats on the run-time of our geode-core distributedTests.

There are 7431 geode-core distributedTests.  On my last run these took 
280 minutes to execute.


5% of the tests run for over 10 seconds and are taking 50% of the time 
(140 minutes).


2.5% run for over 20 seconds and are taking 33% of the time (92 minutes).

0.8% run for over 30 seconds and are taking 20% of the time (55 minutes).

ClientAuthorizationDUnitTest.testAllOpsWithFailover() runs for 7 minutes 
and wins the prize for longest-running test.  The runner-up trails far 
behind at 2.1 minutes.
 10034 ms 
com.gemstone.gemfire.internal.cache.wan.asyncqueue.AsyncEventListenerOffHeapDUnitTest
 testParallelAsyncEventQueueWithConflationEnabled
 10214 ms com.gemstone.gemfire.management.LocatorManagementDUnitTest 
testWillingManagersWithPortZero
 10321 ms com.gemstone.gemfire.internal.cache.IncrementalBackupDUnitTest 
testIncompleteInBaseline
 10362 ms com.gemstone.gemfire.cache30.SearchAndLoadDUnitTest testConcurrentLoad
 10444 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.DurableClientBug39997DUnitTest 
testNoServerAvailableOnStartup
 10460 ms com.gemstone.gemfire.management.internal.security.MultiUserDUnitTest 
testMultiUser
 10484 ms 
com.gemstone.gemfire.cache.query.internal.index.ConcurrentIndexOperationsOnOverflowRegionDUnitTest
 testAsyncIndexInitDuringEntryDestroyAndQueryOnPR
 10494 ms 
com.gemstone.gemfire.cache.query.internal.index.ConcurrentIndexOperationsOnOverflowRegionDUnitTest
 testAsyncIndexInitDuringEntryDestroyAndQueryOnPersistentRR
 10499 ms 
com.gemstone.gemfire.cache.query.internal.index.ConcurrentIndexOperationsOnOverflowRegionDUnitTest
 testAsyncIndexInitDuringEntryDestroyAndQueryOnRR
 10507 ms com.gemstone.gemfire.cache.query.dunit.QueryUsingPoolDUnitTest 
testClientServerCompiledQueryTimeBasedCleanup
 10639 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.InterestListDUnitTest 
testInterestListRegistration_ALL_KEYS
 10643 ms com.gemstone.gemfire.pdx.PdxTypeExportDUnitTest testPeer
 10663 ms com.gemstone.gemfire.distributed.LocatorDUnitTest 
testLeadFailureAndCoordShutdown
 10667 ms 
com.gemstone.gemfire.cache.query.internal.index.ConcurrentIndexOperationsOnOverflowRegionDUnitTest
 testAsyncIndexInitDuringEntryDestroyAndQueryOnPersistentPR
 10671 ms 
com.gemstone.gemfire.internal.cache.partitioned.StreamingPartitionOperationManyDUnitTest
 testStreamingManyProvidersNoExceptions
 10672 ms com.gemstone.gemfire.internal.cache.PartitionedRegionQueryDUnitTest 
testPartitionRegionDebugMessageQueryTraceOnRemoteServerOnly
 10681 ms com.gemstone.gemfire.cache30.ClientMembershipSelectorDUnitTest 
testClientMembershipEventsInClient
 10724 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.InstantiatorPropagationDUnitTest
 testInstantiatorsWith2ClientsN2Servers
 10747 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.CacheServerTransactionsDUnitTest
 testInvalidatesOneServerToClientTransactionsPropagation
 10785 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.CacheServerTransactionsSelectorDUnitTest
 testInvalidatesOneServerToClientTransactionsPropagation
 10787 ms 
com.gemstone.gemfire.cache.client.internal.LocatorLoadBalancingDUnitTest 
testLoadMessaging
 10789 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.CacheServerTransactionsDUnitTest
 testDestroysOneServerToClientTransactionsPropagation
 10816 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.CacheServerTransactionsSelectorDUnitTest
 testDestroysOneServerToClientTransactionsPropagation
 10820 ms 
com.gemstone.gemfire.distributed.internal.DistributionManagerDUnitTest 
testKickOutSickMember
 10942 ms 
com.gemstone.gemfire.cache30.DistributedAckOverflowRegionCCEOffHeapDUnitTest 
testConcurrentEvents
 10976 ms 
com.gemstone.gemfire.internal.cache.persistence.PersistentRecoveryOrderDUnitTest
 testGIIDuringDestroy
 10996 ms com.gemstone.gemfire.cache30.DistributedAckOverflowRegionCCEDUnitTest 
testConcurrentEvents
 10997 ms com.gemstone.gemfire.cache30.GlobalRegionCCEDUnitTest 
testConcurrentEvents
 11011 ms 
com.gemstone.gemfire.internal.cache.persistence.PersistentRecoveryOrderOldConfigDUnitTest
 testGIIDuringDestroy
 11019 ms com.gemstone.gemfire.cache30.DistributedAckRegionCCEOffHeapDUnitTest 
testConcurrentEvents
 11046 ms 
com.gemstone.gemfire.management.internal.configuration.SharedConfigurationUsingDirDUnitTest
 updateClusterConfigDirWithTwoLocatorsAndRollingServerRestart
 11127 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.InterestListEndpointDUnitTest 
testDirectPutOnServer
 11142 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.InterestListEndpointDUnitTest 
testUpdaterThreadIsAliveForFailedEndPoint
 11143 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.InterestListEndpointSelectorDUnitTest
 testDirectPutOnServer
 11146 ms com.gemstone.gemfire.cache30.GlobalRegionCCEOffHeapDUnitTest 
testConcurrentEvents
 11146 ms 
com.gemstone.gemfire.internal.cache.tier.sockets.InterestListEndpointDUnitTest 
testInterestListEndpo

Re: Review Request 50607: Update of cq stats causing disk read

2016-07-29 Thread Bruce Schuchardt


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


Ship it!




Ship It!

- Bruce Schuchardt


On July 29, 2016, 4:58 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50607/
> ---
> 
> (Updated July 29, 2016, 4:58 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Bruce Schuchardt, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This test verifies perf of durable client -queue with overflow. In case of 
> overflow client queue uses LIFO policy. Because of that we put new entry on 
> disk first. But, after putting that entry into queue that thread needs to 
> update Cq stats, which causes that thread to fetch that value again from 
> disk. that hurts performance. we should be updating any(all) client stats 
> before putting that value into disk.
> 
> Using "hw.getClientUpdateMessage()" to update the stats..
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ha/HARegionQueue.java
>  c5746ed 
> 
> Diff: https://reviews.apache.org/r/50607/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

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

2016-07-28 Thread Bruce Schuchardt


commit 4c5a699490b2702d6abe3a40e5d74a7e2f6b5acf
Author: Bruce Schuchardt 
Date:   Tue Jul 26 13:49:00 2016 -0700

fixes flakyTest target to allow it to run Rest API tests

diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index fdd6e25..499eca2 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -387,6 +387,7 @@ def dependOnInstalledProduct = {
 test dependOnInstalledProduct
 distributedTest dependOnInstalledProduct
 integrationTest dependOnInstalledProduct
+flakyTest dependOnInstalledProduct

 // Make build final task to generate all test and product resources
 build.dependsOn installDist


Le 7/28/2016 à 1:22 PM, William Markito a écrit :

My assumption here was that flaky would not be preventing the release, but
indeed it's worth double checking, specially if there is something we could
do in the build pipeline to fix it...

On Thu, Jul 28, 2016 at 12:02 PM, Dan Smith  wrote:


Bruce pointed out that there is a consistently failing test on this release
branch. I'm not sure if this should hold things up or not since this is
marked as a "flaky" test.

:*geode-assembly:flakyTest*


com.gemstone.gemfire.rest.internal.web.controllers.RestAPIsOnMembersFunctionExecutionDUnitTest

testFunctionExecutionOnAllMembers FAILED

 java.lang.AssertionError: unable to locate geode-web-api WAR file

 java.lang.NullPointerException



On Wed, Jul 27, 2016 at 12:28 PM, William Markito 
wrote:


All,

This is the sixth release candidate Apache Geode, version
1.0.0-incubating.M3.

We're including the feedback received in RC5 fixing a classpath issue on
'gfsh' caused by wrong build arguments.

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&version=12335358

*** Please download, test and vote by Monday, August 1, 0800 hrs US
Pacific.

Note that we are voting upon the source (tag):
rel/v1.0.0-incubating.M3.RC6




https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;a=tag;h=refs/tags/rel/v1.0.0-incubating.M3.RC6

Commit ID: c70efdff8fda585d5e0aa6ab0229cac6f1757d59




https://git-wip-us.apache.org/repos/asf?p=incubator-geode.git;a=commit;h=c70efdff8fda585d5e0aa6ab0229cac6f1757d59

Source and binary files:



https://dist.apache.org/repos/dist/dev/incubator/geode/1.0.0-incubating.M3.RC6/

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

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

1 2 3 4 5 >

1 - 100 of 472 matches

Mail list logo