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 <u...@apache.org> wrote:

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

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

--Udo


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

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

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


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

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

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

The build failed with multiple formatting error on each file.

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

Regards
Nabarun

On Mon, Oct 24, 2016 at 3:50 PM Bruce Schuchardt <bschucha...@pivotal.io>
wrote:


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

On Mon, Oct 24, 2016 at 3:47 PM, Dan Smith <dsm...@pivotal.io> 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 <jstew...@pivotal.io> wrote:


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



On Oct 21, 2016, at 4:16 PM, Kirk Lund <kl...@pivotal.io> 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 <jstew...@pivotal.io>

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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io>

wrote:

Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com>

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 <jstew...@pivotal.io

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 <asf.mbr...@gmail.com>

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 <kh...@pivotal.io>

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 <kl...@apache.org>

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 <kl...@pivotal.io>

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

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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io> wrote:


Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io>

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 <asf.mbr...@gmail.com> 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 <kh...@pivotal.io>

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 <kl...@apache.org>

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 <kl...@pivotal.io>

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 <dsm...@pivotal.io>

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 <dsm...@pivotal.io>

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

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]

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",
-
-   };
+"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 <kdul...@pivotal.io>

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 <kh...@pivotal.io>

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

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 <bschucha...@pivotal.io>
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 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 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/src/test/ja

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: 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 <ukohlme...@pivotal.io> 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 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 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 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

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

/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: 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 <kl...@pivotal.io> 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 <bschucha...@pivotal.io>
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
> 
>

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: 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 <jb...@pivotal.io> 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 <dsm...@pivotal.io> 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 <ukohlme...@pivotal.io>
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: 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: [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 <ukohlme...@pivotal.io>
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 <kh...@pivotal.io> 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 <kl...@pivotal.io> 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 <jb...@pivotal.io> 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

Re: Review Request 46590: GEODE-1237 remove the check for network-partition-detection uniformity

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On April 22, 2016, 10:29 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46590/
> ---
> 
> (Updated April 22, 2016, 10:29 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Each member now determines whether to force shutdown due to loss of quorum.  
> They also decide whether locators are preferred as membership coordinators.  
> This primarily meant removing the method 
> GMSJoinLeave.inheritSettingsFromLocator() but the ID settings needed to be 
> made in other places (JGroupsMessenger.establishLocalAddress() and 
> GMSJoinLeave.started()).
> 
> I also noticed that some of the code was still using "splitBrain" and "SB" 
> instead of networkPartitionDetection and "NPD" and fixed that.
> 
> One test had to be removed from LocatorDUnitTest and a couple of others 
> needed to be adjusted to have all of the members set 
> network-partition-detection-enabled=true.  Precheckin is running and I'll fix 
> any other tests that have this problem.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
>  a5d269b1e59b451e30ca7e0400fdb8f2ccfc515c 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/InternalDistributedMember.java
>  81058f85b814be7387808b691e13b8c841148544 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/NetMember.java
>  43f3cf173e3cc5f941fc6ab9e1a3a20cd51c111a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/GMSMember.java
>  84ace6ce1826369be1cd94a4d3b98e02b93b6671 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/ServiceConfig.java
>  9f0bbf78fee922bc9d841cddaab71b2313aa53ab 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java
>  c8b6aaae6b16c01b265674f2b5438f5adcb5edf4 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  05350e5e99a58b7dea7a60ba8da7d2699a348562 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  a9abea3b442612db467bd3b5a390ef955924cdba 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/i18n/ParentLocalizedStrings.java
>  8285a651b6e0285d14ea3f82b5eb73460bd6fa56 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
>  702a8592735456b07e05d46d5301043ed9f674f3 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
>  bf13420b84206227230adc48165922d2126e6272 
> 
> Diff: https://reviews.apache.org/r/46590/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: [DISCUSS] Geode Examples (GEODE-33)

[Udo Kohlmeyer]

Hey William,

I'm assuming that these examples would both be in Java and Native Client 
(where applicable)?


--Udo

On 21/04/2016 3:14 pm, William Markito wrote:

Hey folks,

Here is a proposal for the initial README.md file and structure for Geode
examples subproject.

Ideally each item on the list would become a project on its own, very
similar to what Spring Data GemFire and I've also added a few items from
the old GemFire examples.

This would also be a good list of easy and helpful contributions for people
that would like to help in the project.

Thoughts ?

Patch for review - https://reviews.apache.org/r/46474/

--
# Apache Geode examples

This is the home of Apache Geode examples that are bundled with the
project. Contributions[2] and corrections are as usual welcome
and if you have any suggestions come talk to us at [
dev@geode.incubator.apache.org](mailto:dev@geode.incubator.apache.org) or
just submit a [pull request](
https://github.com/apache/incubator-geode/pull/new/develop).

## Requirements

All examples:
  * Needs to be testable. (unit tests, integration tests or what's
applicable) - Tests will ran through project CI.
  * Should be `Gradle` projects or part of existing ones. There could
be a few exceptions here, but community should have consensus to
accept
  * Have to follow code format & style from Apache Geode [1]
guidelines.
  * Should contain a `README.md` file with step-by-step instruction on how
to setup and run. (Diagrams give extra credits).
  * Donations need to be licensed through ASL 2.0 and contributors need to
file an ICLA[3]

## Structure

### Quick start & Installation

   [Link to the docs page]

### Basics

   * Replicated Region
   * Partitioned Region
   * Persistence
   * OQL (Querying)

### Intermediate

   * PDX & Serialization
   * Functions
   * CacheLoader & CacheWriter
   * Listeners
   * Async Event Queues
   * Continuous Querying
   * Transactions
   * Eviction
   * Expiration
   * Overflow
   * Security
   * Off-heap

### Advanced

   * WAN Gateway
   * Durable subscriptions
   * Delta propagation
   * Network partition detection
   * D-lock
   * Compression
   * Resource manager

### Use cases & integrations

  This section should have self contained little projects that illustrate a
use case or integration with some other projects.

   * SpringBoot Application
   * HTTP Session replication
   * Redis
   * Memcached
   * Spark Connector

## References

- [1] -
https://cwiki.apache.org/confluence/display/GEODE/Criteria+for+Code+Submissions
- [2] - https://cwiki.apache.org/confluence/display/GEODE/How+to+Contribute
- [3] - http://www.apache.org/licenses/#clas

Re: [DISCUSS] Geode Examples (GEODE-33)

[Udo Kohlmeyer]

Looks good.

I think that PDX is both and Intermediate and Advanced topic. As PDX can 
be a simple tool to use, like ReflectionBasedSerialization or custom 
PDXSeraliazation. But PDX can also be more complex with one using the 
PDXInstanceFactory.


--Udo

On 21/04/2016 3:14 pm, William Markito wrote:

Hey folks,

Here is a proposal for the initial README.md file and structure for Geode
examples subproject.

Ideally each item on the list would become a project on its own, very
similar to what Spring Data GemFire and I've also added a few items from
the old GemFire examples.

This would also be a good list of easy and helpful contributions for people
that would like to help in the project.

Thoughts ?

Patch for review - https://reviews.apache.org/r/46474/

--
# Apache Geode examples

This is the home of Apache Geode examples that are bundled with the
project. Contributions[2] and corrections are as usual welcome
and if you have any suggestions come talk to us at [
dev@geode.incubator.apache.org](mailto:dev@geode.incubator.apache.org) or
just submit a [pull request](
https://github.com/apache/incubator-geode/pull/new/develop).

## Requirements

All examples:
  * Needs to be testable. (unit tests, integration tests or what's
applicable) - Tests will ran through project CI.
  * Should be `Gradle` projects or part of existing ones. There could
be a few exceptions here, but community should have consensus to
accept
  * Have to follow code format & style from Apache Geode [1]
guidelines.
  * Should contain a `README.md` file with step-by-step instruction on how
to setup and run. (Diagrams give extra credits).
  * Donations need to be licensed through ASL 2.0 and contributors need to
file an ICLA[3]

## Structure

### Quick start & Installation

   [Link to the docs page]

### Basics

   * Replicated Region
   * Partitioned Region
   * Persistence
   * OQL (Querying)

### Intermediate

   * PDX & Serialization
   * Functions
   * CacheLoader & CacheWriter
   * Listeners
   * Async Event Queues
   * Continuous Querying
   * Transactions
   * Eviction
   * Expiration
   * Overflow
   * Security
   * Off-heap

### Advanced

   * WAN Gateway
   * Durable subscriptions
   * Delta propagation
   * Network partition detection
   * D-lock
   * Compression
   * Resource manager

### Use cases & integrations

  This section should have self contained little projects that illustrate a
use case or integration with some other projects.

   * SpringBoot Application
   * HTTP Session replication
   * Redis
   * Memcached
   * Spark Connector

## References

- [1] -
https://cwiki.apache.org/confluence/display/GEODE/Criteria+for+Code+Submissions
- [2] - https://cwiki.apache.org/confluence/display/GEODE/How+to+Contribute
- [3] - http://www.apache.org/licenses/#clas

Re: Review Request 46474: GEODE-33 - Initial structure for geode-examples with list of examples

[Udo Kohlmeyer]

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




geode-examples/README.md (line 29)
<https://reviews.apache.org/r/46474/#comment193491>

I think this could be both an Intermediate topic and Advanced.
Basic PDX usage like ReflectionBased and PdxSerializer is Intermediate 
topic.
But you can then have Advanced topic where you then work with the 
PDXInstanceFactory.


- Udo Kohlmeyer


On April 21, 2016, 5:13 a.m., William Markito wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46474/
> ---
> 
> (Updated April 21, 2016, 5:13 a.m.)
> 
> 
> Review request for geode, Anthony Baker and Dan Smith.
> 
> 
> Bugs: GEODE-33
> https://issues.apache.org/jira/browse/GEODE-33
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Initial structure for geode-examples with list of examples
> 
> 
> Diffs
> -
> 
>   geode-examples/README.md PRE-CREATION 
>   geode-examples/build.gradle PRE-CREATION 
>   geode-examples/gradle.properties PRE-CREATION 
>   settings.gradle c579dce 
> 
> Diff: https://reviews.apache.org/r/46474/diff/
> 
> 
> Testing
> ---
> 
> Build with latest snapshot
> Build with specific geode version from command line (gradle build 
> -PgeodeVersion=X.Y.Z)
> 
> 
> Thanks,
> 
> William Markito
> 
>

Re: Review Request 46457: GEODE-1268: Cleanup of multiple AvailablePort.getRandomAvailablePort invocations

[Udo Kohlmeyer]

> On April 20, 2016, 10:45 p.m., Bruce Schuchardt wrote:
> > Looks okay but using keepers would be safer.  When a locator starts, for 
> > instance, it consumes more than 1 tcp/ip port so it might use one of the 
> > others that you got from getRandomAvailableTcpPorts().

Using Keepers is a great idea. What would be required is an @After which will 
release all keepers. If one would do this is code we would have to have 
try-catch-finally. Where we release the keeper in the finally.. Otherwise the 
test won't clean up after itself


- Udo


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


On April 21, 2016, 1:58 a.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46457/
> ---
> 
> (Updated April 21, 2016, 1:58 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1268: Cleanup of multiple AvailablePort.getRandomAvailablePort 
> invocations with correct AvailablePortHelper invocation
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java
>  3b5f23fcc3bcbf1f1d83c0d491636203e221cfc2 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/ClientAuthorizationDUnitTest.java
>  961e8446628cd18bdbd847abf0b105cdfb020565 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/ClientAuthorizationTestCase.java
>  4cc8155328e40fb0ccba4d93ec6f9886436fe5ff 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/DeltaClientPostAuthorizationDUnitTest.java
>  61ff55adbd8fb1f73ba77eccb5d72641df88c867 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/P2PAuthenticationDUnitTest.java
>  56f5186685e60ff8f0827f9b26332c59f65d89cd 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/security/ClientAuthzObjectModDUnitTest.java
>  012ac332c181454ff68a5d1b60df773402cce756 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/security/ClientCQPostAuthorizationDUnitTest.java
>  e3e788688919d19615dfdfc9a528772810816e2b 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/security/ClientPostAuthorizationDUnitTest.java
>  963d2f02d061b6529e35aef14024e684007f4926 
>   
> geode-cq/src/test/java/com/gemstone/gemfire/security/MultiUserDurableCQAuthzDUnitTest.java
>  3a97b7c361c2082c44d76604460f630adaed12aa 
>   
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/WANConfigurationJUnitTest.java
>  84eb365ef3d698af6007757886edfa5cc5dabf2f 
> 
> Diff: https://reviews.apache.org/r/46457/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: Review Request 46457: GEODE-1268: Cleanup of multiple AvailablePort.getRandomAvailablePort invocations

[Udo Kohlmeyer]

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

(Updated April 21, 2016, 1:58 a.m.)


Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.


Changes
---

Amended some setup. Would have caused bugs.


Repository: geode


Description
---

GEODE-1268: Cleanup of multiple AvailablePort.getRandomAvailablePort 
invocations with correct AvailablePortHelper invocation


Diffs (updated)
-

  
geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 
3b5f23fcc3bcbf1f1d83c0d491636203e221cfc2 
  
geode-core/src/test/java/com/gemstone/gemfire/security/ClientAuthorizationDUnitTest.java
 961e8446628cd18bdbd847abf0b105cdfb020565 
  
geode-core/src/test/java/com/gemstone/gemfire/security/ClientAuthorizationTestCase.java
 4cc8155328e40fb0ccba4d93ec6f9886436fe5ff 
  
geode-core/src/test/java/com/gemstone/gemfire/security/DeltaClientPostAuthorizationDUnitTest.java
 61ff55adbd8fb1f73ba77eccb5d72641df88c867 
  
geode-core/src/test/java/com/gemstone/gemfire/security/P2PAuthenticationDUnitTest.java
 56f5186685e60ff8f0827f9b26332c59f65d89cd 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/ClientAuthzObjectModDUnitTest.java
 012ac332c181454ff68a5d1b60df773402cce756 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/ClientCQPostAuthorizationDUnitTest.java
 e3e788688919d19615dfdfc9a528772810816e2b 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/ClientPostAuthorizationDUnitTest.java
 963d2f02d061b6529e35aef14024e684007f4926 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/MultiUserDurableCQAuthzDUnitTest.java
 3a97b7c361c2082c44d76604460f630adaed12aa 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/WANConfigurationJUnitTest.java
 84eb365ef3d698af6007757886edfa5cc5dabf2f 

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


Testing
---

precheckin running


Thanks,

Udo Kohlmeyer

Review Request 46457: GEODE-1268: Cleanup of multiple AvailablePort.getRandomAvailablePort invocations

[Udo Kohlmeyer]

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

Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.


Repository: geode


Description
---

GEODE-1268: Cleanup of multiple AvailablePort.getRandomAvailablePort 
invocations with correct AvailablePortHelper invocation


Diffs
-

  
geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java 
3b5f23fcc3bcbf1f1d83c0d491636203e221cfc2 
  
geode-core/src/test/java/com/gemstone/gemfire/security/ClientAuthorizationDUnitTest.java
 961e8446628cd18bdbd847abf0b105cdfb020565 
  
geode-core/src/test/java/com/gemstone/gemfire/security/ClientAuthorizationTestCase.java
 4cc8155328e40fb0ccba4d93ec6f9886436fe5ff 
  
geode-core/src/test/java/com/gemstone/gemfire/security/DeltaClientPostAuthorizationDUnitTest.java
 61ff55adbd8fb1f73ba77eccb5d72641df88c867 
  
geode-core/src/test/java/com/gemstone/gemfire/security/P2PAuthenticationDUnitTest.java
 56f5186685e60ff8f0827f9b26332c59f65d89cd 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/ClientAuthzObjectModDUnitTest.java
 012ac332c181454ff68a5d1b60df773402cce756 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/ClientCQPostAuthorizationDUnitTest.java
 e3e788688919d19615dfdfc9a528772810816e2b 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/ClientPostAuthorizationDUnitTest.java
 963d2f02d061b6529e35aef14024e684007f4926 
  
geode-cq/src/test/java/com/gemstone/gemfire/security/MultiUserDurableCQAuthzDUnitTest.java
 3a97b7c361c2082c44d76604460f630adaed12aa 
  
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/misc/WANConfigurationJUnitTest.java
 84eb365ef3d698af6007757886edfa5cc5dabf2f 

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


Testing
---

precheckin running


Thanks,

Udo Kohlmeyer

Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address

[Udo Kohlmeyer]

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

(Updated April 13, 2016, 7:39 p.m.)


Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.


Changes
---

reverted gradle change


Repository: geode


Description
---

GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind 
address configuration for http-service-bind-address


Diffs (updated)
-

  
geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java
 a91df05 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 a8921d7 

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


Testing
---

battery tests rest.bt


Thanks,

Udo Kohlmeyer

Re: Review Request 46108: GEODE-1216 Fix the scalability of remove member

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On April 12, 2016, 6:51 p.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46108/
> ---
> 
> (Updated April 12, 2016, 6:51 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Uncomment the code that is for optimization when sending the suspect 
> requests. So when there are more than 4 members in the view, the suspect 
> request will only be sent to at most 5 members, which might include a random 
> non-preferred coordinator.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
>  5427d77 
> 
> Diff: https://reviews.apache.org/r/46108/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: Review Request 46056: GEODE-1178 Unexpected DistributedSystemDisconnectedException caused by RejectedExecutionException

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On April 12, 2016, 3:51 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46056/
> ---
> 
> (Updated April 12, 2016, 3:51 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-1178
> https://issues.apache.org/jira/browse/GEODE-1178
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This has been reported to JGroups.  While they're deciding what to do about 
> it I have coded a workaround in our StatRecorder class.  StatRecorder sits in 
> the JGroups stack just above the transport protocol that is throwing this 
> exception from its down() method.  StatRecorder will now catch the exception 
> and, after sleeping a short amount of time (10ms) it will retry as long as 
> the Manager is not shutting down.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/Services.java
>  4484c00adfbaa0d43e223515699201b4a2c6f102 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
>  510c5a8609d4d178da7b65667f6e244cbd1c01c9 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
>  2dfeeaaabcac9af4bd31f8359e1e62cdcdee41cc 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/StatRecorder.java
>  e29d71eb1c877153bfe99cef27566b76d6bea80f 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/StatRecorderJUnitTest.java
>  91d4a5738fcbf9a073e894afe4d0382e077a2a61 
> 
> Diff: https://reviews.apache.org/r/46056/diff/
> 
> 
> Testing
> ---
> 
> New unit test.  Precheckin is underway.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address

[Udo Kohlmeyer]

> On April 11, 2016, 4:44 p.m., Mark Bretl wrote:
> > I agree that we should not be including test jars in our product lib. 
> > 
> > Also, was precheckin or Travis CI run on this change? It looks like tests 
> > not in Geode source were run to verify this change.

The gradle change shall be reverted. Precheckin is in progress.


- Udo


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


On April 10, 2016, 9:59 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> ---
> 
> (Updated April 10, 2016, 9:59 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending 
> bind address configuration for http-service-bind-address
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java
>  a91df05a9931bddd860a79e6b8783fc991ecc2a4 
>   
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
>  a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5 
> 
> Diff: https://reviews.apache.org/r/45993/diff/
> 
> 
> Testing
> ---
> 
> battery tests rest.bt
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address

[Udo Kohlmeyer]

> On April 11, 2016, 3:43 p.m., Bruce Schuchardt wrote:
> > geode-assembly/build.gradle, line 116
> > <https://reviews.apache.org/r/45993/diff/1/?file=1338503#file1338503line116>
> >
> > Is this modifying the assembled product's lib directory?
> 
> Anthony Baker wrote:
> I agree, doesn't seem right to modify the contents of lib/ to run a unit 
> test.

gradle change reverted


- Udo


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


On April 10, 2016, 9:59 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45993/
> ---
> 
> (Updated April 10, 2016, 9:59 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending 
> bind address configuration for http-service-bind-address
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java
>  a91df05a9931bddd860a79e6b8783fc991ecc2a4 
>   
> geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
>  a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5 
> 
> Diff: https://reviews.apache.org/r/45993/diff/
> 
> 
> Testing
> ---
> 
> battery tests rest.bt
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Review Request 45994: GEODE-1200 : Deprecating InetAddressUtil

[Udo Kohlmeyer]

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

Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.


Repository: geode


Description
---

GEODE-1200 : Deprecating InetAddressUtil


Diffs
-

  
geode-core/src/main/java/com/gemstone/gemfire/admin/internal/InetAddressUtil.java
 05089a7b3e00d3b41ddfa350ac1b1b84fc568bd8 

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


Testing
---


Thanks,

Udo Kohlmeyer

Review Request 45993: GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind address configuration for http-service-bind-address

[Udo Kohlmeyer]

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

Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Jianxia Chen.


Repository: geode


Description
---

GEODE-1201: Adding compileRuntimeLibs to geode-assembly for tests Amending bind 
address configuration for http-service-bind-address


Diffs
-

  geode-assembly/build.gradle b7d05e2ce5fe34d3324e2b55212ff7c8ca02d9e0 
  
geode-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java
 a91df05a9931bddd860a79e6b8783fc991ecc2a4 
  
geode-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/swagger/config/RestApiPathProvider.java
 a8921d7d6f2a062a9aa5e27a8376a7e39c8503d5 

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


Testing
---

battery tests rest.bt


Thanks,

Udo Kohlmeyer

Re: Review Request 45844: GEODE-1187 If a server launched by Gfsh goes into auto-reconnect the server's PID file is deleted

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On April 6, 2016, 11:39 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45844/
> ---
> 
> (Updated April 6, 2016, 11:39 p.m.)
> 
> 
> Review request for geode, Hitesh Khamesra, Jianxia Chen, Jens Deppe, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ServerLauncher needed to be made aware of auto-reconnect.  I changed 
> getCache() to look for a new, reconnected cache and I changed isWaiting() to 
> see if the current cache is in a reconnecting state.
> 
> I also changed the shutdown logic to cancel reconnect attempts if the cache 
> is in that state when a stop is requested.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/ServerLauncher.java 
> 2a8c9f3c232a981fb8123df57c1f207d5de81f4f 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/ServerLauncherJUnitTest.java
>  d3a7050749004fd62975d10024f9e59752700e54 
> 
> Diff: https://reviews.apache.org/r/45844/diff/
> 
> 
> Testing
> ---
> 
> new tests for reconnect and for stopping the server.  Precheckin is underway. 
>  I ran manual network-down tests as well using iptable manipulation to 
> simulate a network partition with and without the change.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Http Service Bind Address - default behaviour

[Udo Kohlmeyer]

Hi there,

I would just like to confirm some "simple" functionality regarding the 
HTTP Service bind address.


If no bind address has been provided for the Http Service, should the 
default be to bind to the system bind-address or server-bind-address?


--Udo

Re: Review Request 45599: GEODE-1160 TransactionWriter is unexpectedly triggered if updating data entries with using PDX serializer

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On April 1, 2016, 7:22 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45599/
> ---
> 
> (Updated April 1, 2016, 7:22 p.m.)
> 
> 
> Review request for geode, Darrel Schneider, Hitesh Khamesra, Jianxia Chen, 
> and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This inhibits invocation of transaction writers and listners for operations 
> on internal cache Regions.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java
>  1a4d3777fc9efdba813f2215ef9f8deed55bbeef 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXEvent.java 
> e686ceae6ac010cc4e04972453cb90a578d7dade 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXRmtEvent.java 
> b378c8e5949c6fc310d1cb9fd9634e795c4751cb 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXState.java 
> 9e8dd18fd5065be1a9b94086732746bd8f770463 
>   
> geode-core/src/test/java/com/gemstone/gemfire/pdx/PdxSerializableDUnitTest.java
>  1e901bc2f12eba2e9ed9ea77e4cb57a97c0beb4c 
> 
> Diff: https://reviews.apache.org/r/45599/diff/
> 
> 
> Testing
> ---
> 
> precheckin including a new test for this problem
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 45503: GEODE-962: RestAPIsWithSSLDUnitTest.testMutualAuthentication CI failure

[Udo Kohlmeyer]

> On March 30, 2016, 10:29 p.m., Bruce Schuchardt wrote:
> > Is there any documentation that shows how to create the HTTP client?  If so 
> > does it need to be modified?

I don't think that we have any documentation on creating an HTTP client. We 
specify we expose via REST and it is up to the implementing customer to know 
how to create an HTTPs connection. This problem is limited to our 
implementation of the test client.


- Udo


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


On March 30, 2016, 8:22 p.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45503/
> ---
> 
> (Updated March 30, 2016, 8:22 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, Jianxia Chen, 
> and Jens Deppe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-962: RestAPIsWithSSLDUnitTest.testMutualAuthentication CI failure (on 
> specifically Docker)
> - Updated trusted.keystore and untrusted.keystore to have certificates that 
> will only expire in 2026. 
> - Changed the way that the HTTP client creates a SSLContext.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/rest/internal/web/controllers/RestAPIsWithSSLDUnitTest.java
>  1656eda898767b4272b07944ccbda975b1a97ab0 
>   geode-core/src/test/resources/ssl/trusted.keystore 
> ac40e04b8599850e9d21833cd9df61fb3db3c1fa 
>   geode-core/src/test/resources/ssl/untrusted.keystore 
> aa73eeb4c6a586cee40092194898c9e75a052d2b 
> 
> Diff: https://reviews.apache.org/r/45503/diff/
> 
> 
> Testing
> ---
> 
> distributedTests passing on local and on docker.
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>

Re: unit tests

[Udo Kohlmeyer]

Hi there Dor,

Haven't heard back from you. Assuming that your issue has been resolved.

--Udo

On 29/03/2016 5:15 pm, Dor Ben Dov wrote:

Hi,

When I compile without the skip tests flags it fails, does it happens to you 
also ?


Dor


This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,
you may review at http://www.amdocs.com/email_disclaimer.asp

Re: Review Request 45058: GEODE-1107 CI failure: RestAPIsWithSSLDUnitTest.testSimpleSSL

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On March 24, 2016, 10 p.m., Jianxia Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45058/
> ---
> 
> (Updated March 24, 2016, 10 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, Jens Deppe, and 
> Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Set jmx-manager-port to 0 so that it does not allow remote client 
> connections. Hence avoid starting the agent. Then there is no worry about the 
> port already in use for the agent. HTTP service is still started though.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/com/gemstone/gemfire/rest/internal/web/controllers/RestAPIsWithSSLDUnitTest.java
>  852591f 
>   
> geode-core/src/test/java/com/gemstone/gemfire/test/dunit/standalone/DUnitLauncher.java
>  99548b3 
> 
> Diff: https://reviews.apache.org/r/45058/diff/
> 
> 
> Testing
> ---
> 
> RestAPIsWithSSLDUnitTest
> 
> 
> Thanks,
> 
> Jianxia Chen
> 
>

Re: Review Request 45449: GEODE-923 In test changed "System.currentTimeMillis" to "System.nanoTime()" as Connection mgr uses nantime

[Udo Kohlmeyer]

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




geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/pooling/ConnectionManagerJUnitTest.java
 (line 236)
<https://reviews.apache.org/r/45449/#comment188897>

Is this nanos or millis? We would need some consistency here. Otherwise it 
gets really confusing really quickly



geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/pooling/ConnectionManagerJUnitTest.java
 (line 272)
<https://reviews.apache.org/r/45449/#comment188899>

Maybe we could use "startInNanos". Less confusing as just general "start"



geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/pooling/ConnectionManagerJUnitTest.java
 (line 286)
<https://reviews.apache.org/r/45449/#comment188900>

Maybe we could print out the time unit. "Elapsed time(ms): " would be more 
helpful


- Udo Kohlmeyer


On March 29, 2016, 10:08 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45449/
> ---
> 
> (Updated March 29, 2016, 10:08 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jianxia Chen, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Aligned test code with ConnectionManager..
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/cache/client/internal/pooling/ConnectionManagerJUnitTest.java
>  bb407a4 
> 
> Diff: https://reviews.apache.org/r/45449/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>