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

2017-06-27 Thread Dan Smith
Looks like this build hung in PersistentPartitionedRegionDUnitTest. Here
are the last lines in
https://builds.apache.org/job/Geode-nightly/879/artifact/geode-core/build/distributedTest/distributedTest-progress.txt
:

2017-06-27 09:13:00.547 + Completed test
org.apache.geode.internal.cache.partitioned.PersistentPartitionedRegionDUnitTest
testReplicateAfterPersistent with result: SUCCESS
2017-06-27 09:13:00.548 + Starting test
org.apache.geode.internal.cache.partitioned.PersistentPartitionedRegionDUnitTest
testDiskConflictWithCoLocation


On Tue, Jun 27, 2017 at 8:46 PM, Apache Jenkins Server <
jenk...@builds.apache.org> wrote:

> See  redirect?page=changes>
>
> Changes:
>
> [kmiller] GEODE-2819 Document http status code 414 for PUT
>
> [hkhamesra] GEODE-3075: Initial refactor adding NewProtocolServerConnection
>
> [hkhamesra] GEODE-3075: Add an integration test for the
> ServerConnectionFactory
>
> [hkhamesra] GEODE-2995: Handle stream of ProtoBuf encoded messages
>
> [hkhamesra] GEODE-3075: changes in response to feedback; refactor some.
>
> [hkhamesra] GEODE-2995: Implementing review feedback
>
> [hkhamesra] GEODE-2995: more review feedback and spotlessApply
>
> [hkhamesra] GEODE-2995: incorporating even more review feedback
>
> [hkhamesra] GEODE-3075: fix an integration test.
>
> [hkhamesra] merge GEODE-2995, GEODE-3775 into develop with integration
> test.
>
> [hkhamesra] Add ServiceLoadingFailureException to excludedClasses.txt so
> tests pass.
>
> [hkhamesra] Move new exception to sanctioned serializables.
>
> [jiliao] GEODE-1958: Rolling back changes to decrypt method
>
> --
> [...truncated 66.03 KB...]
> :geode-assembly:defaultDistributionConfig
> :geode-assembly:depsJar
> :geode-benchmarks:compileJava UP-TO-DATE
> :geode-benchmarks:processResources UP-TO-DATE
> :geode-benchmarks:classes UP-TO-DATE
> :geode-cq:compileJavaNote: Some input files use or override a deprecated
> API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-cq:processResources
> :geode-cq:classes
> :geode-lucene:compileJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-lucene:processResources
> :geode-lucene:classes
> :geode-old-client-support:compileJavaNote: Some input files use or
> override a deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
>
> :geode-old-client-support:processResources
> :geode-old-client-support:classes
> :geode-protobuf:extractIncludeProto
> Download https://repo1.maven.org/maven2/com/google/protobuf/
> protobuf-java/3.3.1/protobuf-java-3.3.1.pom
> Download https://repo1.maven.org/maven2/com/google/protobuf/
> protobuf-parent/3.3.1/protobuf-parent-3.3.1.pom
> Download https://repo1.maven.org/maven2/com/google/protobuf/
> protobuf-java/3.3.1/protobuf-java-3.3.1.jar
> :geode-protobuf:extractProto
> :geode-protobuf:generateProto
> Download https://repo1.maven.org/maven2/com/google/protobuf/
> protoc/3.0.0/protoc-3.0.0.pom
> Download https://repo1.maven.org/maven2/com/google/protobuf/
> protoc/3.0.0/protoc-3.0.0-linux-x86_64.exe
> :geode-protobuf:compileJavaNote: Some input files use unchecked or unsafe
> operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-protobuf:processResources
> :geode-protobuf:classes
> :geode-pulse:compileJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-pulse:copyGemFireVersionFile
> :geode-pulse:processResources
> :geode-pulse:classes
> :geode-rebalancer:compileJava
> :geode-rebalancer:processResources UP-TO-DATE
> :geode-rebalancer:classes
> :geode-wan:compileJava
> :geode-wan:processResources
> :geode-wan:classes
> :geode-web:compileJava UP-TO-DATE
> :geode-web:processResources UP-TO-DATE
> :geode-web:classes UP-TO-DATE
> :geode-web-api:compileJavaNote: Some input files use unchecked or unsafe
> operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-web-api:processResources
> :geode-web-api:classes
> :geode-assembly:docs
> :geode-assembly:gfshDepsJar
> :geode-common:javadocJar
> :geode-common:sourcesJar
> :geode-common:signArchives SKIPPED
> :geode-core:javadocJar
> :geode-core:raJar
> :geode-core:jcaJar
> :geode-core:sourcesJar
> :geode-core:signArchives SKIPPED
> :geode-core:webJar
> :geode-cq:jar
> :geode-cq:javadoc
> :geode-cq:javadocJar
> :geode-cq:sourcesJar
> :geode-cq:signArchives SKIPPED
> :geode-json:javadocJar
> :geode-json:sourcesJar
> :geode-json:signArchives SKIPPED
> :geode-lucene:jar
> 

Re: Review Request 60442: GEODE-3130: Refactoring AcceptorImpl communication mode switch

2017-06-27 Thread Udo Kohlmeyer

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


Fix it, then Ship it!




Fix `AcceptorImpl.this` reference and then ship


geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
Line 1457 (original), 1457 (patched)


What does `AcceptorImpl.this` return? `this` should be enough?


- Udo Kohlmeyer


On June 26, 2017, 6:31 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60442/
> ---
> 
> (Updated June 26, 2017, 6:31 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3130
> https://issues.apache.org/jira/browse/GEODE-3130
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This review addresses Udo's last feedback for GEODE-2995.  This change also 
> cleaned up the imports on the AcceptorImplJUnitTest.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  50f7006fa 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplJUnitTest.java
>  7aa11b7ca 
> 
> 
> Diff: https://reviews.apache.org/r/60442/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin in progress.
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>



Re: Review Request 60451: GEODE-2996: adding Put handler

2017-06-27 Thread Udo Kohlmeyer

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




geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
Lines 68 (patched)


This has nothing to do with the ProtobufOpsProcessor. Maybe the builder can 
be passed in.



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Line 42 (original), 39 (patched)


Maybe we do this before the key is decoded. After line 34 if possible.



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Lines 42 (patched)


Maybe this should be replaced with an errorMessage, stating this region 
does not exist.



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Lines 54 (patched)


`encoding` should start with capitol



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Lines 56 (patched)


`codec` should start with capitol



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Lines 62 (patched)


Maybe this should return an empty list.



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Lines 66 (patched)


This provides no information about the failure... An `ErrorMessage` should 
be a more suitable replacement



geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
Line 59 (original), 78 (patched)


Can I have a `success=false` and `keyExists=true`? What does `keyExists` 
provide me in functionality above and beyond `success` or an `ErrorMessage`?



geode-protobuf/src/main/proto/region_API.proto
Line 35 (original), 35 (patched)


not sure `success` or `keyExists` provides any value here `result` 
should either have a value or not. 
What does `keyExists` denote? 
What counts as a `failure`?



geode-protobuf/src/test/java/org/apache/geode/protocol/MessageUtil.java
Line 20 (original), 31 (patched)


Given this class is Protobuf specific, we either put it in the `protobuf` 
specific package or call it `ProtobufMessageUtil`



geode-protobuf/src/test/java/org/apache/geode/protocol/MessageUtil.java
Lines 46 (patched)


This is too specific... What about all the other encoding types? 
Int,long,short,byte?


- Udo Kohlmeyer


On June 27, 2017, 1:20 a.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60451/
> ---
> 
> (Updated June 27, 2017, 1:20 a.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2996
> https://issues.apache.org/jira/browse/GEODE-2996
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This is a continuation of the review Alex submitted this morning with the 
> following changes:
> 
> Addresses review feedback for GEODE-2996, mainly refactoring 
> getOpertionHandler to handle failures like the putOperationHandler
> Adding put operations to the RoundTripCacheConnectionJUnitTest, which is the 
> integration test for the protobuf module
> Removing service loading for protobuf operations and instead have the 
> ProtobufStreamProcessor populate its OperationHandlerRegistry
> Remove exception throwing from OperationHandler.process calls and remove 
> TypeEncodingException
> Fixing ProtobufOpsProcessor to handle responses for types other than get
> 
> 
> Diffs
> -
> 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java
>  7683e3bf3 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java
>  8e3a33149 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
>  d426149e4 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
>  d7b5d4bd2 
>   
> 

Re: Review Request 60486: GEODE-3105: Adding handler for GetRegions

2017-06-27 Thread Udo Kohlmeyer

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




geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
Line 50 (original), 50 (patched)


Feels like this is going to end up being a TranslationUtilService. Can you 
discusss internally... `wrapResponseForOperationTypeID` is a candidate for 
movement as well...



geode-protobuf/src/main/proto/region_API.proto
Line 107 (original), 107 (patched)


What does this represent? `Failure` should be defined in the  `Success` can 
be defined as either an empty or populated regions list.

In the `ResponseHeader` does contain an `errorCode`. What we seem to be 
missing is a real `ErrorMessage`, that would contain the `exceptionCode` and 
`errorMessage`.


- Udo Kohlmeyer


On June 27, 2017, 11:31 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60486/
> ---
> 
> (Updated June 27, 2017, 11:31 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3105
> https://issues.apache.org/jira/browse/GEODE-3105
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Added a handler which will catch incoming getRegion requests and will call 
> into the cache's rootRegion and return the names of the regions it finds.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
>  8edd83c0f 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
>  29d33170c 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
>  21dbef5bc 
>   
> geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRegionsRequestOperationHandler.java
>  PRE-CREATION 
>   geode-protobuf/src/main/proto/region_API.proto adeb011c6 
>   geode-protobuf/src/test/java/org/apache/geode/protocol/MessageUtil.java 
> 73d0803b6 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java
>  1fbe82159 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/GetRegionRequestOperationHandlerJUnitTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60486/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit test verifying hanlder behavior.
> Added integration test verifying module correctness for getRegion.
> Running precheckin.
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>



Build failed in Jenkins: Geode-nightly-flaky #48

2017-06-27 Thread Apache Jenkins Server
See 


Changes:

[jiliao] GEODE-2919: fix lucene security tests

[jiliao] GEODE-2919: revert: fix lucene security tests

[bschuchardt] GEODE-3072: Events do not get removed from the client queue for 
1.0

[hkhamesra] GEODE-3130: Refactoring AcceptorImpl

[hkhamesra] GEODE-2996: incorporating review feedback and adding integration 
test

--
Started by upstream project "Geode-nightly" build number 879
originally caused by:
 Started by timer
[EnvInject] - Loading node environment variables.
Building remotely on ubuntu-eu2 (ubuntu trusty) in workspace 

 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url 
 > https://git-wip-us.apache.org/repos/asf/geode.git # timeout=10
Fetching upstream changes from https://git-wip-us.apache.org/repos/asf/geode.git
 > git --version # timeout=10
 > git fetch --tags --progress 
 > https://git-wip-us.apache.org/repos/asf/geode.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/develop^{commit} # timeout=10
Checking out Revision 565e61f74409d59cc96c9fbf1ad75d331b728301 
(refs/remotes/origin/develop)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 565e61f74409d59cc96c9fbf1ad75d331b728301
 > git branch -a -v --no-abbrev # timeout=10
 > git branch -D develop # timeout=10
 > git checkout -b develop 565e61f74409d59cc96c9fbf1ad75d331b728301
 > git rev-list 137ced6bea482209efe5db7d87b58edefd9b7222 # timeout=10
Cleaning workspace
 > git rev-parse --verify HEAD # timeout=10
Resetting working tree
 > git reset --hard # timeout=10
 > git clean -fdx # timeout=10
[Gradle] - Launching build.
[Geode-nightly-flaky] $ 
 --no-daemon 
flakyTest
Downloading https://services.gradle.org/distributions/gradle-2.14.1-bin.zip
...
Unzipping 

 to 

Set executable permissions for: 

To honour the JVM settings for this build a new JVM will be forked. Please 
consider using the daemon: 
https://docs.gradle.org/2.14.1/userguide/gradle_daemon.html.
Generating JAR file 'gradle-api-2.14.1.jar'

Build failed in Jenkins: Geode-nightly #879

2017-06-27 Thread Apache Jenkins Server
See 


Changes:

[kmiller] GEODE-2819 Document http status code 414 for PUT

[hkhamesra] GEODE-3075: Initial refactor adding NewProtocolServerConnection

[hkhamesra] GEODE-3075: Add an integration test for the ServerConnectionFactory

[hkhamesra] GEODE-2995: Handle stream of ProtoBuf encoded messages

[hkhamesra] GEODE-3075: changes in response to feedback; refactor some.

[hkhamesra] GEODE-2995: Implementing review feedback

[hkhamesra] GEODE-2995: more review feedback and spotlessApply

[hkhamesra] GEODE-2995: incorporating even more review feedback

[hkhamesra] GEODE-3075: fix an integration test.

[hkhamesra] merge GEODE-2995, GEODE-3775 into develop with integration test.

[hkhamesra] Add ServiceLoadingFailureException to excludedClasses.txt so tests 
pass.

[hkhamesra] Move new exception to sanctioned serializables.

[jiliao] GEODE-1958: Rolling back changes to decrypt method

--
[...truncated 66.03 KB...]
:geode-assembly:defaultDistributionConfig
:geode-assembly:depsJar
:geode-benchmarks:compileJava UP-TO-DATE
:geode-benchmarks:processResources UP-TO-DATE
:geode-benchmarks:classes UP-TO-DATE
:geode-cq:compileJavaNote: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-cq:processResources
:geode-cq:classes
:geode-lucene:compileJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-lucene:processResources
:geode-lucene:classes
:geode-old-client-support:compileJavaNote: Some input files use or override a 
deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:geode-old-client-support:processResources
:geode-old-client-support:classes
:geode-protobuf:extractIncludeProto
Download 
https://repo1.maven.org/maven2/com/google/protobuf/protobuf-java/3.3.1/protobuf-java-3.3.1.pom
Download 
https://repo1.maven.org/maven2/com/google/protobuf/protobuf-parent/3.3.1/protobuf-parent-3.3.1.pom
Download 
https://repo1.maven.org/maven2/com/google/protobuf/protobuf-java/3.3.1/protobuf-java-3.3.1.jar
:geode-protobuf:extractProto
:geode-protobuf:generateProto
Download 
https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.0.0/protoc-3.0.0.pom
Download 
https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.0.0/protoc-3.0.0-linux-x86_64.exe
:geode-protobuf:compileJavaNote: Some input files use unchecked or unsafe 
operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-protobuf:processResources
:geode-protobuf:classes
:geode-pulse:compileJavaNote: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-pulse:copyGemFireVersionFile
:geode-pulse:processResources
:geode-pulse:classes
:geode-rebalancer:compileJava
:geode-rebalancer:processResources UP-TO-DATE
:geode-rebalancer:classes
:geode-wan:compileJava
:geode-wan:processResources
:geode-wan:classes
:geode-web:compileJava UP-TO-DATE
:geode-web:processResources UP-TO-DATE
:geode-web:classes UP-TO-DATE
:geode-web-api:compileJavaNote: Some input files use unchecked or unsafe 
operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-web-api:processResources
:geode-web-api:classes
:geode-assembly:docs
:geode-assembly:gfshDepsJar
:geode-common:javadocJar
:geode-common:sourcesJar
:geode-common:signArchives SKIPPED
:geode-core:javadocJar
:geode-core:raJar
:geode-core:jcaJar
:geode-core:sourcesJar
:geode-core:signArchives SKIPPED
:geode-core:webJar
:geode-cq:jar
:geode-cq:javadoc
:geode-cq:javadocJar
:geode-cq:sourcesJar
:geode-cq:signArchives SKIPPED
:geode-json:javadocJar
:geode-json:sourcesJar
:geode-json:signArchives SKIPPED
:geode-lucene:jar
:geode-lucene:javadoc
:geode-lucene:javadocJar
:geode-lucene:sourcesJar
:geode-lucene:signArchives SKIPPED
:geode-old-client-support:jar
:geode-old-client-support:javadoc
:geode-old-client-support:javadocJar
:geode-old-client-support:sourcesJar
:geode-old-client-support:signArchives SKIPPED
:geode-pulse:javadoc
:geode-pulse:javadocJar
:geode-pulse:sourcesJar
:geode-pulse:war
:geode-pulse:signArchives SKIPPED
:geode-rebalancer:jar
:geode-rebalancer:javadoc
:geode-rebalancer:javadocJar
:geode-rebalancer:sourcesJar
:geode-rebalancer:signArchives SKIPPED
:geode-wan:jar
:geode-wan:javadoc
:geode-wan:javadocJar
:geode-wan:sourcesJar
:geode-wan:signArchives SKIPPED
:geode-web:javadoc UP-TO-DATE
:geode-web:javadocJar
:geode-web:sourcesJar
:geode-web:war
:geode-web:signArchives SKIPPED
:geode-web-api:javadoc
:geode-web-api:javadocJar
:geode-web-api:sourcesJar

[GitHub] geode pull request #608: GEODE-3140: Removed DiskDirRule and replaced use wi...

2017-06-27 Thread jinmeiliao
Github user jinmeiliao commented on a diff in the pull request:

https://github.com/apache/geode/pull/608#discussion_r124434616
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesPersistenceIntegrationTest.java
 ---
@@ -48,13 +49,13 @@
 public class LuceneQueriesPersistenceIntegrationTest extends 
LuceneIntegrationTest {
 
   @Rule
-  public DiskDirRule diskDirRule = new DiskDirRule();
+  public TemporaryFolder tempFolderRule = new TemporaryFolder(new 
File("."));
--- End diff --

Is there any reason we want to pass in a parentFolder of current dir? Can 
we just use the temp folder that we don't care about where it is?


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


Re: Review Request 60375: GEODE-3117: fix Gateway authentication with legacy security

2017-06-27 Thread Jinmei Liao


> On June 26, 2017, 11:58 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
> > Line 82 (original), 82 (patched)
> > 
> >
> > since we already pass in the entire sys, do we need to pass in 
> > sys.getSecurityService() as well? Can we change the signaure of the 
> > constructor to take an InternalDistributedSystem, and directly uses the 
> > security service inside the passed sys?
> 
> Kirk Lund wrote:
> I'm trying to separate our major services instead of folding them in 
> together. For example, I don't want to go to Cache to get DistributedSystem 
> (but you can) and I don't want our code to go to DistributedSystem to get 
> SecurityService. The link between DistributedSystem and SecurityService is 
> tenuous at best -- DistributedSystem USES SecurityService but it doesn't 
> logically own it except for the fact that DS is the 1st object in Geode that 
> needs to use it. So from the point-of-view of an object like HandShake, I'm 
> trying to give it individual services. This also has the side effect of 
> making classes such as HandShake easier to unit test when mocking the 
> collaborators. Another reason I'm doing this is because it makes ALL of the 
> collorators visible at the constructor level which is a good practice to 
> follow.
> 
> If we want to start using Dependency Injection (which I want to), then we 
> need to do even more of this -- passing in every collaborator individually 
> rather than passing in a god object to invoke getters against on it to get 
> the other dependencies.
> 
> Or to summarize even more, I'm trying to steer away from bad code to good 
> code (good object-oriented design) and god objects are one of the worst 
> anti-patterns.

If you envisioned that security service will be rippsed out of ds in the 
future, and wants to gear towards that way, then I can see your point. But from 
the current state of things I would not call it bad o-o design though. If I see 
any method call that's in the shape of callXYZ(o, o.getX(), o.getY(), 
o.getZ()), then I would question why bother pssing in the last three 
parameters. To me, that's anti-pattern.


- Jinmei


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


On June 26, 2017, 6:02 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60375/
> ---
> 
> (Updated June 26, 2017, 6:02 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3117
> https://issues.apache.org/jira/browse/GEODE-3117
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3117: fix Gateway authentication with legacy security
> 
> * add GatewayLegacyAuthenticationRegressionTest to reproduce bug
> * fix authentication of Gateway sender/receiver with 
> SECURITY_CLIENT_AUTHENTICATOR
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  a419d575010d39d4dab6c5c8f9748928c1764344 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  32735b9ab17fe9467cea46096bd177902145e4bd 
>   
> geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/GatewayLegacyAuthenticationRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60375/diff/1/
> 
> 
> Testing
> ---
> 
> * new test reproduces GEODE-3117: GatewayLegacyAuthenticationRegressionTest
> * precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



[GitHub] geode issue #608: GEODE-3140: Removed DiskDirRule and replaced use with Temp...

2017-06-27 Thread DivineEnder
Github user DivineEnder commented on the issue:

https://github.com/apache/geode/pull/608
  
@ladyVader @nabarunnag @boglesby @jhuynh1 @upthewaterspout @gesterzhou 


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


[GitHub] geode pull request #608: GEODE-3140: Removed DiskDirRule and replaced use wi...

2017-06-27 Thread DivineEnder
GitHub user DivineEnder opened a pull request:

https://github.com/apache/geode/pull/608

GEODE-3140: Removed DiskDirRule and replaced use with TemporaryFolder…

… rule

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [ ] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to dev@geode.apache.org.


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

$ git pull https://github.com/DivineEnder/geode feature/GEODE-3140

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

https://github.com/apache/geode/pull/608.patch

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

This closes #608


commit 8e43d6139c13ad5de6355f34637d92af8e9d699e
Author: David Anuta 
Date:   2017-06-28T00:00:00Z

GEODE-3140: Removed DiskDirRule and replaced use with TemporaryFolder rule




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


Failed: jinmeiliao/geode#2 (extractCommandRefactor - 5d676d1)

2017-06-27 Thread Travis CI
Build Update for jinmeiliao/geode
-

Build: #2
Status: Failed

Duration: 27 minutes and 3 seconds
Commit: 5d676d1 (extractCommandRefactor)
Author: Jinmei Liao
Message: wip

View the changeset: 
https://github.com/jinmeiliao/geode/compare/e5204ce12245^...5d676d1f003b

View the full build log and details: 
https://travis-ci.org/jinmeiliao/geode/builds/247723706?utm_source=email_medium=notification

--

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



Re: New Apache commons-validato.jar

2017-06-27 Thread Kirk Lund
+1 for adding Commons-Validator

On Tue, Jun 27, 2017 at 9:58 AM, Hitesh Khamesra <
hitesh...@yahoo.com.invalid> wrote:

> We are planning to include Apache commons-validator_1.6.jar in Geode. As
> part of GEODE-2804, we need to validate whether host string is configured
> as IP or not. Please let us know if there is any issue with it.
>
> Thanks.
> Hitesh
>
>


Re: New Apache commons-validato.jar

2017-06-27 Thread Anthony Baker
Thanks for the heads up.  It’s Apache licensed so no problems with that.

To add it to the runtime classpath you’ll need to modify 
geode-assembly/build.gradle and fix the expected_jars.txt file.

Anthony

> On Jun 27, 2017, at 9:58 AM, Hitesh Khamesra  
> wrote:
> 
> We are planning to include Apache commons-validator_1.6.jar in Geode. As part 
> of GEODE-2804, we need to validate whether host string is configured as IP or 
> not. Please let us know if there is any issue with it.
> 
> Thanks.
> Hitesh
> 



Re: Review Request 60375: GEODE-3117: fix Gateway authentication with legacy security

2017-06-27 Thread Kirk Lund


> On June 26, 2017, 11:58 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
> > Line 82 (original), 82 (patched)
> > 
> >
> > since we already pass in the entire sys, do we need to pass in 
> > sys.getSecurityService() as well? Can we change the signaure of the 
> > constructor to take an InternalDistributedSystem, and directly uses the 
> > security service inside the passed sys?

I'm trying to separate our major services instead of folding them in together. 
For example, I don't want to go to Cache to get DistributedSystem (but you can) 
and I don't want our code to go to DistributedSystem to get SecurityService. 
The link between DistributedSystem and SecurityService is tenuous at best -- 
DistributedSystem USES SecurityService but it doesn't logically own it except 
for the fact that DS is the 1st object in Geode that needs to use it. So from 
the point-of-view of an object like HandShake, I'm trying to give it individual 
services. This also has the side effect of making classes such as HandShake 
easier to unit test when mocking the collaborators. Another reason I'm doing 
this is because it makes ALL of the collorators visible at the constructor 
level which is a good practice to follow.

If we want to start using Dependency Injection (which I want to), then we need 
to do even more of this -- passing in every collaborator individually rather 
than passing in a god object to invoke getters against on it to get the other 
dependencies.

Or to summarize even more, I'm trying to steer away from bad code to good code 
(good object-oriented design) and god objects are one of the worst 
anti-patterns.


- Kirk


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


On June 26, 2017, 6:02 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60375/
> ---
> 
> (Updated June 26, 2017, 6:02 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3117
> https://issues.apache.org/jira/browse/GEODE-3117
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3117: fix Gateway authentication with legacy security
> 
> * add GatewayLegacyAuthenticationRegressionTest to reproduce bug
> * fix authentication of Gateway sender/receiver with 
> SECURITY_CLIENT_AUTHENTICATOR
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  a419d575010d39d4dab6c5c8f9748928c1764344 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  32735b9ab17fe9467cea46096bd177902145e4bd 
>   
> geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/GatewayLegacyAuthenticationRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60375/diff/1/
> 
> 
> Testing
> ---
> 
> * new test reproduces GEODE-3117: GatewayLegacyAuthenticationRegressionTest
> * precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



[GitHub] geode pull request #607: GEODE-3105: adding GetRegion handler for protobuf p...

2017-06-27 Thread WireBaron
GitHub user WireBaron opened a pull request:

https://github.com/apache/geode/pull/607

GEODE-3105: adding GetRegion handler for protobuf protocol

Added a handler which will catch incoming getRegion requests and will call 
into the cache's rootRegion and return the names of the region it finds.
Added unit test verifying hanlder behavior.
Added integration test verifying module correctness for getRegion.

Signed-off-by: Hitesh Khamesra 

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [x] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to dev@geode.apache.org.


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

$ git pull https://github.com/WireBaron/geode feature/GEODE-3105

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

https://github.com/apache/geode/pull/607.patch

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

This closes #607


commit f2d55db4e3eabfb51d13284c7cb0b0bacf5b2c05
Author: Brian Rowe 
Date:   2017-06-27T23:15:53Z

GEODE-3105: adding GetRegion handler for protobuf protocol

Added a handler which will catch incoming getRegion requests and will call 
into the cache's rootRegion and return the names of the region it finds.
Added unit test verifying hanlder behavior.
Added integration test verifying module correctness for getRegion.

Signed-off-by: Brian Rowe 
Signed-off-by: Hitesh Khamesra 




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


Review Request 60486: GEODE-3105: Adding handler for GetRegions

2017-06-27 Thread Brian Rowe

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

Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.


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


Repository: geode


Description
---

Added a handler which will catch incoming getRegion requests and will call into 
the cache's rootRegion and return the names of the regions it finds.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
 8edd83c0f 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
 29d33170c 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java
 21dbef5bc 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRegionsRequestOperationHandler.java
 PRE-CREATION 
  geode-protobuf/src/main/proto/region_API.proto adeb011c6 
  geode-protobuf/src/test/java/org/apache/geode/protocol/MessageUtil.java 
73d0803b6 
  
geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java
 1fbe82159 
  
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/GetRegionRequestOperationHandlerJUnitTest.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/60486/diff/1/


Testing
---

Added unit test verifying hanlder behavior.
Added integration test verifying module correctness for getRegion.
Running precheckin.


Thanks,

Brian Rowe



Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Bruce Schuchardt

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




geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
Line 107 (original), 110 (patched)


Your new parameter isn't listed in the useless javadoc of this method.  I 
think you should delete the javadoc.



geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
Lines 40 (patched)


typo:  InetSocketAddress, not InetScoketAddress



geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
Lines 72 (patched)


There must be a better name for this method.  I thought it was a typo.  
Maybe getSocketInetAddressNoLookup or something else describing its function.



geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
Lines 346 (patched)


You can use a lambda for Callable

() -> {
  return source.getOnlineLocators().isEmpty();
  }


- Bruce Schuchardt


On June 27, 2017, 1:04 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60312/
> ---
> 
> (Updated June 27, 2017, 1:04 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
> O'Sullivan.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Cache InetAddress if configure host as ip string.
> 
> 1. We keep configure host string in HostAddress class
> 2. We reuse InetsocketAddress if it is ipString.
> 3. Client has logic to retry thus we keep InetsocketAddress even if 
>it is not ip string.
> 
> GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.
> 
> GEODE-2940 Now we don't validate configure host string on start. As
> there is possibility that host may not available.
> 
> 1. Earlier wan config were failing because of that. Now we just keep
> those configure host string. And try this later.
> 
> Added unit tests for it.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle 39bb542 
>   geode-assembly/src/test/resources/expected_jars.txt 6260167 
>   geode-core/build.gradle 7f34b4a 
>   
> geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
>  c1bfc93 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  53d401a 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
> 3ded54a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
>  01c6157 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  a4b3a50 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  da295ab 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  aff1938 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  bc3d708 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  6b54170 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
>  5ab1bed 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
>  1dc2fd1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  92cfd96 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
> d4fdbd0 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  0efba01 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  88832ba 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
>  789d326 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  3cc3cfc 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
>  9f6c5fb 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
>  9d63050 
>   
> 

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #599 was SUCCESSFUL (with 1908 tests)

2017-06-27 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #599 was successful.
---
Scheduled
1910 tests in total.

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





--
This message is automatically generated by Atlassian Bamboo

Review Request 60483: GEODE-3139 remove geode-core/src/main artifacts from classpath of backward-compatibility tests

2017-06-27 Thread Bruce Schuchardt

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

Review request for geode, Kirk Lund and Dan Smith.


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


Repository: geode


Description
---

This removes the geode-core product artifacts from old-version JVM classpaths 
for backward-compatibility testing.  I had to change the distributed test 
framework to not refer to InternalClientCache but surprisingly that caused no 
compilation failures.


Diffs
-

  
geode-core/src/test/java/org/apache/geode/test/dunit/cache/internal/JUnit3CacheTestCase.java
 917361fbc3f5c78b115b29a9ddab21af486a6720 
  
geode-core/src/test/java/org/apache/geode/test/dunit/cache/internal/JUnit4CacheTestCase.java
 862974942bcbf41d563215603edc3ca27f5b277c 
  
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java
 21b79e857feb5aab0abba91060c322b43296d91a 
  gradle/test.gradle 3015ec0973a0c9049f124be343ba505b776030a9 


Diff: https://reviews.apache.org/r/60483/diff/1/


Testing
---


Thanks,

Bruce Schuchardt



[GitHub] geode pull request #605: GEODE-2996: incorporating review feedback and addin...

2017-06-27 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/605


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


[GitHub] geode pull request #602: GEODE-3130: Refactoring AcceptorImpl

2017-06-27 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/602


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


[GitHub] geode pull request #606: GEODE-2601: Fixing startup configurations logging t...

2017-06-27 Thread YehEmily
GitHub user YehEmily opened a pull request:

https://github.com/apache/geode/pull/606

GEODE-2601: Fixing startup configurations logging twice

[View the JIRA ticket 
here.](https://issues.apache.org/jira/browse/GEODE-2601)

The original fix (pull request #582) fixed the security banner logging 
twice, but not the startup configurations. This PR finishes the fix.

- [ ] JIRA ticket referenced in the commit message

- [ ] PR rebased against `develop`

- [ ] Initial contribution is a single, squashed commit

- [ ] `gradlew build` runs cleanly

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

$ git pull https://github.com/YehEmily/geode GEODE-2601-2

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

https://github.com/apache/geode/pull/606.patch

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

This closes #606


commit aa22d3c1c9a51bda1a529121453b34045a5802c7
Author: YehEmily 
Date:   2017-06-27T19:56:40Z

GEODE-2601: Fixing banner and startup configurations logging twice




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


Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Hitesh Khamesra

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

(Updated June 27, 2017, 8:04 p.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
O'Sullivan.


Changes
---

Updated patch with latest develop


Repository: geode


Description
---

GEODE-2804 Cache InetAddress if configure host as ip string.

1. We keep configure host string in HostAddress class
2. We reuse InetsocketAddress if it is ipString.
3. Client has logic to retry thus we keep InetsocketAddress even if 
   it is not ip string.

GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.

GEODE-2940 Now we don't validate configure host string on start. As
there is possibility that host may not available.

1. Earlier wan config were failing because of that. Now we just keep
those configure host string. And try this later.

Added unit tests for it.


Diffs (updated)
-

  geode-assembly/build.gradle 39bb542 
  geode-assembly/src/test/resources/expected_jars.txt 6260167 
  geode-core/build.gradle 7f34b4a 
  
geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
 c1bfc93 
  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 53d401a 
  geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
3ded54a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 01c6157 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 a4b3a50 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
 da295ab 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 aff1938 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 bc3d708 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 6b54170 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
 5ab1bed 
  
geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
 1dc2fd1 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 92cfd96 
  geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
d4fdbd0 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 0efba01 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 88832ba 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
 789d326 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 3cc3cfc 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 9f6c5fb 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
 9d63050 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
 a31fa8d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
6a6e0c1 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
 f5a8fcf 
  
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
 d6d5d7c 
  
geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
 dbc2cc6 
  
geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
 6d75064 
  gradle/dependency-versions.properties 6a730a4 


Diff: https://reviews.apache.org/r/60312/diff/2/

Changes: https://reviews.apache.org/r/60312/diff/1-2/


Testing
---


Thanks,

Hitesh Khamesra



[GitHub] geode pull request #601: GEODE-3117: fix Gateway authentication with legacy ...

2017-06-27 Thread pdxrunner
Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/601#discussion_r124376469
  
--- Diff: 
geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/GatewayLegacyAuthenticationRegressionTest.java
 ---
@@ -0,0 +1,429 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.wan.misc;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
+import static 
org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static 
org.apache.geode.distributed.ConfigurationProperties.REMOTE_LOCATORS;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTHENTICATOR;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTHENTICATOR;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTH_INIT;
+import static 
org.apache.geode.distributed.ConfigurationProperties.START_LOCATOR;
+import static org.apache.geode.test.dunit.Host.getHost;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+import static org.awaitility.Awaitility.waitAtMost;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+import java.security.Principal;
+import java.util.Properties;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.LogWriter;
+import org.apache.geode.cache.AttributesFactory;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.DiskStore;
+import org.apache.geode.cache.DiskStoreFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.cache.wan.GatewayReceiver;
+import org.apache.geode.cache.wan.GatewayReceiverFactory;
+import org.apache.geode.cache.wan.GatewaySender;
+import org.apache.geode.cache.wan.GatewaySenderFactory;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.DistributedSystem;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.cache.wan.InternalGatewaySenderFactory;
+import org.apache.geode.security.AuthInitialize;
+import org.apache.geode.security.AuthenticationFailedException;
+import org.apache.geode.security.Authenticator;
+import org.apache.geode.test.dunit.DistributedTestCase;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
+import org.apache.geode.test.junit.categories.WanTest;
+import 
org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder;
+
+/**
+ * Reproduces bug GEODE-3117: "Gateway authentication throws 
NullPointerException" and validates the
+ * fix.
+ */
+@Category({DistributedTest.class, SecurityTest.class, WanTest.class})
+public class GatewayLegacyAuthenticationRegressionTest extends 
DistributedTestCase {
+
+  private static final String USER_NAME = "security-username";
+  private static final String PASSWORD = "security-password";
+
+  private static final AtomicInteger invokeAuthenticateCount = new 
AtomicInteger();
+
+  private static Cache cache;
+  private static GatewaySender sender;
+
+  private VM londonLocatorVM;
+  private VM newYorkLocatorVM;
+  private VM londonServerVM;
+  private VM newYorkServerVM;
+
+  private String londonName;
+  private String newYorkName;
+
+  

[GitHub] geode issue #601: GEODE-3117: fix Gateway authentication with legacy securit...

2017-06-27 Thread PurelyApplied
Github user PurelyApplied commented on the issue:

https://github.com/apache/geode/pull/601
  
+1.  Huzzah for more test coverage.


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


Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Bruce Schuchardt

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

(Updated June 27, 2017, 12:11 p.m.)


Review request for geode and Barry Oglesby.


Changes
---

Addressing some of the recent code hygene comments


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


Repository: geode


Description
---

EventID and ThreadIdentifier hold the serialized form of portions of an 
InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
and was causing .equals and .hashCode for these objects to return false when 
they should have returned true owing to additional data being in the serialized 
form of the identifier.

This change set modifies the equals and hashCode methods of the classes to 
ensure that they return the correct results in the presence of this additional 
ID data.

I created a dunit test to reproduce the problem but I think the behavior of 
HARegionQueues isn't reliable enough to check in that test.  I'm afraid that it 
would end up being a "flaky" test that fails periodically.  Instead, I'm 
relying on a new junit test and the test that Barry already checked in.

Note: I've also included two other things in this change set.

First, LocalRegion.dumpBackingMap() is a test-hook that should log its results 
at "info" level.  I used it in debugging this problem.

Second, I've added a new backward-compatibility version so now we have 100 and 
110.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java 
a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
  geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
 ec165a5af210966241fdc1cee81702231557cc8c 
  
geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
 29b22be6cc866217f165b755f11e68e1343843fe 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
 5fb8fa201b4c059d5d458a6af0915273f600f69f 
  geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 


Diff: https://reviews.apache.org/r/60446/diff/3/

Changes: https://reviews.apache.org/r/60446/diff/2-3/


Testing
---


Thanks,

Bruce Schuchardt



Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Alexander Murmann

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


Ship it!




All my concerns are about refactoring and code hygiene and at least one of them 
was brough up by Galen. Can we merge this as it is and address the refactoring 
concerns as a follow up commit that goes to develop but not to the release 
branch?

- Alexander Murmann


On June 26, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60446/
> ---
> 
> (Updated June 26, 2017, 10:24 p.m.)
> 
> 
> Review request for geode and Barry Oglesby.
> 
> 
> Bugs: GEODE-3072
> https://issues.apache.org/jira/browse/GEODE-3072
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> EventID and ThreadIdentifier hold the serialized form of portions of an 
> InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
> and was causing .equals and .hashCode for these objects to return false when 
> they should have returned true owing to additional data being in the 
> serialized form of the identifier.
> 
> This change set modifies the equals and hashCode methods of the classes to 
> ensure that they return the correct results in the presence of this 
> additional ID data.
> 
> I created a dunit test to reproduce the problem but I think the behavior of 
> HARegionQueues isn't reliable enough to check in that test.  I'm afraid that 
> it would end up being a "flaky" test that fails periodically.  Instead, I'm 
> relying on a new junit test and the test that Barry already checked in.
> 
> Note: I've also included two other things in this change set.
> 
> First, LocalRegion.dumpBackingMap() is a test-hook that should log its 
> results at "info" level.  I used it in debugging this problem.
> 
> Second, I've added a new backward-compatibility version so now we have 100 
> and 110.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
>  ec165a5af210966241fdc1cee81702231557cc8c 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
>  29b22be6cc866217f165b755f11e68e1343843fe 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  5fb8fa201b4c059d5d458a6af0915273f600f69f 
>   geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 
> 
> 
> Diff: https://reviews.apache.org/r/60446/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Alexander Murmann

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


Ship it!




Ship It!

- Alexander Murmann


On June 26, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60446/
> ---
> 
> (Updated June 26, 2017, 10:24 p.m.)
> 
> 
> Review request for geode and Barry Oglesby.
> 
> 
> Bugs: GEODE-3072
> https://issues.apache.org/jira/browse/GEODE-3072
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> EventID and ThreadIdentifier hold the serialized form of portions of an 
> InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
> and was causing .equals and .hashCode for these objects to return false when 
> they should have returned true owing to additional data being in the 
> serialized form of the identifier.
> 
> This change set modifies the equals and hashCode methods of the classes to 
> ensure that they return the correct results in the presence of this 
> additional ID data.
> 
> I created a dunit test to reproduce the problem but I think the behavior of 
> HARegionQueues isn't reliable enough to check in that test.  I'm afraid that 
> it would end up being a "flaky" test that fails periodically.  Instead, I'm 
> relying on a new junit test and the test that Barry already checked in.
> 
> Note: I've also included two other things in this change set.
> 
> First, LocalRegion.dumpBackingMap() is a test-hook that should log its 
> results at "info" level.  I used it in debugging this problem.
> 
> Second, I've added a new backward-compatibility version so now we have 100 
> and 110.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
>  ec165a5af210966241fdc1cee81702231557cc8c 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
>  29b22be6cc866217f165b755f11e68e1343843fe 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  5fb8fa201b4c059d5d458a6af0915273f600f69f 
>   geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 
> 
> 
> Diff: https://reviews.apache.org/r/60446/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Alexander Murmann

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




geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java
Lines 396 (patched)


Is there any way at all we can make the code explain itself enough so that 
we don't need the comment repeated three times? In all three cases we are 
ensuring that a difference is smaller than the Member data length. Maybe a 
method could be introduced that has a self explanatory name? Maybe something 
like `compareSignificantByteSize(byte[] memberID, int otherSize)`?

Maybe we even could have a class MemberID that is responsible for this 
functionality.



geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java
Lines 437 (patched)


Could the "19" be demagicalized by introducing a well named constant 
instead of a comment?


- Alexander Murmann


On June 26, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60446/
> ---
> 
> (Updated June 26, 2017, 10:24 p.m.)
> 
> 
> Review request for geode and Barry Oglesby.
> 
> 
> Bugs: GEODE-3072
> https://issues.apache.org/jira/browse/GEODE-3072
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> EventID and ThreadIdentifier hold the serialized form of portions of an 
> InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
> and was causing .equals and .hashCode for these objects to return false when 
> they should have returned true owing to additional data being in the 
> serialized form of the identifier.
> 
> This change set modifies the equals and hashCode methods of the classes to 
> ensure that they return the correct results in the presence of this 
> additional ID data.
> 
> I created a dunit test to reproduce the problem but I think the behavior of 
> HARegionQueues isn't reliable enough to check in that test.  I'm afraid that 
> it would end up being a "flaky" test that fails periodically.  Instead, I'm 
> relying on a new junit test and the test that Barry already checked in.
> 
> Note: I've also included two other things in this change set.
> 
> First, LocalRegion.dumpBackingMap() is a test-hook that should log its 
> results at "info" level.  I used it in debugging this problem.
> 
> Second, I've added a new backward-compatibility version so now we have 100 
> and 110.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
>  ec165a5af210966241fdc1cee81702231557cc8c 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
>  29b22be6cc866217f165b755f11e68e1343843fe 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  5fb8fa201b4c059d5d458a6af0915273f600f69f 
>   geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 
> 
> 
> Diff: https://reviews.apache.org/r/60446/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



[GitHub] geode pull request #605: GEODE-2996: incorporating review feedback and addin...

2017-06-27 Thread WireBaron
GitHub user WireBaron opened a pull request:

https://github.com/apache/geode/pull/605

GEODE-2996: incorporating review feedback and adding integration test

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [ x] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to dev@geode.apache.org.


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

$ git pull https://github.com/WireBaron/geode feature/GEODE-2996

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

https://github.com/apache/geode/pull/605.patch

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

This closes #605


commit 3a2d2573151d260e8f030403a03cd47c88e366bf
Author: Alexander Murmann 
Date:   2017-06-27T00:56:06Z

GEODE-2996: incorporating review feedback and adding integration test

Addresses review feedback for GEODE-2996, mainly refactoring 
getOpertionHandler to handle failures like the putOperationHandler
Adding put operations to the RoundTripCacheConnectionJUnitTest, which is 
the integration test for the protobuf module
Removing service loading for protobuf operations and instead have the 
ProtobufStreamProcessor populate its OperationHandlerRegistry
Remove exception throwing from OperationHandler.process calls and remove 
TypeEncodingException

Signed-off-by: Brian Rowe 
Signed-off-by: Alexander Murmann 




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


Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Galen O'Sullivan


> On June 27, 2017, 6:19 p.m., Galen O'Sullivan wrote:
> > The diff doesn't apply cleanly on develop on my machine. What's the base 
> > commit?

Or better, can you merge or rebase on develop please?


- Galen


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


On June 21, 2017, 9:35 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60312/
> ---
> 
> (Updated June 21, 2017, 9:35 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
> O'Sullivan.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Cache InetAddress if configure host as ip string.
> 
> 1. We keep configure host string in HostAddress class
> 2. We reuse InetsocketAddress if it is ipString.
> 3. Client has logic to retry thus we keep InetsocketAddress even if 
>it is not ip string.
> 
> GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.
> 
> GEODE-2940 Now we don't validate configure host string on start. As
> there is possibility that host may not available.
> 
> 1. Earlier wan config were failing because of that. Now we just keep
> those configure host string. And try this later.
> 
> Added unit tests for it.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle 39bb542 
>   geode-assembly/src/test/resources/expected_jars.txt 6260167 
>   geode-core/build.gradle 7f34b4a 
>   
> geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
>  c1bfc93 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  070451c 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
> 3ded54a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
>  01c6157 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  1572355 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef57 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9da 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  84d42cf 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  e9476b5 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
>  5ab1bed 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
>  1dc2fd1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
> d4fdbd0 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
>  789d326 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  9169904 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
>  9f6c5fb 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
>  9d63050 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>  a31fa8d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  6a6e0c1 
>   
> geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
>  f5a8fcf 
>   
> geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
>  d6d5d7c 
>   
> geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
>  dbc2cc6 
>   
> geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
>  6d75064 
>   gradle/dependency-versions.properties 183dafc 
> 
> 
> Diff: https://reviews.apache.org/r/60312/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>



Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Galen O'Sullivan

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



The diff doesn't apply cleanly on develop on my machine. What's the base commit?


geode-assembly/build.gradle
Lines 143 (patched)


It's a nit, but it would be nice to remove trailing whitespace.


- Galen O'Sullivan


On June 21, 2017, 9:35 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60312/
> ---
> 
> (Updated June 21, 2017, 9:35 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
> O'Sullivan.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Cache InetAddress if configure host as ip string.
> 
> 1. We keep configure host string in HostAddress class
> 2. We reuse InetsocketAddress if it is ipString.
> 3. Client has logic to retry thus we keep InetsocketAddress even if 
>it is not ip string.
> 
> GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.
> 
> GEODE-2940 Now we don't validate configure host string on start. As
> there is possibility that host may not available.
> 
> 1. Earlier wan config were failing because of that. Now we just keep
> those configure host string. And try this later.
> 
> Added unit tests for it.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle 39bb542 
>   geode-assembly/src/test/resources/expected_jars.txt 6260167 
>   geode-core/build.gradle 7f34b4a 
>   
> geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
>  c1bfc93 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  070451c 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
> 3ded54a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
>  01c6157 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  1572355 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef57 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9da 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  84d42cf 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  e9476b5 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
>  5ab1bed 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
>  1dc2fd1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
> d4fdbd0 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
>  789d326 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  9169904 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
>  9f6c5fb 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/StartupMessageDataJUnitTest.java
>  9d63050 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>  a31fa8d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  6a6e0c1 
>   
> geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorDiscovery.java
>  f5a8fcf 
>   
> geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java
>  d6d5d7c 
>   
> geode-wan/src/main/java/org/apache/geode/internal/cache/wan/AbstractRemoteGatewaySender.java
>  dbc2cc6 
>   
> geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/WanAutoDiscoveryDUnitTest.java
>  6d75064 
>   gradle/dependency-versions.properties 183dafc 
> 
> 
> Diff: https://reviews.apache.org/r/60312/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>



Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Brian Rowe

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




geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
Line 86 (original), 89 (patched)


Since you're poking around in here anyways, can you clean up the formatting 
here



geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
Line 106 (original), 109 (patched)


This doesn't appear to be the most useful javadoc ever, but at the very 
least you should add the new parameter to it.



geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
Lines 117 (patched)


Did you mean to leave this comment in?



geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
Line 161 (original), 161 (patched)


Looks like you wanted to ask Bruce about this, you probably shouldn't merge 
this comment.



geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
Lines 235 (patched)


Can we remove all of the the this. calls in favor of just using 
 directly?  Failing that, can you change 225 and 226 to this.hostname 
for consistency.



geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java
Lines 59 (patched)


Typo, s/loators/locators



geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java
Lines 308 (patched)


remove this


- Brian Rowe


On June 21, 2017, 9:35 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60312/
> ---
> 
> (Updated June 21, 2017, 9:35 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, and Galen 
> O'Sullivan.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2804 Cache InetAddress if configure host as ip string.
> 
> 1. We keep configure host string in HostAddress class
> 2. We reuse InetsocketAddress if it is ipString.
> 3. Client has logic to retry thus we keep InetsocketAddress even if 
>it is not ip string.
> 
> GEODE-3017 IPv6 issue on windows. Above changes fixed this issue.
> 
> GEODE-2940 Now we don't validate configure host string on start. As
> there is possibility that host may not available.
> 
> 1. Earlier wan config were failing because of that. Now we just keep
> those configure host string. And try this later.
> 
> Added unit tests for it.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle 39bb542 
>   geode-assembly/src/test/resources/expected_jars.txt 6260167 
>   geode-core/build.gradle 7f34b4a 
>   
> geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java
>  c1bfc93 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  070451c 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/PoolImpl.java 
> 3ded54a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
>  01c6157 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  1572355 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
>  c6bef57 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  93fa9da 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  84d42cf 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/HostAddress.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  e9476b5 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java
>  5ab1bed 
>   
> geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java
>  1dc2fd1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/PoolFactoryImpl.java 
> d4fdbd0 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java
>  789d326 
>   
> 

Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Bruce Schuchardt


> On June 27, 2017, 10:53 a.m., Galen O'Sullivan wrote:
> > MembershipID seems like a good candidate for extraction into its own class. 
> > This would make, for example, `equals` cleaner. I guess there are 
> > performance reasons for it.
> > 
> > When does the problem occur? Is it when a client reconnects with a newer 
> > version and the same ID? I wonder, would it be possible to make a check for 
> > that case specifically?

This is a blocker for the 1.2.0 release so no refactoring.

The problem occurs when the ID is serialized using a versioned stream with a 
different version than that of the client.  The client's version is not known 
in that code, which is one source of the problem.  There are other places like 
this that need to be taken care of (VMCachedDeserializables in M code, for 
example) but that's not something to tackle here.


> On June 27, 2017, 10:53 a.m., Galen O'Sullivan wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java
> > Lines 415 (patched)
> > 
> >
> > We've just checked whether the length is equal; does this assert 
> > actually check anything?

The assert's already been removed (Kirk's comment)


> On June 27, 2017, 10:53 a.m., Galen O'Sullivan wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
> > Lines 40 (patched)
> > 
> >
> > Is it possible for a Membership ID to be less than 17 bytes and this to 
> > be negative?

No, that's not possible


- Bruce


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


On June 26, 2017, 3:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60446/
> ---
> 
> (Updated June 26, 2017, 3:24 p.m.)
> 
> 
> Review request for geode and Barry Oglesby.
> 
> 
> Bugs: GEODE-3072
> https://issues.apache.org/jira/browse/GEODE-3072
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> EventID and ThreadIdentifier hold the serialized form of portions of an 
> InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
> and was causing .equals and .hashCode for these objects to return false when 
> they should have returned true owing to additional data being in the 
> serialized form of the identifier.
> 
> This change set modifies the equals and hashCode methods of the classes to 
> ensure that they return the correct results in the presence of this 
> additional ID data.
> 
> I created a dunit test to reproduce the problem but I think the behavior of 
> HARegionQueues isn't reliable enough to check in that test.  I'm afraid that 
> it would end up being a "flaky" test that fails periodically.  Instead, I'm 
> relying on a new junit test and the test that Barry already checked in.
> 
> Note: I've also included two other things in this change set.
> 
> First, LocalRegion.dumpBackingMap() is a test-hook that should log its 
> results at "info" level.  I used it in debugging this problem.
> 
> Second, I've added a new backward-compatibility version so now we have 100 
> and 110.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
>  ec165a5af210966241fdc1cee81702231557cc8c 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
>  29b22be6cc866217f165b755f11e68e1343843fe 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  5fb8fa201b4c059d5d458a6af0915273f600f69f 
>   geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 
> 
> 
> Diff: https://reviews.apache.org/r/60446/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt
> 
> 
> Thanks,
> 
> Bruce 

Re: Review Request 60442: GEODE-3130: Refactoring AcceptorImpl communication mode switch

2017-06-27 Thread Galen O'Sullivan

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


Ship it!




Ship It!

- Galen O'Sullivan


On June 26, 2017, 6:31 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60442/
> ---
> 
> (Updated June 26, 2017, 6:31 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3130
> https://issues.apache.org/jira/browse/GEODE-3130
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This review addresses Udo's last feedback for GEODE-2995.  This change also 
> cleaned up the imports on the AcceptorImplJUnitTest.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  50f7006fa 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplJUnitTest.java
>  7aa11b7ca 
> 
> 
> Diff: https://reviews.apache.org/r/60442/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin in progress.
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>



Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Galen O'Sullivan

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


Fix it, then Ship it!




MembershipID seems like a good candidate for extraction into its own class. 
This would make, for example, `equals` cleaner. I guess there are performance 
reasons for it.

When does the problem occur? Is it when a client reconnects with a newer 
version and the same ID? I wonder, would it be possible to make a check for 
that case specifically?


geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java
Lines 415 (patched)


We've just checked whether the length is equal; does this assert actually 
check anything?



geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
Lines 40 (patched)


Is it possible for a Membership ID to be less than 17 bytes and this to be 
negative?


- Galen O'Sullivan


On June 26, 2017, 10:24 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60446/
> ---
> 
> (Updated June 26, 2017, 10:24 p.m.)
> 
> 
> Review request for geode and Barry Oglesby.
> 
> 
> Bugs: GEODE-3072
> https://issues.apache.org/jira/browse/GEODE-3072
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> EventID and ThreadIdentifier hold the serialized form of portions of an 
> InternalDistributedMember identifier.  This serialized form changed in v1.0.0 
> and was causing .equals and .hashCode for these objects to return false when 
> they should have returned true owing to additional data being in the 
> serialized form of the identifier.
> 
> This change set modifies the equals and hashCode methods of the classes to 
> ensure that they return the correct results in the presence of this 
> additional ID data.
> 
> I created a dunit test to reproduce the problem but I think the behavior of 
> HARegionQueues isn't reliable enough to check in that test.  I'm afraid that 
> it would end up being a "flaky" test that fails periodically.  Instead, I'm 
> relying on a new junit test and the test that Barry already checked in.
> 
> Note: I've also included two other things in this change set.
> 
> First, LocalRegion.dumpBackingMap() is a test-hook that should log its 
> results at "info" level.  I used it in debugging this problem.
> 
> Second, I've added a new backward-compatibility version so now we have 100 
> and 110.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  a1b4a9d5684d0148bd9e72c00c674afa299b2b9d 
>   geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java 
> 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java
>  ec165a5af210966241fdc1cee81702231557cc8c 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java
>  29b22be6cc866217f165b755f11e68e1343843fe 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java
>  5fb8fa201b4c059d5d458a6af0915273f600f69f 
>   geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 
> 
> 
> Diff: https://reviews.apache.org/r/60446/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt
> diffall.txt
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Fixed: apache/geode#3023 (develop - 718583b)

2017-06-27 Thread Travis CI
Build Update for apache/geode
-

Build: #3023
Status: Fixed

Duration: 26 minutes and 4 seconds
Commit: 718583b (develop)
Author: Jinmei Liao
Message: GEODE-2919: revert: fix lucene security tests

View the changeset: 
https://github.com/apache/geode/compare/e5204ce12245...718583b29485

View the full build log and details: 
https://travis-ci.org/apache/geode/builds/247595939?utm_source=email_medium=notification

--

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



Broken: apache/geode#3019 (develop - e5204ce)

2017-06-27 Thread Travis CI
Build Update for apache/geode
-

Build: #3019
Status: Broken

Duration: 21 minutes and 27 seconds
Commit: e5204ce (develop)
Author: Jinmei Liao
Message: GEODE-2919: fix lucene security tests

View the changeset: 
https://github.com/apache/geode/compare/137ced6bea48...e5204ce12245

View the full build log and details: 
https://travis-ci.org/apache/geode/builds/247572519?utm_source=email_medium=notification

--

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



Re: Review Request 60442: GEODE-3130: Refactoring AcceptorImpl communication mode switch

2017-06-27 Thread Alexander Murmann

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


Ship it!




Ship It!

- Alexander Murmann


On June 26, 2017, 6:31 p.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60442/
> ---
> 
> (Updated June 26, 2017, 6:31 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3130
> https://issues.apache.org/jira/browse/GEODE-3130
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> This review addresses Udo's last feedback for GEODE-2995.  This change also 
> cleaned up the imports on the AcceptorImplJUnitTest.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  50f7006fa 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplJUnitTest.java
>  7aa11b7ca 
> 
> 
> Diff: https://reviews.apache.org/r/60442/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin in progress.
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>



New Apache commons-validato.jar

2017-06-27 Thread Hitesh Khamesra
We are planning to include Apache commons-validator_1.6.jar in Geode. As part 
of GEODE-2804, we need to validate whether host string is configured as IP or 
not. Please let us know if there is any issue with it.

Thanks.
Hitesh



[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #598 was SUCCESSFUL (with 1908 tests). Change made by John Blum.

2017-06-27 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #598 was successful.
---
This was manually triggered by John Blum.
1910 tests in total.

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




--
Code Changes
--
John Blum (1d5597afb343467bcb14b9f0e7b44a487afde6c7):

>DATAGEODE-15 - Adapt to API changes in RepositoryConfigurationSourceSupport.

John Blum (08b1a147aac8f51db71aa314c1be3c186e029c86):

>DATAGEODE-14 - Improve IndexFactoryBean's resilience and options for handling 
>GemFire IndexExistsExceptions and IndexNameConflictExceptions.
>Related JIRA: https://jira.spring.io/browse/SGF-637
>
>(cherry picked from commit e868c58db9d245adab005b6a5ae1cc0a94275cf3)
>Signed-off-by: John Blum 



--
Tests
--
Fixed Tests (2)
   - GemfireRepositoryConfigurationExtensionTest: Post process with xml 
repository configuration source
   - GemfireRepositoryConfigurationExtensionTest: Post process with xml 
repository configuration source having no mapping context ref attribute

--
This message is automatically generated by Atlassian Bamboo

want to encrypt default username/password in properties files or xml with default encrypted values.

2017-06-27 Thread Dinesh Akhand
Hi Team,

Default user name /password are defined in spring-security.xml  and 
pulse.properties file.
As below.

./geode/locator0/GemFire_anmuser/services/http/0.0.0.0_20248_pulse/webapp/WEB-INF/spring-security.xml:
https://www.amdocs.com/about/email-disclaimer