Re: Where should geode example code live

[Udo Kohlmeyer]

+1


On 11/11/16 9:13 am, Jacob Barrett wrote:

+1

On Thu, Nov 10, 2016 at 11:22 AM Swapnil Bawaskar 
wrote:


+1

On Thu, Nov 10, 2016 at 11:12 AM, Kirk Lund  wrote:


Yep! Replace "gemfire" with "geode"

On Thu, Nov 10, 2016 at 11:07 AM, Kevin Duling 

wrote:

+1

Shouldn't that be org.apache.geode.security.examples?

On Thu, Nov 10, 2016 at 11:03 AM, Jinmei Liao 

wrote:

+1 for both renaming it now and moving it to a separate module going
forward.

On Thu, Nov 10, 2016 at 10:58 AM, Kirk Lund 

wrote:

We have some example code that's not in geode-examples.
geode-core-1.0.0-incubating.jar currently contains this package

with

examples:

org.geode.gemfire.security.templates

Given that template means something a bit different from example,

I'd

like

to rename this package to examples. Is this acceptable?

Going forward, this community needs to spend some considerable

effort

in

improving modularity of geode. The geode-core module should contain

only

the bare essentials that are always necessary when using geode.

Optional

features and the like should move to their own modules. Along the

same

line

of thought, examples should move to geode-examples or some similar
sub-module (or even a separate repo). That sub-module should

generate a

jar

that can be added to the classpath for purposes of demoing or

trying

out

geode (kicking the tires). I'd like the community to come up with
additional ideas on how to facilitate demoing and also make it

easier

in

general to try out geode for the first time.

-Kirk




--
Cheers

Jinmei

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


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 53652: GEODE-2089 entry-idle-time setting on the client side cache is not working as expected

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Nov. 10, 2016, 9: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, 9: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
> 
>

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

[Udo Kohlmeyer]

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




geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
 (lines 170 - 172)
<https://reviews.apache.org/r/53652/#comment225653>

I think this overloaded method is a little confusing. It is setting the 
lastModified but takes lastAccessed as well. Then we only use lastModified.



geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
 (lines 178 - 180)
<https://reviews.apache.org/r/53652/#comment225654>

maybe we use setLastAccessed here instead of the overloaded setLastModified



geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
 (lines 178 - 180)
<https://reviews.apache.org/r/53652/#comment225656>

I think this implementation should read

```setLastModified(lastModified);
setLastAccessed(lastAccessed);``` 

Then the VMStatsDiskLRURegionEntryHeapIntKey can override 
`updateStatsForPut` with its custom implementation.



geode-core/src/main/java/org/apache/geode/internal/cache/VMStatsDiskLRURegionEntryHeapIntKey.java
 (lines 306 - 312)
<https://reviews.apache.org/r/53652/#comment225655>

maybe the !DISABLE_ACCESS_TIME_UPDATE_ON_PUT logic should reside in the 
updateStatsForPut method, or at least override the normal behaviour from 
AbstractRegionEntry.


- Udo Kohlmeyer


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

Re: Spring Data Geode 1.0.0-INCUBATING-RELEASE available!

[Udo Kohlmeyer]

Nice one John!!

I look forward to seeing the Spring Security integration as well.

--Udo


On 11/11/16 5:15 am, John Blum wrote:
I am happy to announce the release of /Spring Data/ for /Apache 
Geode/1.0.0-incubating, which marks the first, official GA release of 
/Apache Geode/ as well as the first GA release of SDG to match.


One of the key themes of this release is *security*.

Currently, /Apache Geode/ builds on /Apache Shiro/, but my intention 
is to integrate /Apache Geode/ with /Spring Security/ as well.  I make 
it extremely simple to configure security correctly for /Apache 
Geode/ using SDG's new Annotation configuration model and support.


https://spring.io/blog/2016/11/10/spring-data-geode-1-0-0-incubating-release-released 

https://twitter.com/john_blum/status/796777167287787520 



Cheers!
John

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

[Udo Kohlmeyer]

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




geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
(lines 3269 - 3284)
<https://reviews.apache.org/r/53557/#comment225104>

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.


- Udo Kohlmeyer


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: more spotless problems on Windows

[Udo Kohlmeyer]

@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 
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: more spotless problems on Windows

[Udo Kohlmeyer]

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 


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.

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

[Udo Kohlmeyer]

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

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: Review Request 53388: GEODE-2059 client SSL handshake attempts do not time out

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Nov. 2, 2016, 4:57 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53388/
> ---
> 
> (Updated Nov. 2, 2016, 4:57 p.m.)
> 
> 
> 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: jmh benchmarks

[Udo Kohlmeyer]

+2


On 3/11/16 5:17 am, Dan Smith wrote:

Hi all,

I'd like to add some support for running benchmarks with jmh to geode. Is
this something we're interested in having? JMH is a framework for easily
writing microbenchmarks. It's probably not that useful for large scale
multiple member benchmarks, but it can help us benchmark and optimize
smaller pieces of code.

You can look at this review request for what I want to change and a couple
of example benchmarks:

https://reviews.apache.org/r/53395/

-Dan

Re: Tweaking the IntelliJ and Eclipse formatters

[Udo Kohlmeyer]

1) 0
2) +1
3) +1
4) 0

On 27/10/16 3:11 pm, Kirk Lund wrote:

I'd like to propose making a few changes to our IntelliJ and Eclipse
formatters as well as the Eclipse importorder (all in etc/):

1) increase max line length (100 is way too short)
2) make (hopefully minor) changes to make the two formatters more
consistent with each other
3) change Eclipse importorder to follow the google style as closely as
possible
4) update the import ordering in IntelliJ formatter to match Eclipse

The goal is to make both formatters produce the exact same results
including ordering of imports while maintaining them to be as close to the
google style as possible. Right now if you run IntelliJ formatter, the
result will fail the spotlessCheck. We may have to make some small
compromises in our adherence to the google style but I think that's
reasonable in order to get the formatters both working consistently.

The gradle spotless tasks currently use the Eclipse formatter. One further
change would be to add the Eclipse importorder file for spotless to use.

-Kirk

Re: Code Formatting in develop

[Udo Kohlmeyer]

Kevin,

Could you please get latest dev. Barry has fixed.

--Udo


On 27/10/16 10:41 am, Kevin Duling wrote:

I just submitted a PR to geode that Travis CI rejected because of code
formatting in classes I had not modified.  Prior to my commit, I'd merged
in the latest develop branch this morning, which picked this up.

The error message reads:


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

geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelQueueRemovalMessage.java

geode-core/src/test/java/org/apache/geode/internal/cache/wan/parallel/ParallelQueueRemovalMessageJUnitTest.java

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


My pull request https://github.com/apache/incubator-geode/pull/273 is held
up until this is corrected.

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Oct. 26, 2016, 6:30 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53199/
> ---
> 
> (Updated Oct. 26, 2016, 6:30 p.m.)
> 
> 
> 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: Review Request 53150: GEODE-2024 Deadlock creating a new distributed lock service Grantor while transactions are in progress

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Oct. 25, 2016, 4:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53150/
> ---
> 
> (Updated Oct. 25, 2016, 4:13 p.m.)
> 
> 
> 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: [DISCUSS] Graduation

[Udo Kohlmeyer]

+1


On 25/10/16 5:43 pm, Joey McAllister wrote:

+1

On Tue, Oct 25, 2016 at 5:42 PM Anthony Baker  wrote:


+1


On Oct 25, 2016, at 5:25 PM, Roman Shaposhnik  wrote:

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

Re: Coding practices/standards

[Udo Kohlmeyer]

+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

investigated and fixed before I can open a pull request to

enable

spotless.

On Oct 14, 2016, at 4:57 PM, Dan Smith 
wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <

jstew...@pivotal.io

wrote:

I agree that the formatter needs fixing up.  Our wiki <
https://cwiki.apache.org/confluence/display/GEOD

Re: Coding practices/standards

[Udo Kohlmeyer]

+1


On 21/10/16 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

investigated and fixed before I can open a pull request to

enable

spotless.

On Oct 14, 2016, at 4:57 PM, Dan Smith 

wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <

jstew...@pivotal.io

wrote:

I agree that the formatter needs fixing up.  Our wiki <
https://cwiki.apache.org/confluence/display/GEODE/Code+

Style+Guide>

says

that we follow the Google Java Style guide, but that is not

actually

what’s

in our formatter templates.  I pushed a new branch <

https://github.com/

jaredjstewart/incubator-geode/tree/

spotlessPluginGoogleStyle>

that

points

spotless at the actual Google Java Style template, and I

think

it

makes

both of the examples you found look better.


On Oct 13, 2016, at 10:11 AM, Dan Smith 

wrote:

+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit

before

reformatting

the code. I tried running spotlessApply, and it did some

unfortunate

reformatting of code to make it less readable.

One problem is with method chaining. We have a few different

factory

APIs

that encourage method chaining, and it put all the method

calls

on

a

single

line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.

getHost()),

port)

- .set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+ClientCacheFactory ccf = new
ClientCacheFactory().addPoolServer(NetworkUtil

Re: C++ Coding Standards

[Udo Kohlmeyer]

+1 - I'm not a C++ dev but I'm all for consistency.


On 21/10/16 8:43 am, Jacob Barrett wrote:

Considering the team just formally adopted some standards and enforcement
for the Java sources I would like to open the discussion for formally
adopting similar standards for the C++ sources.

I propose that we use the Google C++ style as defined by
https://google.github.io/styleguide/cppguide.html and that such format
shall be enforced with clang-format -style=Google.

-Jake

Re: Coding practices/standards

[Udo Kohlmeyer]

olio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID desc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID desc, pkid desc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID asc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID asc , pkid desc limit 10",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID desc, pkid desc limit 10",
-
-   };
+"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc ", "SELECT   ID,

description,

createTime, pkid FROM /portfolio1 pf1 where ID > 10 order by ID

asc,

pkid

asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid asc ", "SELECT

  ID,

description, createTime, pkid FROM /portfolio1 pf1 where ID > 10

and

ID <

20 order by ID desc , pkid desc", "SELECT   ID, description,

createTime,

pkid FROM /portfolio1 pf1 where ID >= 10 and ID <= 20 order by ID

desc,

pkid asc ", "SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1 where ID >= 10 and ID <= 20 order by ID asc, pkid desc",

"SELECT

ID,

description, createTime, pkid FROM /portfolio1 pf1 where ID != 10

order

by

ID asc , pkid desc", "SELECT   ID, description, createTime, pkid

FROM

/portfolio1 pf1 where ID != 10 order by ID desc, pkid asc ",
+"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc limit 5", "SELECT   ID,
description, createTime, pkid FROM /portfolio1 pf1 where ID > 10

order

by

ID asc, pkid asc limit 5", "SELECT   ID, description, createTime,

pkid

FROM

/portfolio1 pf1 where ID > 10 and ID < 20 order by ID asc, pkid

desc

limit

5 ", "SELECT   ID, description, createTime, pkid FROM /portfolio1

pf1

where

ID > 10 and ID < 20 order by ID desc, pkid asc limit 5", "SELECT

  ID,

description, createTime, pkid FROM /portfolio1 pf1 where ID >= 10

and

ID

<=

20 order by ID desc, pkid desc limit 5", "SELECT   ID, description,
createTime, pkid FROM /portfolio1 pf1 where ID >= 10 and ID <= 20

order

by

ID asc, pkid asc limit 5", "SELECT   ID, description, createTime,

pkid

FROM

/portfolio1 pf1 where ID != 10 order by ID asc , pkid desc limit

10",

"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1

where

ID

!= 10 order by ID desc, pkid desc limit 10",
+
+};





On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart <

jstew...@pivotal.io>

wrote:

The task is fully suppressible with -x spotlessCheck.  Also, if

you

have

any formatter errors you can automatically fix them with 'gradle
spotlessApply’.


On Oct 13, 2016, at 9:40 AM, Kevin Duling 

wrote:

If we made formatting a warning, then people would probably

quickly

ignore

it.
If we made formatting an error, we need to be sure we don't get

in

to

the

situation where 's formatter is not in

agreement

with

the

build's checker.

I can live with an additional 17 seconds as well.  And Jared's

already

reduced the build time locally by 50%.  But I still want the

ability

to

suppress the check similar to -x javadoc.

On Wed, Oct 12, 2016 at 9:58 PM, William Markito <

wmark...@pivotal.io>

wrote:


This sounds really good to me as well.  +1

On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart <

jstew...@pivotal.io

wrote:


This is running locally on my laptop.  Since Spotless is only

doing

code

formatting and not any other static analysis, it already has 0

errors.

(Other than, of course, formatting not consistent with the

template.)

On Oct 12, 2016, at 4:11 PM, Kenneth Howe 

wrote:

Agree with Mark, this has to work with 0 errors before it

would

be

useful in precheckin. I think I could live with an additional

17

seconds

most of the time for running the spotlessCheck as suggested.

Jared, Is that 17 seconds running locally on your laptop or

on a

more

capable machine?

Ken


On Oct 12, 2016, at 3:39 PM, Jared Stewart <

jstew...@pivotal.io>

wrote:

If you want to try it out, I pushed a branch to my Geode repo

that

contains this change:

https://github.com/jaredjstewart/incubator-geode/

tree/spotlessPlugin

<

https://github.com/jaredjstewart/incubator-geode/

tree/spotlessPlugin

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

Re: Removal of nonSingleHopCount stat from client

[Udo Kohlmeyer]

Given the feedback received it seems we can remove this stat without 
fear of retribution.


--Udo


On 19/10/16 3:11 pm, Kirk Lund wrote:

+1

FYI: I know of one customer that uses the old "gemfire stats" command,
polling repeatedly for certain stat values written to a gfs file from a
native client as a crude form of monitoring, so some folks do make use of
client stats (we should give them a better way to monitor clients tho)


On Wed, Oct 19, 2016 at 2:56 PM, Bruce Schuchardt 
wrote:


+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

Re: Review Request 53034: GEODE-1927 backward compatibility support

[Udo Kohlmeyer]

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


Ship it!




If this is the only thing that is required to add a new category, then awesome!

- Udo Kohlmeyer


On Oct. 19, 2016, 9:16 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53034/
> ---
> 
> (Updated Oct. 19, 2016, 9:16 p.m.)
> 
> 
> 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
> 
>

Removal of nonSingleHopCount stat from client

[Udo Kohlmeyer]

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 52933: GEODE-1874: setNextNeighbor method allocates a HashSet on every p2p message received

[Udo Kohlmeyer]

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

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: GEODE-1466: geode.properties

[Udo Kohlmeyer]

If such a change is to be introduced.. maybe we call it `SYSTEM_PREFIX` 
or something more generic that we could use within the Geode.


Then we could hopefully cover many to most `gemfire` vs `geode` renaming.

But I agree with @Anthony, if we aren't 100% certain about a change then 
we should hold off until the next release. There is always tomorrow. :D


--Udo


On 14/10/16 8:05 am, Swapnil Bawaskar wrote:

How about introducing a new GEMFIRE_FILE_PREFIX attribute that will default
to "geode" while leaving GEMFIRE_PREFIX default to "gemfire"? Is this
something that will work?

On Thu, Oct 13, 2016 at 1:48 PM, Anthony Baker  wrote:


Hmmm, you would think it would be easier to change a file name :-)

I don’t think we should be pushing destabilizing changes into a release
branch.  If the changes aren’t ready now we always pick them up for the
next release.

Anthony


On Oct 13, 2016, at 1:13 PM, Kirk Lund  wrote:

I'm currently working with Jared and we have spent a few days working
on GEODE-1466. We've been trying to get geode to the point where it can
automatically search for, find and use either geode.properties or
gemfire.properties (preferring geode.properties if both are found).

We were intending to break this up into three subtasks with the hope of
including #1 in Geode 1.0.0 incubating:

1) locating geode.properties and gemfire.properties if user has not
specified a specific properties file

2) support specifying geode configuration properties with system

properties

geode. as well as gemfire.

ex: -Dgemfire.off-heap-memory-size=40g or -Dgeode.off-heap-memory-size=

40g

3) modify all other system properties in geode to support alias of

gemfire

as well as geode where the name of the system property currently contains
gemfire

ex: -Dgemfire.Query.VERBOSE=true or -Dgeode.Query.VERBOSE=true

The community is planning to create the Geode 1.0.0 incubating RC

tomorrow.

The work we have completed so far involves modifying geode to search for
both geode.properties and gemfire.properties to use whichever one is

found.

This turns out to be too complex to complete by tomorrow (send me a email
directly if you want more detailed info which mostly involves
DistributionConfig and ConfigSource).

In order to complete this in time, we need to use a different strategy.
Instead of looking for geode.properties first and then

gemfire.properties,

we are proposing the following change to DistributionConfig:

Change the GEMFIRE_PREFIX = "gemfire." constant to be configurable by a
system property and change the default to be "geode." This places a

greater

burden on the user who is migrating from GemFire to Geode but doesn't

want

to rename gemfire.properties or gemfire system properties. By default,
Geode would search for geode.properties unless the user specifies a new
system property with a different prefix ("gemfire."):

String GEMFIRE_PREFIX = PropertiesPrefix.getGemfireOrGeodePrefix();

Example of overriding this to be "gemfire.":

-DgeodePropertiesPrefix="gemfire."
or
-DgeodePropertiesPrefix="gemfire"

(we'll add the "." for you if you leave it out)

Pivotal or other vendors could also alter this prefix as they see fit.

There are 453 lines of production code that refer to this GEMFIRE_PREFIX
constant. For example, every system property that contains "gemfire." is
currently referring to the constant, so they would also be altered to be
"geode." by default. The JMX notifications also refer to GEMFIRE_PREFIX
such as: GEMFIRE_PREFIX + "distributedsystem.cache.client.joined".

Does anyone know if anything referring to GEMFIRE_PREFIX is persisted in
some way that would break if we make this change? For example, if we
persist any strings built with this constant to a diskstore, then

recovery

from that diskstore would be broken if the prefix value is "geode."

during

recovery of an old diskstore.

Also, a user could conceivably override the GEMFIRE_PREFIX in some

members

of a cluster but not others which could break things in unexpected ways.

One more note: While reviewing uses of GEMFIRE_PREFIX we noticed that
AgentUtil supports "GEMFIRE" (hardcoded) and GEMFIRE_PREFIX+".home" env
vars while all of the online docs specify setting GEMFIRE_HOME as an env
var. I suspect this is already broken (I will file a ticket), but I think
we will also be at risk of breaking additional things that may or may not
be immediately detected by precheckin tests. It's also used by

DtdResolver

for the name of a dtd: new File(GEMFIRE_PREFIX + "dtd). We'll continue
looking for unusual or risky uses of the constant.

Bottom line is making this change is higher risk than not making the
change, and there could be some fallout bugs that require fixes and
additional release candidates for 1.0.0.

Does the community feel this change is desirable for 1.0.0? Or is it

better

to leave it as "gemfire." and move GEODE-1466 to post-1.0.0?

Thanks,
Kirk

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


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

[Udo Kohlmeyer]

+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  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 
wrote:


+1 to running during the precheckin target as well as Travis CI

On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
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 
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 

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

[Udo Kohlmeyer]

+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

Re: Review Request 52725: GEODE-1801: Amended Logic to increment nonSingleHopsCount

[Udo Kohlmeyer]

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

(Updated Oct. 11, 2016, 8:37 p.m.)


Review request for geode, Bruce Schuchardt and Hitesh Khamesra.


Changes
---

Amended class changes to reduce formatting diffs


Repository: geode


Description
---

Amended the logic in the ClientMetadataService to more reliably increment the 
nonSingleHopsCount. This is done rather than amended each data Op that were to 
use it.

Seems to have vast amounts of formatting changes. 
Code that has changed:
ClientMetadataService : scheduleGetPRMetaData. line 455 and 537
GemFireCacheImpl : getClientMetadataService. line 1970 moved the return 
clientMetadataService to outside the sync block
SingleHopStatsDUnitTest: line 370 removed redundant logic. if/else code blocks 
exact same implementation


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientMetadataService.java
 c863d46 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
d166397 
  
geode-core/src/test/java/org/apache/geode/internal/cache/SingleHopStatsDUnitTest.java
 a7c7b48 

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


Testing
---

Pre-checkin started


Thanks,

Udo Kohlmeyer

Review Request 52725: GEODE-1801: Amended Logic to increment nonSingleHopsCount

[Udo Kohlmeyer]

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

Review request for geode, Bruce Schuchardt and Hitesh Khamesra.


Repository: geode


Description
---

Amended the logic in the ClientMetadataService to more reliably increment the 
nonSingleHopsCount. This is done rather than amended each data Op that were to 
use it.

Seems to have vast amounts of formatting changes. 
Code that has changed:
ClientMetadataService : scheduleGetPRMetaData. line 455 and 537
GemFireCacheImpl : getClientMetadataService. line 1970 moved the return 
clientMetadataService to outside the sync block
SingleHopStatsDUnitTest: line 370 removed redundant logic. if/else code blocks 
exact same implementation


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientMetadataService.java
 c863d46 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
d166397 
  
geode-core/src/test/java/org/apache/geode/internal/cache/SingleHopStatsDUnitTest.java
 a7c7b48 

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


Testing
---

Pre-checkin started


Thanks,

Udo Kohlmeyer

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Oct. 7, 2016, 7:13 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, 7:13 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
>  31a72c8 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableJUnitTest.java 
> 21accef 
>   geode-core/src/test/resources/jta/cachejta.xml 9a36ee6 
>   
> geode-core/src/test/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml
>  92454c4 
>   
> geode-core/src/test/resources/org/apache/geode/cache/query/functional/index-creation-with-eviction.xml
>  861bb23 
>   
> geode-core/src/test/resources/org/apache/geode/cache/query/functional/index-creation-without-eviction.xml
>  f8bdc78 
>   
> geode-core/src/test/resources/org/apache/geode/cache/query/functional/index-recovery-overflow.xml
>  4ca5e2f 
>   
> geode-core/src/test/resources/org/apache/geode/cache/query/internal/index/cachequeryindex.xml
>  ebcc0a4 
>   
> geode-core/src/test/resources/org/apache/geode/cache/query/internal/index/cachequeryindexwitherror.xml
>  caa3cc9 
>   
> geode-core/src/test/resources/org/apache/geode/cache/query/partitioned/PRIndexCreation.xml
>  4236a7d 
>   
> geode-core/src/test/resources/org/apache/geode/cache30/attributesUnordered.xml
>  aa8c3f4 
>   geode-core/src/test/resources/org/apac

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

[Udo Kohlmeyer]

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: Review Request 52524: GEODE-1927 backward compatibility support

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Oct. 4, 2016, 7:36 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52524/
> ---
> 
> (Updated Oct. 4, 2016, 7:36 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 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 52271: GEODE-1938: Big Snapshot File Read Exception via SnapshotReader API

[Udo Kohlmeyer]

> On Oct. 5, 2016, 7:58 p.m., Hitesh Khamesra wrote:
> > geode-core/src/main/java/org/apache/geode/pdx/internal/ClientTypeRegistration.java,
> >  line 269
> > <https://reviews.apache.org/r/52271/diff/3/?file=1522497#file1522497line269>
> >
> > you may want to put break statement here..As we aleady got exception

So it fails on the first exception regardless of pool... We would fail on this 
exception anyway, but only after registering the type with the other pools. I 
don't mind the registration of the type on the servers even if it were to fail.


- Udo


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


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

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

[Udo Kohlmeyer]

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


Changes
---

Updated code after testing with a large snapshot file


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

  
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

Re: CI bugs

[Udo Kohlmeyer]

+1


On 5/10/2016 6:28 AM, Nabarun Nag wrote:

Hi,

I removed the flaky tags from the test in the WAN module. I went through
the commits and made sure they had indeed removed the pauses.

Please do let me know if I have missed something. Or if these these tickets
should still be marked flaky.

ParallelGatewaySenderOperationsDUnitTest:
GEODE-933: Flaky tag was removed - Wait.pause was no longer used.

ParallelWANStatsDUnitTest
GEODE-977: Was Flaky because stats were being read from the wrong VM

Others:
GEODE-1066: Wait criterions were replaced with Awaitility, also order of
creating cache, regions and senders/receivers were modified.  No
wait.pauses were used.
GEODE-1147: No wait.pause were present. It was assumed that refactoring of
the WANTestBase and the code had fixed the issue.
GEODE-1148: Flaky tag was removed - Wait.pause was no longer used along
with other modifications to make the test stable.
GEODE-1207: Stopped WAN Locator Discovery Thread when locator is stopped.  No
wait.pauses were used.
GEODE-1011: Wait.pauses were removed. Code was modified to stabilize it.
GEODE-1062: Thread.sleep was removed. Issue was resolved by refactoring of
WANTestBase.
GEODE-1032: Listeners were put in to slow down the receivers. No
wait.pauses were used.
GEODE-1121: Memory configurations were changed to stabilize the test.  No
wait.pauses were used.

Regards
Nabarun


On Tue, Oct 4, 2016 at 11:04 AM Swapnil Bawaskar 
wrote:

+1

On Tue, Oct 4, 2016 at 10:58 AM, John Blum  wrote:


+1

On Tue, Oct 4, 2016 at 10:11 AM, Kirk Lund  wrote:


Please don't close flaky tickets or remove FlakyTest category unless you
know of a specific commit revision that makes some timing changes to the
test. Unless you replace all the Thread.sleeps with await() calls it's
going to fail again when GC occurs during the test. Just because a test
doesn't fail in 30+ runs, doesn't mean it's not flaky.

ParallelGatewaySenderOperationsDUnitTest has a bunch of these calls:

   Wait.pause(2000);

That's pretty much our definition of flaky.

ParallelWANStatsDUnitTest is worse because it has even shorter sleeps:

   pause(200);

I'm replacing the sleeps in the ManagementTestBase tests with Awaitility
calls and the tests are dropping from 5 minutes to 30 seconds per dunit
class. So that's another huge motivator to get rid of these broken

sleeps.

-Kirk


On Sun, Oct 2, 2016 at 7:33 PM, Xiaojian Zhou  wrote:


GEODE-933 and GEODE-977 are not reproducible either after run 30+

times.

So

they are not flaky and can be closed for now.

On Sat, Oct 1, 2016 at 11:30 PM, Xiaojian Zhou 

wrote:

1011, 1062, 1066, 1147 have been run 30+ times without reproduce. So

it's

not flaky. I think we can close them. If reproduced someday, we can
re-open.


On Sat, Oct 1, 2016 at 5:09 PM, Anthony Baker 

wrote:

I reviewed a bunch of CI failures today.  I closed out duplicates

and

added the ‘CI’ label to JIRA tickets that were missing it.  I just

posted a

big review to add the FlakyTest category to bugs with

non-reproducible

failures—pretty much any CI bug that is currently open.  Your

comments

are

appreciated (I can push a feature branch if that’s easier):

https://reviews.apache.org/r/52468/

I found several open issues where the flaky category had been

removed.

Can these be marked resolved?

GEODE-933
GEODE-977
GEODE-1011
GEODE-1062
GEODE-1066
GEODE-1147

I have a suspicion that the following open issues are actually

fixed.

Any ideas?

GEODE-1918
GEODE-1333,1334,1335


Anthony





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

Re: Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Oct. 3, 2016, 11:20 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52488/
> ---
> 
> (Updated Oct. 3, 2016, 11:20 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-420: fix Pulse test when not using any SSLConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
>  089dbacb056da84d6d6ceb36dad6f09b9ce73890 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
>  a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
>  5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 
> 
> Diff: https://reviews.apache.org/r/52488/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Udo Kohlmeyer]

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




geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 
(line 72)
<https://reviews.apache.org/r/52488/#comment219481>

Eventhough it might be "good practice" to check for null,I don't think we 
should check for null. There will always be a SSLConfig object and we should 
expect one. We should rather fix the issue if the SSLConfig is not correctly 
configured.



geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 
(line 198)
<https://reviews.apache.org/r/52488/#comment219479>

I don't think you should pass null. The SSLConfigurationFactory will always 
provide you with a SSLConfig object. This object might be a ssl-enabled=false, 
but that is ok. I think this should should be replaced with:

SSLConfigurationFactory.setDistributionConfig(new DistributionConfig);
final Server jetty = JettyHelper.initJetty(null, 8090, 
SSLConfigurationFactory.getSSLConfigForComponent(SecurableCommunicationChannel.WEB));


- Udo Kohlmeyer


On Oct. 3, 2016, 5:52 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52488/
> ---
> 
> (Updated Oct. 3, 2016, 5:52 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-420: fix Pulse test when not using any SSLConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
>  089dbacb056da84d6d6ceb36dad6f09b9ce73890 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
>  a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
>  5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 
> 
> Diff: https://reviews.apache.org/r/52488/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

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

[Udo Kohlmeyer]

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

(Updated Sept. 29, 2016, 11:31 p.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
 9476ef4 
  
geode-core/src/test/java/org/apache/geode/internal/cache/snapshot/GFSnapshotDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/cache/snapshot/TestObject.java
 PRE-CREATION 

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


Testing (updated)
---

precheckin - Done
regression - snapshot/snapshot.bt - Done


Thanks,

Udo Kohlmeyer

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Sept. 28, 2016, 4:08 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52172/
> ---
> 
> (Updated Sept. 28, 2016, 4:08 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/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/s

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

[Udo Kohlmeyer]

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

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: Review Request 52324: GEODE-1941: delete unimplemented tests

[Udo Kohlmeyer]

> On Sept. 28, 2016, 2:03 a.m., Udo Kohlmeyer wrote:
> > Ship It!

These tests are valuable... They needed to be completed. I'll complete them.


- Udo


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


On Sept. 28, 2016, 12:18 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52324/
> ---
> 
> (Updated Sept. 28, 2016, 12:18 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Jinmei Liao, 
> Jared Stewart, Kevin Duling, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1941
> https://issues.apache.org/jira/browse/GEODE-1941
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1941: delete unimplemented tests
> 
> This does not complete ticket GEODE-1941. It will remain open until
> the unimplemented tests are created or are determined to not be
> valuable.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorFactoryJUnitTest.java
>  b9faff0 
> 
> Diff: https://reviews.apache.org/r/52324/diff/
> 
> 
> Testing
> ---
> 
> SocketCreatorFactoryJUnitTest
> precheckin
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52324: GEODE-1941: delete unimplemented tests

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Sept. 28, 2016, 12:18 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52324/
> ---
> 
> (Updated Sept. 28, 2016, 12:18 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Jinmei Liao, 
> Jared Stewart, Kevin Duling, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1941
> https://issues.apache.org/jira/browse/GEODE-1941
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1941: delete unimplemented tests
> 
> This does not complete ticket GEODE-1941. It will remain open until
> the unimplemented tests are created or are determined to not be
> valuable.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorFactoryJUnitTest.java
>  b9faff0 
> 
> Diff: https://reviews.apache.org/r/52324/diff/
> 
> 
> Testing
> ---
> 
> SocketCreatorFactoryJUnitTest
> precheckin
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Build failed in Jenkins: Geode-spark-connector #78

[Udo Kohlmeyer]

I can easily fix this.

Sure we have a utility lying around in the core that can handle 
"String.isEmpty"


--Udo


On 24/09/2016 9:56 AM, Anthony Baker wrote:

Yep, I’m seeing failures on any client app that doesn’t explicitly include 
spring as dependency.

Exception in thread "main" java.lang.NoClassDefFoundError: 
org/springframework/util/StringUtils
at 
org.apache.geode.internal.net.SSLConfigurationFactory.configureSSLPropertiesFromSystemProperties(SSLConfigurationFactory.java:274)
at 
org.apache.geode.internal.net.SSLConfigurationFactory.configureSSLPropertiesFromSystemProperties(SSLConfigurationFactory.java:270)
at 
org.apache.geode.internal.net.SSLConfigurationFactory.createSSLConfigForComponent(SSLConfigurationFactory.java:138)
at 
org.apache.geode.internal.net.SSLConfigurationFactory.getSSLConfigForComponent(SSLConfigurationFactory.java:67)
at 
org.apache.geode.internal.net.SocketCreatorFactory.getSocketCreatorForComponent(SocketCreatorFactory.java:67)
at 
org.apache.geode.distributed.internal.tcpserver.TcpClient.(TcpClient.java:69)
at 
org.apache.geode.cache.client.internal.AutoConnectionSourceImpl.(AutoConnectionSourceImpl.java:114)
at 
org.apache.geode.cache.client.internal.PoolImpl.getSourceImpl(PoolImpl.java:579)
at 
org.apache.geode.cache.client.internal.PoolImpl.(PoolImpl.java:219)
at 
org.apache.geode.cache.client.internal.PoolImpl.create(PoolImpl.java:132)
at 
org.apache.geode.internal.cache.PoolFactoryImpl.create(PoolFactoryImpl.java:319)
at 
org.apache.geode.internal.cache.GemFireCacheImpl.determineDefaultPool(GemFireCacheImpl.java:2943)
at 
org.apache.geode.internal.cache.GemFireCacheImpl.initializeDeclarativeCache(GemFireCacheImpl.java:1293)
at 
org.apache.geode.internal.cache.GemFireCacheImpl.initialize(GemFireCacheImpl.java:1124)
at 
org.apache.geode.internal.cache.GemFireCacheImpl.basicCreate(GemFireCacheImpl.java:765)
at 
org.apache.geode.internal.cache.GemFireCacheImpl.createClient(GemFireCacheImpl.java:740)
at 
org.apache.geode.cache.client.ClientCacheFactory.basicCreate(ClientCacheFactory.java:235)
at 
org.apache.geode.cache.client.ClientCacheFactory.create(ClientCacheFactory.java:189)
at HelloWorld.main(HelloWorld.java:25)
Caused by: java.lang.ClassNotFoundException: 
org.springframework.util.StringUtils
at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
... 19 more

Anthony



On Sep 23, 2016, at 4:34 PM, Dan Smith  wrote:

I created GEODE-1934 for this. It looks like the problem is actually that
our dependencies for geode-core are messed up. spring-core is marked
optional, but we're using it in critical places like this
SSLConfigurationFactory.

In my opinion we shouldn't depend on spring-core at all unless we're
actually going to use it for things other than StringUtils. I think we've
accidentally introduced dependencies on it because the gfsh code in the
core is pulling in a bunch of spring libraries.

-Dan


On Fri, Sep 23, 2016 at 9:12 AM, Apache Jenkins Server <
jenk...@builds.apache.org> wrote:


See 

Changes:

[hkhamesra] GEODE-37 In spark connector we call TcpClient static method to
get the

[klund] GEODE-1906: fix misspelling of Successfully

[upthewaterspout] GEODE-1915: Prevent deadlock registering instantiators
with gateways

--
[...truncated 1883 lines...]
16/09/23 16:11:05 INFO HttpFileServer: HTTP File server directory is
/tmp/spark-f13dac55-087f-4379-aeed-616fbdc7ffac/httpd-
02c1fab9-faa0-47f4-b0f3-fd44383eeeb3
16/09/23 16:11:05 INFO HttpServer: Starting HTTP Server
16/09/23 16:11:05 INFO Utils: Successfully started service 'HTTP file
server' on port 40135.
16/09/23 16:11:05 INFO SparkEnv: Registering OutputCommitCoordinator
16/09/23 16:11:10 WARN Utils: Service 'SparkUI' could not bind on port
4040. Attempting port 4041.
16/09/23 16:11:15 INFO Utils: Successfully started service 'SparkUI' on
port 4041.
16/09/23 16:11:15 INFO SparkUI: Started SparkUI at http://localhost:4041
16/09/23 16:11:15 INFO Executor: Starting executor ID  on host
localhost
16/09/23 16:11:15 INFO AkkaUtils: Connecting to HeartbeatReceiver:
akka.tcp://sparkDriver@localhost:54872/user/HeartbeatReceiver
16/09/23 16:11:16 INFO NettyBlockTransferService: Server created on 41182
16/09/23 16:11:16 INFO BlockManagerMaster: Trying to register BlockManager
16/09/23 16:11:16 INFO BlockManagerMasterActor: Registering block manager
localhost:41182 with 2.8 GB RAM, BlockManagerId(, localhost, 41182)
16/09/23 16:11:16 INFO BlockManagerMaster: Registered BlockManager
=== GeodeRunner: stop server 1.
=== GeodeRunner: stop se

Re: Hibernate module and Geode 1.0 ?

[Udo Kohlmeyer]

Correct. Put it on a feature branch awaiting contributions.


On 23/09/2016 8:41 AM, Anthony Baker wrote:

By remove, do you mean “put it on a feature branch awaiting contributions?”

hibernate-3.5.5 was released in 2010 and has undergone significant changes 
since then.  Seems reasonable to put this feature on hold until can be brought 
current.

Anthony


On Sep 22, 2016, at 3:31 PM, Udo Kohlmeyer  wrote:

+1 to removing until updated to newer version

Do we know if anyone in the big-bad world is using it?

--Udo


On 23/09/2016 8:15 AM, William Markito wrote:

Folks,

We're still building the Hibernate cache module [1] but it's compatible
only with a very old version (3.5) and given that the API has completely
changed and unless someone in the community wants to help getting this
module up-to-date with at least Hibernate 5.x I'd like propose to remove
the module from 1.0 / develop until we can work on updating that module.

Given that it's already a separate module it shouldn't be that hard to be
removed.

Thoughts ?

[1]
http://geode.docs.pivotal.io/docs/tools_modules/hibernate_cache/chapter_overview.html

Re: Hibernate module and Geode 1.0 ?

[Udo Kohlmeyer]

+1 to removing until updated to newer version

Do we know if anyone in the big-bad world is using it?

--Udo


On 23/09/2016 8:15 AM, William Markito wrote:

Folks,

We're still building the Hibernate cache module [1] but it's compatible
only with a very old version (3.5) and given that the API has completely
changed and unless someone in the community wants to help getting this
module up-to-date with at least Hibernate 5.x I'd like propose to remove
the module from 1.0 / develop until we can work on updating that module.

Given that it's already a separate module it shouldn't be that hard to be
removed.

Thoughts ?

[1]
http://geode.docs.pivotal.io/docs/tools_modules/hibernate_cache/chapter_overview.html

Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

[Udo Kohlmeyer]

> On Sept. 15, 2016, 6:40 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java,
> >  line 1728
> > 
> >
> > Why is this deleted? This is for ssl-enabled-components (Udo's work) 
> > and has nothing to do with security-enabled-components aka 
> > authentication&authorization (A&A). 
> > 
> > After Udo's latest commit, we probably need to change his code to use 
> > SSLSecurableComponents or SSLSecurableChannels and then change our code to 
> > have its own separate SecurableComponents with a different list of 
> > constants (depending on what Swapnil's requirements are).
> 
> Kirk Lund wrote:
> I just looked at Udo's code. I think this needs to change to be 
> SecurableCommunicationChannels. Udo introduced that for 
> SSL_ENABLED_COMPONENTS, but I think he missed this place in the code. So 
> instead of deleting it, we probably just need to change the list to what he 
> has in SecurableCommunicationChannels (http changes to web) and change the 
> @link to SecurableCommunicationChannels.

I've just committed the updated javadoc.


- Udo


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


On Sept. 15, 2016, 6:14 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> ---
> 
> (Updated Sept. 15, 2016, 6:14 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil 
> Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not 
> have to implement an AuthInit object, they can just put 
> security-username/security-password in their properties file.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
>  d843792168e91b39b686c84de66b0087c1ae65b4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  68ec0c0041f204775541db396022a1df14c868fe 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  00372aebc8d72376ff6613c13637faeaac8d1523 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 
> 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   
> geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java
>  870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

[Udo Kohlmeyer]

> On Sept. 15, 2016, 6:40 p.m., Kirk Lund wrote:
> >

The ssl-enabled-components uses SecurableCommunicationChannels. I'll be 
updating this javadoc to reflect that.


- Udo


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


On Sept. 15, 2016, 6:14 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51923/
> ---
> 
> (Updated Sept. 15, 2016, 6:14 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil 
> Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1883: making AuthInit optional when starting a server/client
> 
> When starting a server trying to connect to a locator, customer should not 
> have to implement an AuthInit object, they can just put 
> security-username/security-password in their properties file.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
>  d843792168e91b39b686c84de66b0087c1ae65b4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  68ec0c0041f204775541db396022a1df14c868fe 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  00372aebc8d72376ff6613c13637faeaac8d1523 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  7380c9a0cb7382f8a2758acc1018fd793ce815df 
>   geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 
> 9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
>   
> geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java
>  870ff91ab1a3d3537754268f6e2f2c33f24141b7 
> 
> Diff: https://reviews.apache.org/r/51923/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: securing geode components

[Udo Kohlmeyer]

 be undone or changed in

the

next

iteration and most of us are following weekly iterations.

After a release anything exposed in a User API is very difficult

to

change

due to backwards compatibility constraints. I think we should be

much

more

careful with User APIs in Geode going forward to avoid some of the

problems

we have with pre-existing Geode User APIs that we inherited.

-Kirk

On Thu, Sep 8, 2016 at 2:07 PM, Udo Kohlmeyer <

ukohlme...@pivotal.io>

wrote:

As GEODE-420 deals with SSL comms configuration and GEODE-1648

with

Authentication&Authorization I think we need to be careful in

what

is

feasible and what is logical.

For SSL comms it was decided that the following components are

relevant

[1] <https://cwiki.apache.org/confluence/display/GEODE/Revised+S
SL+properties>:

* Locator => The comms channel between Locators + the initial

comms

  channel between clients and locator
* Cluster => Internode comms channel (peer to peer)
* Server => Client-server comms channel
* Gateway => Comms channel between WAN Gateway senders/receivers
* HTTP => Any HTTP comms. incl REST and Pulse
* JMX => Any JMX comms

These components were selected as they seem to be logical

boundaries

and

communication interfaces.

I think the specialization of HTTP, for

Authentication&Authorization

are

functions of those interfaces:

* REST-admin
* REST-dev
* Pulse

I think that comms and functions exposed by those comms should

not

be

mixed. I think that securing the comms channel is a factor of

"trust".

Do I

implicitly trust the interface/system that I am connected to or

are

connecting to.

I think concepts like "management" is a concept in function. Do I

allow

a

user to access admin API's? The function of management should not

determine

if a system trusts another systems connection. When a new comms

interface

is added (say messaging), we want to be able to trust that comms

channel.

The "management" function should still work regardless of

interface,

be

it

jmx,http/rest,prop tcp,messaging.

--Udo

[1]: https://cwiki.apache.org/confluence/display/GEODE/

Revised+SS

L+properties



On 9/09/2016 5:49 AM, Swapnil Bawaskar wrote:


GEODE-1648 and GEODE-420 are both trying to add geode properties

to

secure

only some components.
GEODE-1648 is intending to add a property named
"security-enabled-components" that will allow users to turn off
authentication/authorization for some components
GEODE-420 is intending to add a property named

"ssl-enabled-components"

that will allow users to turn off ssl. for either client/server,
peer-to-peer or wan communication.

Since both deal with security, I think we should have the same

list

of

components for these new geode properties. Intent of this thread

is

to

arrive at a consensus on what these components are.

I would like to propose the following components:
Cluster => stands for peer-to-peer
Server => client/server and developer rest API
WAN => gateway sender/receiver
Management => jmx, admin-rest, pulse

Thanks!
Swapnil.






--
Cheers

Jinmei






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

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Sept. 7, 2016, 4:25 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51696/
> ---
> 
> (Updated Sept. 7, 2016, 4:25 p.m.)
> 
> 
> 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 51728: GEODE-1570: upgrade spring libraries and fix tests

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Sept. 8, 2016, 3:45 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51728/
> ---
> 
> (Updated Sept. 8, 2016, 3:45 p.m.)
> 
> 
> Review request for geode, Kevin Duling, Kirk Lund, Udo Kohlmeyer, and Dan 
> Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * updated the spring framework libraries
> * updated the spring security libraries and related upgrades
> * fixed the tests and uitests
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/rest/internal/web/RestInterfaceJUnitTest.java
>  8246671a7e70267d64e354ad3ce43c1afb56f7c3 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/rest/internal/web/RestSecurityDUnitTest.java
>  847ca7675da6c0490e0fdc9ceacda89c4a1ec7e8 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle ea1fce20f45b523e98b62b7569275c0079313a5a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  529336f813cfd6a0f2b227281084a805f34c1729 
>   geode-pulse/build.gradle e53a698700f0de62088b3ac50cb96d4841315124 
>   
> geode-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/security/GemFireAuthentication.java
>  391ad39d22dfe37ee056c7cdeb6d2bf6554206ae 
>   
> geode-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  2d7d6068337e95a3bcc6acc1aa211bf8c6da19d1 
>   
> geode-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberGatewayHubService.java
>  dd84b75c5742b3457d82a1a101521948cc8508f2 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> 60edb18ba615b69e6fc04fde35bf829dcfef34db 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> b14d03d2a27f061c37af8a644610156fc4b651a9 
>   
> geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAuthTest.java
>  65cd47fb111e765cc515466728221ef6607221ca 
>   
> geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAutomatedTest.java
>  299a343bd7258c97a5ce8b63ae5e5e35ae242142 
>   
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/util/JSONUtils.java
>  cb9b39df46701903a1c5c0bf78d7e728083a27a3 
>   
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/util/JsonWriter.java
>  a0ff676a9eeb64cf1c8151a60a20633a38ef80f1 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> e96acb0d2805abce804b04be678374054652f671 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> ce659336c0cad8ce8e75de50ee69856259c69af2 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> a19520cb6f75bd63136d99ff62efd2b9d5f45643 
> 
> Diff: https://reviews.apache.org/r/51728/diff/
> 
> 
> Testing
> ---
> 
> precheckin and uitests and pulse
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: securing geode components

[Udo Kohlmeyer]

As GEODE-420 deals with SSL comms configuration and GEODE-1648 with 
Authentication&Authorization I think we need to be careful in what is 
feasible and what is logical.


For SSL comms it was decided that the following components are relevant 
[1] 
:


 * Locator => The comms channel between Locators + the initial comms
   channel between clients and locator
 * Cluster => Internode comms channel (peer to peer)
 * Server => Client-server comms channel
 * Gateway => Comms channel between WAN Gateway senders/receivers
 * HTTP => Any HTTP comms. incl REST and Pulse
 * JMX => Any JMX comms

These components were selected as they seem to be logical boundaries and 
communication interfaces.


I think the specialization of HTTP, for Authentication&Authorization are 
functions of those interfaces:


 * REST-admin
 * REST-dev
 * Pulse

I think that comms and functions exposed by those comms should not be 
mixed. I think that securing the comms channel is a factor of "trust". 
Do I implicitly trust the interface/system that I am connected to or are 
connecting to.


I think concepts like "management" is a concept in function. Do I allow 
a user to access admin API's? The function of management should not 
determine if a system trusts another systems connection. When a new 
comms interface is added (say messaging), we want to be able to trust 
that comms channel. The "management" function should still work 
regardless of interface, be it jmx,http/rest,prop tcp,messaging.


--Udo

[1]: 
https://cwiki.apache.org/confluence/display/GEODE/Revised+SSL+properties



On 9/09/2016 5:49 AM, Swapnil Bawaskar wrote:

GEODE-1648 and GEODE-420 are both trying to add geode properties to secure
only some components.
GEODE-1648 is intending to add a property named
"security-enabled-components" that will allow users to turn off
authentication/authorization for some components
GEODE-420 is intending to add a property named "ssl-enabled-components"
that will allow users to turn off ssl. for either client/server,
peer-to-peer or wan communication.

Since both deal with security, I think we should have the same list of
components for these new geode properties. Intent of this thread is to
arrive at a consensus on what these components are.

I would like to propose the following components:
Cluster => stands for peer-to-peer
Server => client/server and developer rest API
WAN => gateway sender/receiver
Management => jmx, admin-rest, pulse

Thanks!
Swapnil.

Re: Review Request 51612: GEODE-1834: initilize the socketcreator with the correct ssl settings

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Sept. 2, 2016, 6:38 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51612/
> ---
> 
> (Updated Sept. 2, 2016, 6:38 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Kevin Duling, Kirk Lund, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1834: initilize the socketcreator with the correct ssl settings
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/JmxManagerLocatorRequest.java
>  861f51d4e0077440ab3128e6053c389cecfd9bb8 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java
>  09a25a67d748524cbbc19099c453d045b415ce5c 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/ConnectToLocatorSSLDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51612/diff/
> 
> 
> Testing
> ---
> 
> the newly added tests to cover both cluster-ssl-* and jmx-ssl-* settings. 
> This would test for both GEM-877 and GEM-626
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

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

[Udo Kohlmeyer]

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

(Updated Sept. 6, 2016, 9:30 p.m.)


Review request for geode, Bruce Schuchardt and Hitesh Khamesra.


Summary (updated)
-

GEODE-1792: Configuration consistency in SSL configuration


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

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

[Udo Kohlmeyer]

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

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: M3 is done, what's next?

[Udo Kohlmeyer]

+1 for package renaming

+1 for sooner than later


On 3/09/2016 2:46 AM, Dan Smith wrote:

+1 For renaming the packages. It would be really nice to graduate ASAP! Is
there anything else from a code perspective that we need to do before
graduation? If so we should also get that in 1.0.

It would be nice to get a few more examples in the codebase for 1.0. We
should probably just generally review the documentation we're shipping with
1.0. Actually, it would be nice if the docs hosted on
http://geode.docs.pivotal.io/ could get incorporated as well (I think
pivotal is still planning on donating those docs?), but I don't think we
should hold up 1.0 or graduation based on that.

We should probably review our dependencies and update anything that's out
of date for 1.0.

We should also coordinate the package renaming with Spring Data Geode.

-Dan


On Fri, Sep 2, 2016 at 9:08 AM, Greg Chase  wrote:


On Fri, Sep 2, 2016 at 8:46 AM, Anthony Baker  wrote:


with one exception:  we need to rename our source packages to
‘org.apache.geode’ [3].

I think we should move forward with package renaming *now* and include
that in the scope for the 1.0.0-incubating release.

As previously discussed [4] we’d like to preserve protocol compatibility
for existing users of client/server and WAN.  This should only affect a
handful of classes that would remain in the ‘com.gemstone.gemfire’
namespace (we should identify those soon).


Agreed.  Now is the time.  Later is always worse then now when it occurs.

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


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 51227: GEODE-420: Locator SSL

[Udo Kohlmeyer]

/JettyHelperJUnitTest.java
 1c66780 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommandsDUnitTest.java
 5720955 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
 21cdfbc 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommandsDUnitTest.java
 6f70e57 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
 32cea6a 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/HTTPServiceSSLSupportJUnitTest.java
 7a96adf 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommandsDUnitTest.java
 2691e09 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommandsDUnitTest.java
 86c61b6 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/SharedConfigurationCommandsDUnitTest.java
 87a7ab9 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationDUnitTest.java
 3c17f9f 
  
geode-core/src/test/java/com/gemstone/gemfire/security/generator/SSLCredentialGenerator.java
 c591732 
  geode-core/src/test/java/com/gemstone/gemfire/test/dunit/NetworkUtils.java 
dec882e 
  
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
 686779d 
  geode-core/src/test/java/org/apache/geode/redis/RedisDistDUnitTest.java 
eb87797 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/excludedClasses.txt
 ddacf99 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedSerializables.txt
 03288c2 
  geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKey.jks 
PRE-CREATION 
  
geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKeyTrust.jks
 PRE-CREATION 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/driver/PulseUITest.java
 24ba815 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAbstractTest.java
 eeaa069 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorDiscovery.java
 0fd206e 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 5f4943d 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractRemoteGatewaySender.java
 d45d5ef 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewayReceiverImpl.java
 9fd73e6 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 fd2c0b5 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/configuration/ClusterConfigurationDUnitTest.java
 bdd5d71 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 b35dce9 
  gradle/java.gradle 36d88dc 

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


Testing
---

precheckin - completed. All green.
regression testing to follow


Thanks,

Udo Kohlmeyer

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

[Udo Kohlmeyer]

/test/java/com/gemstone/gemfire/management/internal/JettyHelperJUnitTest.java
 1c66780 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommandsDUnitTest.java
 5720955 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
 21cdfbc 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommandsDUnitTest.java
 6f70e57 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
 32cea6a 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/HTTPServiceSSLSupportJUnitTest.java
 7a96adf 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommandsDUnitTest.java
 2691e09 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommandsDUnitTest.java
 86c61b6 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/SharedConfigurationCommandsDUnitTest.java
 87a7ab9 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationDUnitTest.java
 3c17f9f 
  
geode-core/src/test/java/com/gemstone/gemfire/security/generator/SSLCredentialGenerator.java
 c591732 
  geode-core/src/test/java/com/gemstone/gemfire/test/dunit/NetworkUtils.java 
dec882e 
  
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
 686779d 
  geode-core/src/test/java/org/apache/geode/redis/RedisDistDUnitTest.java 
eb87797 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/excludedClasses.txt
 ddacf99 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedSerializables.txt
 03288c2 
  geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKey.jks 
PRE-CREATION 
  
geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKeyTrust.jks
 PRE-CREATION 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/driver/PulseUITest.java
 24ba815 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAbstractTest.java
 eeaa069 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorDiscovery.java
 0fd206e 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 5f4943d 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractRemoteGatewaySender.java
 d45d5ef 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewayReceiverImpl.java
 9fd73e6 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 fd2c0b5 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/configuration/ClusterConfigurationDUnitTest.java
 bdd5d71 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 b35dce9 
  gradle/java.gradle 36d88dc 

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


Testing
---

precheckin - completed. All green.
regression testing to follow


Thanks,

Udo Kohlmeyer

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

[Udo Kohlmeyer]

/com/gemstone/gemfire/management/internal/JettyHelperJUnitTest.java
 1c66780 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommandsDUnitTest.java
 5720955 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
 21cdfbc 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommandsDUnitTest.java
 6f70e57 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
 32cea6a 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/HTTPServiceSSLSupportJUnitTest.java
 7a96adf 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommandsDUnitTest.java
 2691e09 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommandsDUnitTest.java
 86c61b6 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/SharedConfigurationCommandsDUnitTest.java
 87a7ab9 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationDUnitTest.java
 3c17f9f 
  
geode-core/src/test/java/com/gemstone/gemfire/security/generator/SSLCredentialGenerator.java
 c591732 
  geode-core/src/test/java/com/gemstone/gemfire/test/dunit/NetworkUtils.java 
dec882e 
  
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
 686779d 
  geode-core/src/test/java/org/apache/geode/redis/RedisDistDUnitTest.java 
eb87797 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/excludedClasses.txt
 ddacf99 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedSerializables.txt
 03288c2 
  geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKey.jks 
PRE-CREATION 
  
geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKeyTrust.jks
 PRE-CREATION 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/driver/PulseUITest.java
 24ba815 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAbstractTest.java
 eeaa069 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorDiscovery.java
 0fd206e 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 5f4943d 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractRemoteGatewaySender.java
 d45d5ef 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewayReceiverImpl.java
 9fd73e6 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 fd2c0b5 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/configuration/ClusterConfigurationDUnitTest.java
 bdd5d71 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 b35dce9 
  gradle/java.gradle 36d88dc 

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


Testing (updated)
---

precheckin - completed. All green.
regression testing to follow


Thanks,

Udo Kohlmeyer

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

[Udo Kohlmeyer]

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




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

I believe that this static method invocation should not happen here. I 
think that the serialization should be transparent. If we start special casing 
it this, it can quickly become unmanageable.


- Udo Kohlmeyer


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

[Udo Kohlmeyer]

/test/java/com/gemstone/gemfire/management/internal/JettyHelperJUnitTest.java
 1c66780 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommandsDUnitTest.java
 5720955 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
 21cdfbc 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommandsDUnitTest.java
 6f70e57 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
 32cea6a 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/HTTPServiceSSLSupportJUnitTest.java
 7a96adf 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommandsDUnitTest.java
 2691e09 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommandsDUnitTest.java
 86c61b6 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/SharedConfigurationCommandsDUnitTest.java
 87a7ab9 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationDUnitTest.java
 3c17f9f 
  
geode-core/src/test/java/com/gemstone/gemfire/security/generator/SSLCredentialGenerator.java
 c591732 
  geode-core/src/test/java/com/gemstone/gemfire/test/dunit/NetworkUtils.java 
dec882e 
  
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
 686779d 
  geode-core/src/test/java/org/apache/geode/redis/RedisDistDUnitTest.java 
eb87797 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/excludedClasses.txt
 ddacf99 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedSerializables.txt
 03288c2 
  geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKey.jks 
PRE-CREATION 
  
geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKeyTrust.jks
 PRE-CREATION 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/driver/PulseUITest.java
 24ba815 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAbstractTest.java
 eeaa069 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorDiscovery.java
 0fd206e 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 5f4943d 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractRemoteGatewaySender.java
 d45d5ef 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewayReceiverImpl.java
 9fd73e6 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 fd2c0b5 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/configuration/ClusterConfigurationDUnitTest.java
 bdd5d71 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 b35dce9 
  gradle/java.gradle 36d88dc 

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


Testing (updated)
---

precheckin
regression testing to follow


Thanks,

Udo Kohlmeyer

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

[Udo Kohlmeyer]

/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommandsDUnitTest.java
 5720955 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
 21cdfbc 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommandsDUnitTest.java
 6f70e57 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
 32cea6a 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/HTTPServiceSSLSupportJUnitTest.java
 7a96adf 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommandsDUnitTest.java
 2691e09 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommandsDUnitTest.java
 86c61b6 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/SharedConfigurationCommandsDUnitTest.java
 87a7ab9 
  
geode-core/src/test/java/com/gemstone/gemfire/management/internal/configuration/SharedConfigurationDUnitTest.java
 3c17f9f 
  
geode-core/src/test/java/com/gemstone/gemfire/security/generator/SSLCredentialGenerator.java
 c591732 
  geode-core/src/test/java/com/gemstone/gemfire/test/dunit/NetworkUtils.java 
dec882e 
  
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/internal/JUnit4DistributedTestCase.java
 686779d 
  geode-core/src/test/java/org/apache/geode/redis/RedisDistDUnitTest.java 
eb87797 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/excludedClasses.txt
 ddacf99 
  
geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedSerializables.txt
 03288c2 
  geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKey.jks 
PRE-CREATION 
  
geode-core/src/test/resources/com/gemstone/gemfire/internal/net/multiKeyTrust.jks
 PRE-CREATION 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/driver/PulseUITest.java
 24ba815 
  
geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseAbstractTest.java
 eeaa069 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorDiscovery.java
 0fd206e 
  
geode-wan/src/main/java/com/gemstone/gemfire/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 5f4943d 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractRemoteGatewaySender.java
 d45d5ef 
  
geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewayReceiverImpl.java
 9fd73e6 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
 fd2c0b5 
  
geode-wan/src/test/java/com/gemstone/gemfire/management/internal/configuration/ClusterConfigurationDUnitTest.java
 bdd5d71 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 b35dce9 
  gradle/java.gradle 36d88dc 

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


Testing
---


Thanks,

Udo Kohlmeyer

Re: Trying to upgrade spring-security

[Udo Kohlmeyer]

I think, if were going to upgrade to 4.1.2, we might considering 
upgrading the rest of the Spring libraries to the 4.3.2 compatible.


spring-hateos.version = 0.16.0.RELEASE > 0.21.0 spring-shell.version 
= 1.1.0.RELEASE > 1.2.0 spring-ldap-core.version = 1.3.2.RELEASE 
> 2.1.0 spring-security.version = 3.1.7.RELEASE > 4.1.2 
spring-tx.version = 3.2.12.RELEASE > 4.3.2 springframework.version = 
4.2.4.RELEASE > 4.3.2



On 17/08/2016 2:03 AM, Kevin Duling wrote:

Yes, that's my experience so far. But this particular error persists
regardless of how deep down the rabbit hole I gone and I haven't been able
to solve it.  I think it's due to an older slf4j being bundled in to jetty.

I'll give those commands a try and see whether they help or not.

On Tue, Aug 16, 2016 at 8:53 AM, Anthony Baker  wrote:


Not sure if this is related to your error, but it looks like that version
of spring-security would at least require moving to spring framework 4.3.1
and possibly more dep updates as well.  You should be able to run some
gradle commands (dependencies, dependencyInsight, dumpDependencies) to help
sort out the full list of version changes needed.

Anthony


On Aug 16, 2016, at 8:39 AM, Kevin Duling  wrote:

I'm working on adding security to the REST methods and wanted to start by
upgrading spring-security to the latest version.

Bumping the version from 4.1.1 from 3.1.7, I ran in to
GfshCommandsOverHttpSecurityTest failing under geode-web.  The error

was:

[warn 2016/08/12 15:37:52.078 PDT  tid=0x1] loader constraint
violation: when resolving method "org.slf4j.impl.StaticLoggerBinder.
getLoggerFactory()Lorg/slf4j/ILoggerFactory;" the class loader

(instance of

org/eclipse/jetty/webapp/WebAppClassLoader) of the current class,
org/slf4j/LoggerFactory, and the class loader (instance of
sun/misc/Launcher$AppClassLoader) for the method's defining class,
org/slf4j/impl/StaticLoggerBinder, have different Class objects for the
type org/slf4j/ILoggerFactory used in the signature
java.lang.LinkageError: loader constraint violation: when resolving

method

"org.slf4j.impl.StaticLoggerBinder.getLoggerFactory()Lorg/slf4j/

ILoggerFactory;"

the class loader (instance of org/eclipse/jetty/webapp/

WebAppClassLoader)

of the current class, org/slf4j/LoggerFactory, and the class loader
(instance of sun/misc/Launcher$AppClassLoader) for the method's defining
class, org/slf4j/impl/StaticLoggerBinder, have different Class objects

for

the type org/slf4j/ILoggerFactory used in the signature

Re: Publishing of geode javadocs

[Udo Kohlmeyer]

Maybe the review of the Javadoc for any class should be done as part of 
the final review of a story.


Any public facing classes should be checked that the javadoc is correct 
for the affected classes.


That the render correctly in the IDE, I think might be a side affect of 
making sure that they docs are actually checked.


--Udo


On 13/08/2016 10:00 AM, Kirk Lund wrote:

We really need to find an automated solution for discovering javadoc errors
earlier than manual inspection or looking for warnings in the gradle
output.

We change our build to fail on javadoc errors because it starts to fail due
to 3rd party library javadocs, but at a minimum there must be some
alternative for enforcing no warnings in the build.

As for ConfigurationProperties' javadocs, I don't think it's generating any
warnings but the javadocs contain malformed html so the result is clearly
broken. I'm not sure if there's anyway to automate detecting problems like
that.

-Kirk


On Fri, Aug 12, 2016 at 4:51 PM, Dan Smith  wrote:


I'm not sure if we're publishing the nightly javadocs anywhere, but you can
build them from source. If you do a ./gradlew build, the external javadocs
are in geode-assembly/build/install/apache-geode/javadoc

-Dan

On Fri, Aug 12, 2016 at 4:31 PM, Kirk Lund  wrote:


I can find the following from Google, but this is the javadocs for the M2
release:

http://geode.incubator.apache.org/releases/latest/javadoc/

Does this need manual updating for M3?

Are we publishing nightly javadocs?

When I use CTRL-J (in IntelliJ) on the ConfiguratingProperties class, the
javadocs look malformed, so I'd like to review real javadoc html for the
current state of develop.

-Kirk

Re: Revised SSL properties failure scenario advice.

[Udo Kohlmeyer]

Hi there Darrel,

You are correct I ssl-certificate-alias and ssl-default-alias would 
be the same. I must have missed the property


You are also correct in stating the question more clearly. If a 
component provides a specific alias and one has not set the 
ssl-ceritificate-alias (or shall we call it ssl-default-alias) then we 
should fail.


I believe that the SSL api's would throw an exception if an alias is 
provided that does not exist within the keystore. I might have to 
investigate how I can validate this at startup before the locator/server 
tries to join the cluster.


--Udo

On 12/08/2016 8:39 AM, Darrel Schneider wrote:

how does ssl-default-alias interact with this one in the spec:
ssl-certificate-alias=[empty,string] (default - use first cert in
keystore)
I thought ssl-certificate-alias was the one to use by default. It could be
overridden by a component one.
And the default for it is to use the first certificate in the keystore. Are
you suggesting that the default is only the first cert when the keystore
only has one?

I think having both ssl-default-alias and ssl-certificate-alias would be
confusing (I'm currently confused by it).

I also don't understand the failure scenario you describe. Here is an
example of what I hear you saying:
if the keystore how more than one cert and you do not configure a default
but you do configure a specific cert alias for each component then fail.
If all the certs they configure are in the key store then what is the
reason for failure? Is some other component is using the default one and
the default is either undefined or non-existent then I would vote for
failure.


On Wed, Aug 10, 2016 at 4:59 PM, Kirk Lund  wrote:


+1 to introduce "ssl-default-alias" and fail if it's not set for multi-key
keystore


On Wed, Aug 10, 2016 at 4:34 PM, Bruce Schuchardt 
wrote:


+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 <
https://cwiki.apache.org/confluence/display/GEODE/

Revised+SSL+properties>,

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

Revised SSL properties failure scenario advice.

[Udo Kohlmeyer]

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: Javadoc warnings

[Udo Kohlmeyer]

Hmmm...

Can you please get your team in check I get the following since 
updating  :P :D :D :D



/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java:49: 
warning - @return tag has no arguments.
/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java:57: 
warning - @return tag has no arguments.
/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java:65: 
warning - @return tag has no arguments.
/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java:73: 
warning - @return tag has no arguments.
/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java:48: 
warning - @return tag has no arguments.
/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java:383: 
warning - @return tag has no arguments.
/Users/ukohlmeyer/projects/gemfire/open/geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java:412: 
warning - @return tag has no arguments.



--Udo

On 20/07/2016 3:37 PM, Kirk Lund wrote:

Reminder: please watch for javadoc warnings in your code before committing.

:geode-core:javadoc

/Users/klund/dev/gemfire/open/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java:2040:
warning - @guarded.By is an unknown tag.

/Users/klund/dev/gemfire/open/geode-core/src/main/java/org/apache/geode/security/GeodePermission.java:49:
warning - @return tag has no arguments.

-Kirk

Re: Review Request 49729: Used static instance of logger while logging

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On July 6, 2016, 9:39 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49729/
> ---
> 
> (Updated July 6, 2016, 9:39 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Used static instance of logger while logging
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegion.java
>  26c91e0 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXEntryState.java
>  c6caefa 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/DestroyRegionOnDataStoreMessage.java
>  225d1af 
> 
> Diff: https://reviews.apache.org/r/49729/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 49666: GEODE-1613 CI failure: ConnectionPoolDUnitTest.test021ClientGetOfInvalidServerEntry

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On July 5, 2016, 11:21 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49666/
> ---
> 
> (Updated July 5, 2016, 11:21 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1613
> https://issues.apache.org/jira/browse/GEODE-1613
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The suspect string that caused this test to fail showed that the executor 
> being used had been terminated.  The code scheduling a task in this executor 
> needs to perform a cancellation check if execution is rejected.
> 
> I rewrote all of the other cancellation checks in this method to avoid 
> throwing Cancel exceptions during shutdown.  There's no reason for it to do 
> that - it can just exit the run() method.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/DataSerializerRecoveryListener.java
>  0c3f69229a3bdbbe8fea66f7e5dc893d460e2785 
> 
> Diff: https://reviews.apache.org/r/49666/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Docker testing support for Geode

[Udo Kohlmeyer]

Hi there,

Whilst thinking about how to improve our testing capability (the 
provisioning of machines and VMs), I was pointed at this project.

https://github.com/palantir/docker-compose-rule

I wonder if this is something that we can look at. Maybe some extension 
of this, could help with our multi-machine testing needs.


--Udo

Re: update website for WAN, CQ and native client

[Udo Kohlmeyer]

+1,

to me "Multi-site" is too rigid. People might forget that this is a 
inter-cluster comms channel. Which would allow them to have many 
clusters running in the same DC, sharing data between them, satisfying 
the data requirements for different systems/applications.



On 22/06/2016 4:54 AM, Gregory Chase wrote:

I'm +0 to "Multi-site", but like "multi-cluster" slightly better for two
reasons...

1) It comes after "Clustering", showing a ++ more clearly
2) In my opinion its more inclusive of both "multi-site" and also workload
partitioning where two clusters might be in the same data center, but have
different applications they serve.

Thoughts?

On Tue, Jun 21, 2016 at 11:49 AM, John Blum  wrote:


+1

On Tue, Jun 21, 2016 at 11:43 AM, Swapnil Bawaskar 
wrote:


Take 3: http://i.imgur.com/VUQRw4u.png
1. Replaced Redis logo with http://fontawesome.io/icon/plug/
2. Changed WAN to Multi-Site.

Thanks!
Swapnil.

On Sat, Jun 18, 2016 at 7:22 PM, theseusyang 
wrote:


I agree with Greg, "Multi-Cluster"  is accurated especially for
cluster-2-cluster replication in one DC site.



--
View this message in context:


http://apache-geode-incubating-developers-forum.70738.x6.nabble.com/update-website-for-WAN-CQ-and-native-client-tp6659p6763.html

Sent from the Apache Geode (Incubating) Developers Forum mailing list
archive at Nabble.com.




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

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

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On June 10, 2016, 3:12 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48556/
> ---
> 
> (Updated June 10, 2016, 3:12 p.m.)
> 
> 
> Review request for geode, Udo Kohlmeyer and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Modified JavaApiIntegrationTest as it was pointing to 
> DistributedSystemConfigProperties to the renamed ConfigurationProperties
> 
> 
> Diffs
> -
> 
>   
> geode-spark-connector/geode-spark-connector/src/it/java/ittest/io/pivotal/geode/spark/connector/JavaApiIntegrationTest.java
>  1bae89b 
> 
> Diff: https://reviews.apache.org/r/48556/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: Review Request 48665: GEODE-1542 shared/unordered tcp/ip connection times out, initiating suspicion

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On June 13, 2016, 10:50 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48665/
> ---
> 
> (Updated June 13, 2016, 10:50 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1542
> https://issues.apache.org/jira/browse/GEODE-1542
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This disables timing out of shared/unordered TcpConduit connections.  We 
> don't want them to time out because we are using them to initiate suspect 
> processing on other members.
> 
> The ticket also pointed out a problem with the "final check" mechanism in the 
> health monitor.  I tracked that down to improper use of SocketCreator to 
> create the server-socket in GMSHealthMonitor.  It was creating sn SSL socket 
> if SSL is enabled but the client-side of the check uses non-SSL sockets.  I 
> changed the server to use non-SSL sockets as well since no useful information 
> is sent over the final-check TCP/IP connections & they need to be lightweight 
> and fast.
> 
> While looking at logs I also found that the heartbeat request sent at the 
> beginning of a final-check had a request-ID even though it's not waiting for 
> a response.  That causes processing of the response to do more work than 
> necessary so I changed it to remove the request-ID from the message.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
>  f27e0b8d238fd4cda3a81a5d1edf199ebeb1c3c7 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  8dce1a5eb46b26a7b9ecc3e5b538d98e7e9f720e 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messages/HeartbeatRequestMessage.java
>  3c08e3383c2bf8f53cca291825ddb45ef69d0184 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java 
> 367d4a7bbe1334ad393da337030a1d02089cfcaf 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java 
> 85e351106285db571ecc3f388bbb11979562c3ed 
> 
> Diff: https://reviews.apache.org/r/48665/diff/
> 
> 
> Testing
> ---
> 
> precheckin, SSL integration testing.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Partitioned Region With Overflow Behavior Question

[Udo Kohlmeyer]

Hi there Avinash,

Firstly I would change the critical-heap-percentage to something closer 
to 70 or 80. This setting is a "stop-the-world" percentage, where the 
amount of data inserted is greater than what can be evicted.


Also I would make sure that the CMSOccupancyPercentage is 3-5% less than 
the eviction setting. This way the JVM will try and do a GC before 
causing the cluster to evict data. Also depending on JVM size, you might 
want to increase the eviction percentage to maybe around 60-70%.


The behavior of the OVERFLOW* files are not deleted is correct. When you 
restart the server it will not use those files, but Geode does not 
delete them on shutdown.


As for the overflow files for the client, it does sound like expected 
behavior as well. I imagine that this behavior would be seen on the 
server as well, but you might not have noticed it.


If a region is not defined to be persistent, you can be assured that 
these files will not be once after a restart.


--Udo


On 11/06/2016 1:26 pm, Avinash Dongre wrote:

Hi All,
I have following scenario and would like to confirm the behavior

1. Region is PARTITION_OVERFLOW
2. Server is stated with --eviction-heap-percentage=50
--critical-heap-percentage=95
3. I started my puts into this region
4. After Some time Eviction starts and I see OVERFLOW* files are created
5. Delete the Region
Overflow files are not deleted , Is this expected Behavior ??

6. I start my client again ( I have not restarted the server ) , I see
immediately new OVERFLOW files are created

Is this expected behavior ?

Thanks
Avinash

GEODE-1377: DistributedSystemConfigProperties

[Udo Kohlmeyer]

Hi there Guys,

As per a previous thread, I've moved all the javadoc from 
DistributedSystem to DistributedSystemConfigProperties.


I have also now renamed DistributedSystemConfigProperties to 
ConfigurationProperties.


Please make sure that for all new tests and development that you 
reference the properties from the ConfigurationProperties file.


Thank you

--Udo

Re: Eclipse and IntelliJ formatters

[Udo Kohlmeyer]

I really don't have a preference in regards to import formatting, as 
long as Eclipse and Intellij do the same thing.


--Udo


On 10/06/2016 3:54 am, Kirk Lund wrote:

Sorry this is such a mess. Please vote on how you want it to work and then
after everyone finishes voting, I'll update both to be the same as each
other.

-Kirk


On Thu, Jun 9, 2016 at 10:53 AM, Kirk Lund  wrote:


The Eclipse formatter has now been updated to match the IntelliJ formatter
as closely as possible. Below is detailed descriptions of the changes:

1) rename Eclipse formatter Profile Name from GemFire to Geode

2) update Organize Imports to following order:

all unmatched static imports

java imports

javax imports

all unmatched type imports

batterytest.* (hydra)

cacheRunner.* (hydra)

hydra.* (hydra)

parReg.* (hydra)

perffmwk.* (hydra)

com.gemstone.* (geode/gemfire code)

com.vmware.gemfire.* (geode/gemfire code)

io.pivotal.geode.* (geode/gemfire code)

(each of the above groups are separated by one blank line -- this is same
as before)

The changes to the IntelliJ formatter:

1) rename IntelliJ formatter from gemfireCodeStyleScheme to
geodeCodeStyleScheme

2) update Import Layout to following order:

Import static all other imports

import java.*
import javax.*

import all other imports

import batterytest.*
import cacheRunner.*
import hydra.*
import parReg.*
import perffmwk.*

import com.gemstone.*
import com.vmware.gemfire.*
import io.pivotal.geode.*

I prefer the IntelliJ import layout, BUT I couldn't perfectly control
where the  are inserted in the Eclipse formatter. Apparently
every block of related imports just automatically gets surrounded by a
.

Should we import  between every block in IntelliJ to match
Eclipse?

-Kirk

Re: Review Request 48239: GEODE-1498 CI Failure: DurableClientCommandsDUnitTest.testCloseDurableClients

[Udo Kohlmeyer]

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


Ship it!





geode-cq/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DurableClientCommandsDUnitTest.java
 (line 248)
<https://reviews.apache.org/r/48239/#comment201231>

this could have been replaced with the lambda equivalent of ```java int 
listeningPort = server1.invoke("...", () -> 
startCacheServer(0,false,regionName))```


- Udo Kohlmeyer


On June 3, 2016, 11:42 p.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48239/
> ---
> 
> (Updated June 3, 2016, 11:42 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Use port 0 for cache server and return a random available for durable client.
> 
> 
> Diffs
> -
> 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/DurableClientCommandsDUnitTest.java
>  c51f875 
> 
> Diff: https://reviews.apache.org/r/48239/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: build error with development branch...

[Udo Kohlmeyer]

Maybe try 1.8.0_92... I know it works


On 3/06/2016 7:47 am, Dan Smith wrote:

Hmm, does that -ea mean it's an early access build? I would recommend
running with a later version of java 8.

-Dan

On Thu, Jun 2, 2016 at 2:41 PM, Anilkumar Gingade 
wrote:


If gradle is using the java installed/set in my environment, then it is:

java version "1.8.0_20-ea"
Java(TM) SE Runtime Environment (build 1.8.0_20-ea-b05)
Java HotSpot(TM) 64-Bit Server VM (build 25.20-b05, mixed mode)

I could not see any build output that printed java version it used (nice to
have, if its not there)...

-Anil.



On Thu, Jun 2, 2016 at 2:14 PM, Dan Smith  wrote:


Develop builds for me. And travis seems happy -
https://travis-ci.org/apache/incubator-geode

But this is actually pretty weird. In Intellij at least, it thinks that
lambda maps to a SerializableCallable even though it doesn't return a
value. I think maybe that's due to the while(true) part. If I comment

that

out, it maps to a runnable. It seems like this particular lambda actually
*is* ambiguous since it will never return normally, it could be either a
callable or a runnable.

What JDK and what revision are you building with? Maybe some newer JDK is
complaining about this?

-Dan

-Dan

On Thu, Jun 2, 2016 at 1:58 PM, Anilkumar Gingade 
wrote:


Hi Devs,

Anyone seeing this issue:




:geode-core:compileTestJava/export/india1/users/agingade/src/gemfire/open/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MultiUserDUnitTest.java:62:

error: reference to invokeAsync is ambiguous

 AsyncInvocation vm1Invoke = vm1.invokeAsync("run as data-reader",

()

->

{

^

   both method invokeAsync(String,SerializableRunnableIF) in VM and

method

invokeAsync(String,SerializableCallableIF) in VM match

   where T is a type-variable:

 T extends Object declared in method
invokeAsync(String,SerializableCallableIF)




/export/india1/users/agingade/src/gemfire/open/geode-core/src/test/java/com/gemstone/gemfire/security/templates/LdapUserAuthenticator.java:89:

warning: LdapCtxFactory is internal proprietary API and may be removed

in a

future release

 env.put(Context.INITIAL_CONTEXT_FACTORY,
com.sun.jndi.ldap.LdapCtxFactory.class.getName());

   ^

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

1 error


-Anil.

Re: build error with development branch...

[Udo Kohlmeyer]

I think that you getting this because vm1.invokeAsync(... seems to need 
to return something but it does not.


Maybe remove the 'AsynInvocation vm1Invoke =' part... As it serves no 
purpose here.


--Udo

On 3/06/2016 6:58 am, Anilkumar Gingade wrote:

Hi Devs,

Anyone seeing this issue:

:geode-core:compileTestJava/export/india1/users/agingade/src/gemfire/open/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MultiUserDUnitTest.java:62:
error: reference to invokeAsync is ambiguous

 AsyncInvocation vm1Invoke = vm1.invokeAsync("run as data-reader", () ->
{

^

   both method invokeAsync(String,SerializableRunnableIF) in VM and method
invokeAsync(String,SerializableCallableIF) in VM match

   where T is a type-variable:

 T extends Object declared in method
invokeAsync(String,SerializableCallableIF)

/export/india1/users/agingade/src/gemfire/open/geode-core/src/test/java/com/gemstone/gemfire/security/templates/LdapUserAuthenticator.java:89:
warning: LdapCtxFactory is internal proprietary API and may be removed in a
future release

 env.put(Context.INITIAL_CONTEXT_FACTORY,
com.sun.jndi.ldap.LdapCtxFactory.class.getName());

   ^

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

1 error


-Anil.

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

[Udo Kohlmeyer]

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


Ship it!




Once CACHE_XML_FILE has referenced the correct 
DistributedSystemConfigProperties.

- Udo Kohlmeyer


On June 2, 2016, 5:07 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48176/
> ---
> 
> (Updated June 2, 2016, 5:07 p.m.)
> 
> 
> Review request for geode, William Markito, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The spark build is currently broken due to an invalid property
> 
> 
> Diffs
> -
> 
>   
> geode-spark-connector/geode-spark-connector/src/it/java/ittest/io/pivotal/geode/spark/connector/JavaApiIntegrationTest.java
>  1c6a5a2 
> 
> Diff: https://reviews.apache.org/r/48176/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

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

[Udo Kohlmeyer]

> On June 2, 2016, 5:16 p.m., Dan Smith wrote:
> > geode-spark-connector/geode-spark-connector/src/it/java/ittest/io/pivotal/geode/spark/connector/JavaApiIntegrationTest.java,
> >  line 62
> > <https://reviews.apache.org/r/48176/diff/1/?file=1405059#file1405059line62>
> >
> > Should this use DistributedSystemConfigProperties.CACHE_XML_FILE 
> > instead?

Agreed, this should be DistributedSystemConfigProperties.CACHE_XML_FILE.


- Udo


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


On June 2, 2016, 5:07 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48176/
> ---
> 
> (Updated June 2, 2016, 5:07 p.m.)
> 
> 
> Review request for geode, William Markito, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The spark build is currently broken due to an invalid property
> 
> 
> Diffs
> -
> 
>   
> geode-spark-connector/geode-spark-connector/src/it/java/ittest/io/pivotal/geode/spark/connector/JavaApiIntegrationTest.java
>  1c6a5a2 
> 
> Diff: https://reviews.apache.org/r/48176/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: DistributionConfig and Geode system properties

[Udo Kohlmeyer]

John: Perhaps the (interface) name can be simplified to ConfigurationProperties 
too.

Funnily enough I initially had called it SystemConfigurationProperties, 
but later renamed it because it felt too generic.


Dan: 3) DistributedSystem has a ton of javadocs describing each property and 
what it does. I wonder if those javadocs should move to the constants in this 
interface instead?

+1, Currently the Javadoc links back to the DistributedSystem javadoc, 
also happy to move that.


DAN: 1) Is the idea with the change that all code should reference the 
constants in DistributedSystemConfigProperties. For example, I should use 
DistributedSystemConfigProperties.CACHE_XML_FILE when writing a test, rather 
than DistributionConfig.CACHE_XML_FILE? In that case, maybe DistributionConfig 
should not extend DistributedSystemConfigProperties?

Correct, when referencing CACHE_XML_FILE, it should come from DSCP. The 
refactor started with the implementation of the interface, but we should 
cut the ties. Also with the current refactor I've tried to do minimally 
invasive surgery.
Step1: was to extract all the properties into a single location and 
*hopefully* fix all code referencing it.
Step2: Investigate the possibility to move/refactor the attribute 
configuration logic (attr type,min,max,default). Currently none of that 
information is directly linked to the DSCP properties.


Bruce: Also, CacheFactory has two methods that should point to this new class:  
CacheFactory(Properties) and set(String,String).

Good point, we should change those links. I cannot help other than to 
ask the question, should the ConfigurationProperties not be an Enum? 
Then the */set(String,Sting)/* could become 
/*set(ConfigurationProperty,String) */which could make it even easier to 
configure the system when using the API. If we were to use the enum, we 
could set up (type,min,max,default) in a single location for consumption 
and documentation.


--Udo

On 3/06/2016 4:34 am, Darrel Schneider wrote:

+1 to naming it "ConfigurationProperties"
+1 to moving the javadocs


On Thu, Jun 2, 2016 at 10:46 AM, John Blum  wrote:


Perhaps the (interface) name can be simplified to ConfigurationProperties
too.  Not all properties necessarily involve specifically the distributed
system configuration, but rather the overall Geode system configuration
(Cache, Management, HTTP Service(s), Clients, etc).

On Thu, Jun 2, 2016 at 10:27 AM, Dan Smith  wrote:


First off - nice job, these constants should have been available a
long time ago!

One question a couple of comments:

1) Is the idea with the change that all code should reference the
constants in DistributedSystemConfigProperties. For example, I should
use DistributedSystemConfigProperties.CACHE_XML_FILE when writing a
test, rather than DistributionConfig.CACHE_XML_FILE? In that case,
maybe DistributionConfig should not extend
DistributedSystemConfigProperties?

2) Add @since Geode 1.0 to this interface, as well as some javadocs
describing to users what's contained in this interface.

3) DistributedSystem has a ton of javadocs describing each property
and what it does. I wonder if those javadocs should move to the
constants in this interface instead?

-Dan


On Wed, Jun 1, 2016 at 5:17 PM, Udo Kohlmeyer 
wrote:

Hi there,

As per GEODE-1377 <https://issues.apache.org/jira/browse/GEODE-1377>

I've

refactored DistributionConfig to extract all public system properties

into a

public DistributedSystemConfigProperties interface.

With that refactor I've touched a significant amount of classed and I,

in

advance, apologize for the merging.

I've have tried to find all references to all the properties that we

use

within Geode (be it main and test).

For future development, if we are to use and system properties, that we

use

the provided definition for that property. That will help us to

understand

where the properties are used and will make it easier if we need to

refactor

anything in the future.

I've also tried to find every reference to "gemfire." and replaced it

with

its defined counterpart. This should hopefully help us if we were to

move

away from property definitions like "gemfire.locators" to

"geode.locators".

If someone were to find that I had missed an obvious property to please
refactor and resolve that issue.

In addition to this effort I have noticed that we use a large amount of
properties to configure functionality. I respectfully request from all
developers that if you were to work on some code that uses "internal"
properties to maybe pull it out into a common interface which can be

reused

throughout the code base.

--Udo





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

DistributionConfig and Geode system properties

[Udo Kohlmeyer]

Hi there,

As per GEODE-1377  
I've refactored DistributionConfig to extract all public system 
properties into a public DistributedSystemConfigProperties interface.


With that refactor I've touched a significant amount of classed and I, 
in advance, apologize for the merging.


I've have tried to find all references to all the properties that we use 
within Geode (be it main and test).


For future development, if we are to use and system properties, that we 
use the provided definition for that property. That will help us to 
understand where the properties are used and will make it easier if we 
need to refactor anything in the future.


I've also tried to find every reference to "gemfire." and replaced it 
with its defined counterpart. This should hopefully help us if we were 
to move away from property definitions like "gemfire.locators" to 
"geode.locators". If someone were to find that I had missed an obvious 
property to please refactor and resolve that issue.


In addition to this effort I have noticed that we use a large amount of 
properties to configure functionality. I respectfully request from all 
developers that if you were to work on some code that uses "internal" 
properties to maybe pull it out into a common interface which can be 
reused throughout the code base.


--Udo

Re: Review Request 47908: GEODE-1460 RemoveAll fails with NPE in com.gemstone.gemfire.internal.cache.tier.sockets.CacheClientNotifier.checkAndRemoveFromClientMsgsRegion()

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 26, 2016, 8:34 p.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47908/
> ---
> 
> (Updated May 26, 2016, 8:34 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Bruce Schuchardt, Hitesh 
> Khamesra, Udo Kohlmeyer, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add a lock to synchronize two getInstance methods, so that they return either 
> null ccnSingleton or fully initialized ccnSingleton, i.e. haContainer is not 
> null if it is not gateway receiver.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java
>  62336f0 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java
>  80d05ba 
> 
> Diff: https://reviews.apache.org/r/47908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: Review Request 48095: GEODE-1468 client/server messaging can create large objects

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 31, 2016, 10:15 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48095/
> ---
> 
> (Updated May 31, 2016, 10:15 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1468
> https://issues.apache.org/jira/browse/GEODE-1468
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> After a Message has been sent we invoke clear() on each Part contained by the 
> Message.  This was nulling out the "part" variable of the Part objects but if 
> one of these "parts" was a HeapDataOutputStream it might hold a list of large 
> buffers.  This change set alters Part to close these streams so that their 
> buffers can be cleared.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/HeapDataOutputStream.java
>  20d01da880f2786a01ee4d4bd64681cd646acd31 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java
>  a011875d4ea9aa9a14ac96e568fe6bba464bca89 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java
>  bf90fab4999ce96adced9574678605d2bf8a903a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java
>  9f05aa7a0ae0d433ff9675a5fbcffc9c98ce8e7b 
> 
> Diff: https://reviews.apache.org/r/48095/diff/
> 
> 
> Testing
> ---
> 
> New test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 47245: GEODE-1327 java.util.ConcurrentModificationException while updating log message

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 11, 2016, 5:55 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47245/
> ---
> 
> (Updated May 11, 2016, 5:55 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Now parent thread process copy of unResponsive members
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  88e4d49 
> 
> Diff: https://reviews.apache.org/r/47245/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 47501: GEODE-1387 CI Failure: LocatorLauncherRemoteFileIntegrationTest.testStatusUsingWorkingDirectory

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 17, 2016, 11:49 p.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47501/
> ---
> 
> (Updated May 17, 2016, 11:49 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, Kirk Lund, and 
> Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The locator status file writer and reader are out of sync. The reader may 
> read the empty file before writer writes to it. So write the status to a temp 
> file, then rename it. So that the reader read the status file that has 
> content in it.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/process/ControllableProcess.java
>  f459aed 
> 
> Diff: https://reviews.apache.org/r/47501/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: Review Request 47504: GEODE-1133 SeparateClassloaderTestRunner has to be re-implemented

[Udo Kohlmeyer]

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


Ship it!




I don't know anything of this class. So if it is genuinely not used anymore, 
I'm happy that we remove it.

- Udo Kohlmeyer


On May 18, 2016, 12:01 a.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47504/
> ---
> 
> (Updated May 18, 2016, 12:01 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, Jens Deppe, and 
> Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This class is no longer needed. So remove it.
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules-session/src/test/java/com/gemstone/gemfire/modules/session/junit/SeparateClassloaderTestRunner.java
>  4337f5a 
> 
> Diff: https://reviews.apache.org/r/47504/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: Review Request 47323: GEODE-732 Unable to create PDXInstance from valid JSON using JSONFormatter

[Udo Kohlmeyer]

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



I don't see the test class for this. Can you please attach it.

- Udo Kohlmeyer


On May 12, 2016, 8:10 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47323/
> ---
> 
> (Updated May 12, 2016, 8:10 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> We were not handling list inside list immediately. Fixed it in PdxListHelper, 
> where we add java list in parent list.
> Added test for it. Added one more state LIST_ENDS for readability purpose
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/com/gemstone/gemfire/pdx/JSONFormatter.java 
> 220deaf 
>   
> geode-core/src/main/java/com/gemstone/gemfire/pdx/internal/json/PdxListHelper.java
>  7700b30 
>   
> geode-core/src/test/resources/com/gemstone/gemfire/pdx/jsonStrings/jsonListInsideList.txt
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47323/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 47195: GEODE-699 PartitionedRegionSingleHopDUnitTest.test_MetadataServiceCallAccuracy

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 12, 2016, 10:45 p.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47195/
> ---
> 
> (Updated May 12, 2016, 10:45 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When the fetch task threads are spawned in the client metadata service for 
> single hop, It is possible that certain thread is still lingering during the 
> second round of PUTs, which assumes all such fetch tasks should be completed 
> at that time. The lingering thread causes the flag to be set, which results 
> in test failure. 
> 
> The fix tries to shutdown the ExecutorService and make sure all fetch task 
> threads are terminated before the second round of PUTs. So that there is no 
> unexpected flag change in the second round.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ClientMetadataService.java
>  ed26708 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionSingleHopDUnitTest.java
>  f79d6c6 
> 
> Diff: https://reviews.apache.org/r/47195/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: Review Request 47189: GEODE-1375 When using multicast a new member needs to receive the multicast message digest

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 11, 2016, 5:05 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47189/
> ---
> 
> (Updated May 11, 2016, 5:05 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This reinstates the sending of JoinResponseMessages so that the new member 
> can get the jgroups multicast digest.  The JoinResponseMessages are sent 
> after installing the new membership view, so JGroupsMessenger has been 
> changed to use MERGE_VIEW instead of SET_VIEW to install the digest since it 
> may have already received multicast messages from some members.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  88e4d496e5d89f2b84d5e755fc6471c8790ed98f 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  4a54e8433ccf0bcfdacc3ccc6e790381da6e236a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>  50bed13419d1ed68a405a9f5ed5d5543c3d98813 
> 
> Diff: https://reviews.apache.org/r/47189/diff/
> 
> 
> Testing
> ---
> 
> precheckin is underway.  There is already a unit test checking that a digest 
> is added to a JoinResponseMessage so I didn't need to write one.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Unable to run single dunit

[Udo Kohlmeyer]

When I did this, I had to "wrap" the class with '*'

The command is then.

./gradlew
-DdistributedTest.single=*ListAndDescribeDiskStoreCommandsDUnitTest*

On 5/05/2016 7:38 am, Kirk Lund wrote:

This used to work quite well for me. Why am I not able to run a single
dunit on the command-line now? Is this caused by the gradle upgrade?

/Users/klund/dev/gemfire/open [520]$ find . -name
ListAndDescribeDiskStoreCommandsDUnitTest.java

./geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/ListAndDescribeDiskStoreCommandsDUnitTest.java

/Users/klund/dev/gemfire/open [521]$ ./gradlew
-DdistributedTest.single=ListAndDescribeDiskStoreCommandsDUnitTest
geode-core:distributedTest

:buildSrc:compileJava UP-TO-DATE
:buildSrc:compileGroovy UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar UP-TO-DATE
:buildSrc:assemble UP-TO-DATE
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:compileTestGroovy UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build UP-TO-DATE
:geode-common:compileJava UP-TO-DATE
:geode-common:processResources UP-TO-DATE
:geode-common:classes UP-TO-DATE
:geode-common:jar UP-TO-DATE
:geode-joptsimple:compileJava UP-TO-DATE
:geode-joptsimple:processResources UP-TO-DATE
:geode-joptsimple:classes UP-TO-DATE
:geode-joptsimple:jar UP-TO-DATE
:geode-json:compileJava UP-TO-DATE
:geode-json:processResources UP-TO-DATE
:geode-json:classes UP-TO-DATE
:geode-json:jar UP-TO-DATE
:geode-core:compileJava UP-TO-DATE
:geode-core:createVersionPropertiesFile UP-TO-DATE
:geode-core:processResources UP-TO-DATE
:geode-core:classes UP-TO-DATE
:geode-junit:compileJava UP-TO-DATE
:geode-junit:processResources UP-TO-DATE
:geode-junit:classes UP-TO-DATE
:geode-junit:jar UP-TO-DATE
:geode-core:compileTestJava UP-TO-DATE
:geode-core:processTestResources UP-TO-DATE
:geode-core:testClasses UP-TO-DATE
:geode-core:distributedTest FAILED
:combineReports UP-TO-DATE

FAILURE: Build failed with an exception.

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

Could not find matching test for pattern:

ListAndDescribeDiskStoreCommandsDUnitTest

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

BUILD FAILED

Total time: 3.364 secs

Re: Review Request 46940: GEODE-1329 auto-reconnect attempts cease if kicked out during boot-up of the cache

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 3, 2016, 3:43 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46940/
> ---
> 
> (Updated May 3, 2016, 3:43 p.m.)
> 
> 
> Review request for geode and Jianxia Chen.
> 
> 
> Bugs: GEODE-1329
> https://issues.apache.org/jira/browse/GEODE-1329
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is a follow-up to the fix for GEODE-1329 that removes the old 
> reconnectCancelledLock variable and makes reconnectCancelled volatile.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
>  df854170600d7714fff8275edc3db5686d9ec915 
> 
> Diff: https://reviews.apache.org/r/46940/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 46801: GEDOE-1321 GMSHealthMonitor: java.lang.IllegalStateException: Timer already cancelled

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On April 29, 2016, 4:47 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46801/
> ---
> 
> (Updated April 29, 2016, 4:47 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Caught that exception.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
>  2d0f039 
> 
> Diff: https://reviews.apache.org/r/46801/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got java.util.ConcurrentModificationException while updating log message

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> ---
> 
> (Updated May 2, 2016, 4:36 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> added synchronization with copyOnread
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  9f5648b 
> 
> Diff: https://reviews.apache.org/r/46896/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got java.util.ConcurrentModificationException while updating log message

[Udo Kohlmeyer]

> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java,
> >  line 1703
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1703>
> >
> > This can potentially cause a huge amount of unnecessary garbage. Each 
> > time this method is called we create a new HashSet.
> 
> Hitesh Khamesra wrote:
> i don't think so, it will called after 1 second period or when new member 
> will be added

We don't know how long something might run. Which means, it might run a few 
times. I'm just conscious that it might run multiple times, and creating new 
HashSets each time just creates more garbage.

But happy for you to leave it if you think it will not cause issues.


> On May 2, 2016, 7:29 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java,
> >  line 1742
> > <https://reviews.apache.org/r/46896/diff/1/?file=1368587#file1368587line1742>
> >
> > I don't think we need to explicitly create a new HashSet for a getter. 
> > If this set needs to be "cloned" then make it the responsibility of the 
> > method invoking this function to do so.
> 
> Hitesh Khamesra wrote:
> We can do that way as well. But then every accesor has to do.

I'm torn on this one... I don't see a need to force a new collection everytime, 
 but on the other hand.. being safe hasn't killed anyone.


- Udo


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


On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> ---
> 
> (Updated May 2, 2016, 4:36 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> added synchronization with copyOnread
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  9f5648b 
> 
> Diff: https://reviews.apache.org/r/46896/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 46896: GEODE-1327 GMSJoinLeave: got java.util.ConcurrentModificationException while updating log message

[Udo Kohlmeyer]

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




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

I don't understand why this needs to be called here as well as in the while 
block. You could initialize it here,but I don't think we need the extra (new 
HashSet()) call here as well. This call is not required.



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

This can potentially cause a huge amount of unnecessary garbage. Each time 
this method is called we create a new HashSet.



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

I don't think we need synchronized here. There is no chance for any 
concurrent modification here. Even with mutliple threads this is still just a 
getter invocation, there is no danger to concurrently modify anything.



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

I don't think we need to explicitly create a new HashSet for a getter. If 
this set needs to be "cloned" then make it the responsibility of the method 
invoking this function to do so.


- Udo Kohlmeyer


On May 2, 2016, 4:36 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46896/
> ---
> 
> (Updated May 2, 2016, 4:36 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> added synchronization with copyOnread
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  9f5648b 
> 
> Diff: https://reviews.apache.org/r/46896/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: [DISCUSS] What to do with the partial HDFS related code on develop?

[Udo Kohlmeyer]

+1

On 28/04/2016 7:16 am, Dan Smith wrote:

This is done. The HDFS related changes are gone from develop and are now on
their own feature branch - feature/GEODE-10.

-Dan

On Sat, Mar 19, 2016 at 2:05 AM,  wrote:


+1
a possible HDFS integration should also be an optional jar plugin package
in the distribution as Fred started the discussion to split jars anyway.

Best Regards
Manuel Birke

Mercedes-Benz Research & Development North America, Inc.
Address: 309 N Pastoria Ave | Sunnyvale, CA 94085
Mobile: +1 408-203-6026
Phone: +1 408-991-6502

From: Anilkumar Gingade
Sent: ‎3/‎19/‎2016 0:05
To: dev@geode.incubator.apache.org
Subject: Re: [DISCUSS] What to do with the partial HDFS related code on
develop?

+1
Good idea...It will allow us to re-visit the HDFS implementation and right
way to provide this support...

-Anil.






On Fri, Mar 18, 2016 at 3:56 PM, Dan Smith  wrote:


When geode was donated to apache, there was a partially completed
integration with HDFS built in. See GEODE-10.

Since that initial donation, we have removed the APIs related to creating
HDFS regions because they were not yet stable. See GEODE-429.

The problem is that we still have HDFS related code sprinkled throughout
geode-core. This causes us to pull in a bunch of compile dependencies on
hbase and hadoop.

I already created an issue, GEODE-1072, to move the HDFS code it it's own
subproject, but that will take a lot of work.

In the mean time, I propose we go ahead and create a separate branch for
the HDFS related changes and remove the HDFS related code from develop.

The

work to refactor the HDFS integration to be a subproject can take place

on

that branch and geode-core on develop will be cleaner in the mean time.

What do you folks think?

-Dan


If you are not the addressee, please inform us immediately that you have
received this e-mail by mistake, and delete it. We thank you for your
support.

Re: @since tags in our javadocs - old gemfire vs. geode versions

[Udo Kohlmeyer]

Just for my own curiosity... why do we need the @since tag?
What benefit does it provide the product/code?

--Udo

On 27/04/2016 7:43 am, Dan Smith wrote:

It sounds like more people are in favor of Geode 1.0.0 and GemFire x.y.z,
so I created bug GEODE-1316 to implement this change.

-Dan

On Mon, Apr 25, 2016 at 6:36 PM, Udo Kohlmeyer 
wrote:


I must be honest that I've never been a supporter of the @since tag. Imo,
release notes and features should be the paper trail that we provide. Also,
how would we handle a scenario where a class is denoted with @since 6.5.x
and all internals of that class are completely new and replaced with 8.2.x
or even 9.0 (or 1.0 Geode) code?

I think that @since tags become like comments, without somebody changing
things, they just become stale and stagnant. I cannot think of many open
source projects that use @since tags.

But if we must have them in the code base then I prefer to have them as a
Geode x.y.z


On 26/04/2016 9:56 am, Darrel Schneider wrote:


+1 for having on explicit GemFire and Geode


On Mon, Apr 25, 2016 at 4:43 PM, Kenneth Howe  wrote:

+1 to “gemFire x.y.z”

Adding the GemFire makes it obvious where the feature came from, no
inference
required as would happen if we left just a version number for old @since
annotations.

Ken

On Apr 25, 2016, at 4:39 PM, Kirk Lund  wrote:

+1 for @since Geode 1.0.0

If we keep the pre-existing @since tags, then I'd prefer to add
"GemFire"
to them for better clarity. Thus, @since 4.0.0 would be changed to
@since
GemFire 4.0.0. Just my preference.

-Kirk


On Mon, Apr 25, 2016 at 4:32 PM, Sai Boorlagadda <


sboorlaga...@pivotal.io>


wrote:

+1 for Geode 1.0.0

And we can leave current @since tags as-is with out "GemFire" to denote
predate Geode.
So if you see "Geode x.y.z" => added in Geode
   or   "x.y.z" => Predate to Geode (i.e.,)
GemFire.


On Fri, Apr 22, 2016 at 3:37 PM, John Blum  wrote:

+1 for @since Geode 1.0.0.

@since GemFire x.y.z is probably not all that useful from a Geode
perspective, but maybe important in GemFire source, particularly for
features that maybe specific to GemFire, or predate Geode.

On Fri, Apr 22, 2016 at 3:11 PM, Dan Smith <
upthewatersp...@apache.org
wrote:

Hi,

We have a lot of @since tags in our javadocs with old gemfire
versions. I think we are going to keep them in there, we should maybe
do a sweep and add gemfire to the version:

Eg
@since GemFire 5.5

For geode @since tags, we can start from 1.0:
@since 1.0

Or maybe it would be better to be explicit?
@since Geode 1.0

What do you guys think?
-Dan



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



--
Sai Boorlagadda