Re: [VOTE] Apache Geode release - v1.2.0 RC1

2017-06-19 Thread Jinmei Liao
fix for the above 2 issues are merged to 1.2 branch.

On Mon, Jun 19, 2017 at 1:36 PM, Jinmei Liao  wrote:

> -1, due the recent discovery of GEODE-3095
>  and GEODE-3092
> 
>
>
> On Sat, Jun 17, 2017 at 2:57 PM, Anthony Baker  wrote:
>
>> Thanks Mike!  Here’s the corrected text:
>>
>> This is the first release candidate for Apache Geode, version
>> 1.2.0.  Thanks to all the community members for their
>> contributions to this release!
>>
>> *** Please download, test and vote by Wednesday, June 19, 1700 hrs US
>> Pacific. ***
>>
>> It fixes the following issues:
>>https://issues.apache.org/jira/secure/ReleaseNote.jspa?
>> projectId=12318420=12339257
>>
>> Note that we are voting upon the source tags:  rel/v1.2.0.RC1
>>https://git-wip-us.apache.org/repos/asf?p=geode.git;a=commit;h=
>> b6598890d41dc607ca4897ead04a75dd77f3e852
>> 
>>https://git-wip-us.apache.org/repos/asf?p=geode-examples.
>> git;a=commit;h=7f93d95ad06a6f2afee54312585f48435fff11e8
>>
>> Commit ID:
>>b6598890d41dc607ca4897ead04a75dd77f3e852 (geode)
>>7f93d95ad06a6f2afee54312585f48435fff11e8 (geode-examples)
>>
>> Source and binary files:
>>  https://dist.apache.org/repos/dist/dev/geode/1.2.0.RC1
>>
>> Maven staging repo:
>>https://repository.apache.org/content/repositories/orgapachegeode-1019
>>
>> Geode's KEYS file containing PGP keys we use to sign the release:
>>https://git-wip-us.apache.org/repos/asf?p=geode.git;a=blob_
>> plain;f=KEYS;hb=HEAD
>>
>> pub  4096R/C72CFB64 2015-10-01
>>Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
>>
>> Anthony
>>
>>
>>
>> > On Jun 17, 2017, at 2:35 PM, Michael Stolz  wrote:
>> >
>> > That shouldn't say incubating should  it?
>> >
>> > --
>> > Mike Stolz
>> > Principal Engineer - Gemfire Product Manager
>> > Mobile: 631-835-4771
>> >
>> > On Jun 17, 2017 12:01 PM, "Anthony Baker"  wrote:
>> >
>>
>>
>
>
> --
> Cheers
>
> Jinmei
>



-- 
Cheers

Jinmei


Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Galen O'Sullivan


> On June 19, 2017, 6:40 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
> > Line 1426 (original), 1426 (patched)
> > 
> >
> > It seems to me that a protobuf client connection should be reported as 
> > such and not as an empty string.

good catch, fixed.


> On June 19, 2017, 6:40 p.m., Bruce Schuchardt wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryIntegrationTest.java
> > Lines 45 (patched)
> > 
> >
> > Use the RestoreSystemProperties junit rule here & remove the try/finally

good call, fixed.


> On June 19, 2017, 6:40 p.m., Bruce Schuchardt wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
> > Lines 47 (patched)
> > 
> >
> > Since you're testing both with & without the system property being set 
> > you should add a try/finally to this test to clear the property.

Missed that. Good catch.


- Galen


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


On June 20, 2017, 12:53 a.m., Galen O'Sullivan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60157/
> ---
> 
> (Updated June 20, 2017, 12:53 a.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Create `ServerConnectionFactory`, which creates either instances of 
> `LegacyServerConnection` (identical in functionality to the old 
> `ServerConnection`) or `NewProtocolServerConnection` (which is currently 
> basically a stub, but will never get called unless a feature flag is set).
> 
> This is the initial work for GEODE-3074, and will allow us to continue to 
> work on further tasks for that project.
> 
> An explicit goal with this PR is to not disturb any existing functionality. 
> Unless a feature flag is set and a connection with a certain magic byte is 
> received, server connections will work as before. If you see something that 
> you think may break, please let me know.
> 
> Have a nice day!
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  85f914637 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  5eaa5a4bd 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> 9a3241b05 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2a8818cef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e0b5ab8b6 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolMessageHandler.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/LegacyServerConnection.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  947b83652 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
>  2cb8d0817 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  171cfb71a 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
>  c594bf932 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  feea8995d 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
>  2e0ad956c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
>  0e5029bd0 
>   
> 

Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Galen O'Sullivan


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
> > Lines 1424-1426 (original), 1424-1426 (patched)
> > 
> >
> > this conditional is becoming exponentially worse. Could we maybe 
> > extract this to a method/function?

I've made a switch statement above, which is much better.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
> > Line 1427 (original), 1427 (patched)
> > 
> >
> > This does not seem to be used anywhere anymore. Could you please check 
> > and remove

This is used in the constructor for ServerConnection and goes into the thread 
name and possibly elsewhere.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
> > Lines 1428-1441 (original), 1428-1441 (patched)
> > 
> >
> > Doubt that this only needs to happen inside of this conditional. Maybe 
> > we extract this out into its own method and then call it outside of the 
> > method

I made a switch statement, have a look again and see if you like it.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
> > Lines 1429-1431 (original), 1429-1431 (patched)
> > 
> >
> > should we have a similar setting for the CLIENT_TO_SERVER_NEW_PROTOCOL?

yes, good catch, thanks. Fixed.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
> > Lines 32 (patched)
> > 
> >
> > Maybe we rename this to GenericProtocolServerConnection

I don't like the name because it's generic, but it works well enough.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
> > Lines 35 (patched)
> > 
> >
> > clientProtocolMessageHandler

Renaming to `messageHandler`.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
> > Lines 54 (patched)
> > 
> >
> > could we rename this to clientProtocolHandler or 
> > clientProtocolMessageHandler instead of newClientProtocol

That name doesn't give the user any information.. I like 
`protobufProtocolHandler` better.


> On June 19, 2017, 4:24 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
> > Lines 58 (patched)
> > 
> >
> > surely we don't need this here... How do we handle the case if we have 
> > another client protocol handler... this would fail.

We can test this at the factory level. I hadn't been thinking of this as being 
so generic, but it can be pretty easily.


- Galen


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


On June 20, 2017, 12:53 a.m., Galen O'Sullivan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60157/
> ---
> 
> (Updated June 20, 2017, 12:53 a.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Create `ServerConnectionFactory`, which creates either instances of 
> `LegacyServerConnection` (identical in functionality to the old 
> `ServerConnection`) or `NewProtocolServerConnection` (which is currently 
> basically a stub, but will never get called unless a feature flag is set).
> 
> This is the initial work for GEODE-3074, and will allow us to continue to 
> work on further tasks for that project.
> 
> An explicit goal with this PR is to not disturb any existing functionality. 
> Unless a feature flag is set and a connection with a certain magic byte is 
> received, server connections will work as before. If you see something that 
> 

Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Galen O'Sullivan

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

(Updated June 20, 2017, 12:53 a.m.)


Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh Khamesra, 
Udo Kohlmeyer, and Brian Rowe.


Changes
---

Incorporated changes and did some refactoring of `AcceptorImpl`.


Repository: geode


Description
---

Create `ServerConnectionFactory`, which creates either instances of 
`LegacyServerConnection` (identical in functionality to the old 
`ServerConnection`) or `NewProtocolServerConnection` (which is currently 
basically a stub, but will never get called unless a feature flag is set).

This is the initial work for GEODE-3074, and will allow us to continue to work 
on further tasks for that project.

An explicit goal with this PR is to not disturb any existing functionality. 
Unless a feature flag is set and a connection with a certain magic byte is 
received, server connections will work as before. If you see something that you 
think may break, please let me know.

Have a nice day!


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 85f914637 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
5eaa5a4bd 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
9a3241b05 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 2a8818cef 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
 e0b5ab8b6 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolMessageHandler.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/LegacyServerConnection.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
 947b83652 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
 2cb8d0817 
  
geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 171cfb71a 
  
geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
 c594bf932 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 feea8995d 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
 2e0ad956c 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
 0e5029bd0 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/SecurityManagerProvider.java
 ad8e66e0c 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
 b0e20d9b3 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
 794c61097 
  
geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
 afa007f31 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
 daaf18d3f 
  
geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
 bac79ec0e 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
 e8548ed86 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java
 fc4447bb0 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java
 4b7bbfc5a 
  

Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

2017-06-19 Thread Brian Rowe

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

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


Repository: geode


Description
---

This change adds a new module for handling client stresms encoded using the new 
ProtoBuf protocol.  At the top level this can be integrated by passing in the 
input/output streams and cache reference to the ProtobufStreamProcessor.  This 
will decode the message and ultimately dispatch it to an operation specific 
handler which will call back into the passed cache object.  Note that this not 
currently hooked up to geode at all, GEODE-3075 is tracking the work needed to 
hook this up at that level.

This change mainly contains the plumbing and encoding/decoding logic, but also 
contain the Get operation handler as a proof of concept.  Future work will be 
needed to handle other types of operations.


Diffs
-

  geode-protobuf/build.gradle PRE-CREATION 
  geode-protobuf/src/main/java/org/apache/geode/ProtobufUtilities.java 
PRE-CREATION 
  geode-protobuf/src/main/java/org/apache/geode/protocol/OpsProcessor.java 
PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/exception/InvalidProtocolMessageException.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/handler/ProtobufStreamProcessor.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/handler/ProtocolHandler.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/handler/protobuf/ProtobufProtocolHandler.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/ProtobufRequestOperationParser.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/protobuf/GetRequestOperationHandler.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerAlreadyRegisteredException.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerNotRegisteredException.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/ProtobufSerializationService.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationService.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationType.java
 PRE-CREATION 
  geode-protobuf/src/main/java/org/apache/geode/serialization/TypeCodec.java 
PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BinaryCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BooleanCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ByteCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/DoubleCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/FloatCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/IntCodec.java 
PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/JSONCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/LongCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ShortCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/codec/StringCodec.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/exception/UnsupportedEncodingTypeException.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/protobuf/translation/EncodingTypeTranslator.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/protobuf/translation/exception/UnsupportedEncodingTypeException.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/registry/SerializationCodecRegistry.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/registry/exception/CodecAlreadyRegisteredForTypeException.java
 PRE-CREATION 
  
geode-protobuf/src/main/java/org/apache/geode/serialization/registry/exception/CodecNotRegisteredForTypeException.java
 PRE-CREATION 
  geode-protobuf/src/main/proto/basicTypes.proto PRE-CREATION 
  geode-protobuf/src/main/proto/clientProtocol.proto PRE-CREATION 
  geode-protobuf/src/main/proto/region_API.proto PRE-CREATION 
  geode-protobuf/src/main/proto/server_API.proto 

[GitHub] geode pull request #591: GEODE-3073: Renamed OrderByComparatorUnmapped to Or...

2017-06-19 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/591#discussion_r122849064
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
 ---
@@ -941,17 +940,16 @@ private SelectResults 
prepareEmptyResultSet(ExecutionContext context, boolean ig
 SelectResults results;
 if (this.distinct || !this.count) {
--- End diff --

This could stand to be flattened as well, although it will be more 
difficult that the above.  I could go either way on this chunk.


---
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 #591: GEODE-3073: Renamed OrderByComparatorUnmapped to Or...

2017-06-19 Thread PurelyApplied
Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/591#discussion_r122848112
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
 ---
@@ -1415,11 +1413,7 @@ public boolean evaluateCq(ExecutionContext context) 
throws FunctionDomainExcepti
 // add UNDEFINED to results only for NOT EQUALS queries
 if (this.whereClause.getType() == COMPARISON) {
   int operator = ((Filter) this.whereClause).getOperator();
-  if ((operator != TOK_NE && operator != TOK_NE_ALT)) {
-return false;
-  } else {
-return true;
-  }
+  return !(operator != TOK_NE && operator != TOK_NE_ALT);
--- End diff --

That's a lot of negatives.  `return operator == TOK_NE || operator == 
TOK_NE_ALT` read better, in my opinion.


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


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

2017-06-19 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #590 was successful.
---
Scheduled
1870 tests in total.

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





--
This message is automatically generated by Atlassian Bamboo

Build failed in Jenkins: Geode-nightly #871

2017-06-19 Thread Apache Jenkins Server
See 

--
[...truncated 970.53 KB...]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at hydra.MethExecutor.executeObject(MethExecutor.java:245)
at 
org.apache.geode.test.dunit.standalone.RemoteDUnitVM.executeMethodOnObject(RemoteDUnitVM.java:70)
at sun.reflect.GeneratedMethodAccessor2.invoke(Unknown Source)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at sun.rmi.server.UnicastServerRef.dispatch(UnicastServerRef.java:346)
at sun.rmi.transport.Transport$1.run(Transport.java:200)
at sun.rmi.transport.Transport$1.run(Transport.java:197)
at java.security.AccessController.doPrivileged(Native Method)
at sun.rmi.transport.Transport.serviceCall(Transport.java:196)
at 
sun.rmi.transport.tcp.TCPTransport.handleMessages(TCPTransport.java:568)
at 
sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run0(TCPTransport.java:826)
at 
sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.lambda$run$0(TCPTransport.java:683)
at java.security.AccessController.doPrivileged(Native Method)
at 
sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run(TCPTransport.java:682)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:748)

org.apache.geode.cache.wan.GatewayReceiverAutoConnectionSourceDUnitTest > 
testBridgeServerAndGatewayReceiverClientAndServerWithGroup FAILED
java.lang.AssertionError: Suspicious strings were written to the log during 
this run.
Fix the strings or use IgnoredException.addIgnoredException to ignore.
---
Found suspect string in log4j at line 359

[error 2017/06/19 18:37:58.491 UTC  
tid=0x13] failed setting interface to /127.0.1.1: java.net.SocketException: bad 
argument for IP_MULTICAST_IF: address not bound to any interface
java.net.SocketException: bad argument for IP_MULTICAST_IF: address not 
bound to any interface
at java.net.PlainDatagramSocketImpl.socketSetOption0(Native Method)
at 
java.net.PlainDatagramSocketImpl.socketSetOption(PlainDatagramSocketImpl.java:74)
at 
java.net.AbstractPlainDatagramSocketImpl.setOption(AbstractPlainDatagramSocketImpl.java:309)
at java.net.MulticastSocket.setInterface(MulticastSocket.java:471)
at org.jgroups.protocols.UDP.setInterface(UDP.java:443)
at org.jgroups.protocols.UDP.createMulticastSocket(UDP.java:511)
at 
org.jgroups.protocols.UDP.createMulticastSocketWithBindPort(UDP.java:494)
at org.jgroups.protocols.UDP.createSockets(UDP.java:348)
at org.jgroups.protocols.UDP.start(UDP.java:266)
at org.jgroups.stack.ProtocolStack.startStack(ProtocolStack.java:966)
at org.jgroups.JChannel.startStack(JChannel.java:889)
at org.jgroups.JChannel._preConnect(JChannel.java:553)
at org.jgroups.JChannel.connect(JChannel.java:288)
at org.jgroups.JChannel.connect(JChannel.java:279)
at 
org.apache.geode.distributed.internal.membership.gms.messenger.JGroupsMessenger.start(JGroupsMessenger.java:344)
at 
org.apache.geode.distributed.internal.membership.gms.Services.start(Services.java:156)
at 
org.apache.geode.distributed.internal.membership.gms.GMSMemberFactory.newMembershipManager(GMSMemberFactory.java:107)
at 
org.apache.geode.distributed.internal.membership.MemberFactory.newMembershipManager(MemberFactory.java:91)
at 
org.apache.geode.distributed.internal.DistributionManager.(DistributionManager.java:1155)
at 
org.apache.geode.distributed.internal.DistributionManager.(DistributionManager.java:1204)
at 
org.apache.geode.distributed.internal.DistributionManager.create(DistributionManager.java:573)
at 
org.apache.geode.distributed.internal.InternalDistributedSystem.initialize(InternalDistributedSystem.java:712)
at 
org.apache.geode.distributed.internal.InternalDistributedSystem.newInstance(InternalDistributedSystem.java:326)
at 
org.apache.geode.distributed.internal.InternalDistributedSystem.newInstance(InternalDistributedSystem.java:312)
at 
org.apache.geode.distributed.internal.InternalDistributedSystem.newInstance(InternalDistributedSystem.java:306)
at 

Re: Review Request 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

2017-06-19 Thread Ken Howe

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


Ship it!




Ship It!

- Ken Howe


On June 19, 2017, 9:45 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60200/
> ---
> 
> (Updated June 19, 2017, 9:45 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3095: fix parameter type mismatch between the diskstore command and 
> controller
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java
>  c613a8a1d5cca93ce7be785b58f6502d96752d8c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  ea4b60d4ec72def55129706d7e4d8f297d87ddcb 
> 
> 
> Diff: https://reviews.apache.org/r/60200/diff/3/
> 
> 
> Testing
> ---
> 
> GfshCommandOverHttpTest, GfshSecurityCommandOverHttpTest
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



[GitHub] geode issue #592: GEODE-2707: Removing TXLockToken

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

https://github.com/apache/geode/pull/592
  
After the fix Ken pointed out, `++n;`


---
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 issue #585: GEODE-3091: remove empty method

2017-06-19 Thread agingade
Github user agingade commented on the issue:

https://github.com/apache/geode/pull/585
  
The changes looks good.


---
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 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

2017-06-19 Thread Emily Yeh

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


Ship it!




Ship It!

- Emily Yeh


On June 19, 2017, 9:12 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60200/
> ---
> 
> (Updated June 19, 2017, 9:12 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3095: fix parameter type mismatch between the diskstore command and 
> controller
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java
>  c613a8a1d5cca93ce7be785b58f6502d96752d8c 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreOverHttpTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60200/diff/2/
> 
> 
> Testing
> ---
> 
> GfshCommandOverHttpTest, GfshSecurityCommandOverHttpTest
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

2017-06-19 Thread Ken Howe

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


Ship it!




Ship It!

- Ken Howe


On June 19, 2017, 9:12 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60200/
> ---
> 
> (Updated June 19, 2017, 9:12 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3095: fix parameter type mismatch between the diskstore command and 
> controller
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java
>  c613a8a1d5cca93ce7be785b58f6502d96752d8c 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreOverHttpTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60200/diff/2/
> 
> 
> Testing
> ---
> 
> GfshCommandOverHttpTest, GfshSecurityCommandOverHttpTest
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 60202: GEODE-3056: fix the message for invalid partition-resolver

2017-06-19 Thread Ken Howe

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


Ship it!




Ship It!

- Ken Howe


On June 19, 2017, 7:08 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60202/
> ---
> 
> (Updated June 19, 2017, 7:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3056: fix the message for invalid partition-resolver
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  842802ba7b7ed34aa52974dcf327015051f22e1b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  47d150d180dada8066f1e644b293c56096b2a969 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60202/diff/1/
> 
> 
> Testing
> ---
> 
> newly added test and precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

2017-06-19 Thread Jinmei Liao

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

(Updated June 19, 2017, 9:12 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Changes
---

add an integration test


Repository: geode


Description
---

GEODE-3095: fix parameter type mismatch between the diskstore command and 
controller


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java
 c613a8a1d5cca93ce7be785b58f6502d96752d8c 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreOverHttpTest.java
 PRE-CREATION 


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

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


Testing
---

GfshCommandOverHttpTest, GfshSecurityCommandOverHttpTest


Thanks,

Jinmei Liao



Re: Review Request 60202: GEODE-3056: fix the message for invalid partition-resolver

2017-06-19 Thread Patrick Rhomberg

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


Ship it!




Ship It!

- Patrick Rhomberg


On June 19, 2017, 7:08 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60202/
> ---
> 
> (Updated June 19, 2017, 7:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3056: fix the message for invalid partition-resolver
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  842802ba7b7ed34aa52974dcf327015051f22e1b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  47d150d180dada8066f1e644b293c56096b2a969 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60202/diff/1/
> 
> 
> Testing
> ---
> 
> newly added test and precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 60202: GEODE-3056: fix the message for invalid partition-resolver

2017-06-19 Thread Patrick Rhomberg

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




geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
Lines 17 (patched)


One file at a time, we approach a canonical form.  Huzzah!


- Patrick Rhomberg


On June 19, 2017, 7:08 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60202/
> ---
> 
> (Updated June 19, 2017, 7:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3056: fix the message for invalid partition-resolver
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  842802ba7b7ed34aa52974dcf327015051f22e1b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  47d150d180dada8066f1e644b293c56096b2a969 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60202/diff/1/
> 
> 
> Testing
> ---
> 
> newly added test and precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

2017-06-19 Thread Patrick Rhomberg

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


Ship it!




Ship It!

- Patrick Rhomberg


On June 19, 2017, 6:04 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60200/
> ---
> 
> (Updated June 19, 2017, 6:04 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3095: fix parameter type mismatch between the diskstore command and 
> controller
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java
>  c613a8a1d5cca93ce7be785b58f6502d96752d8c 
> 
> 
> Diff: https://reviews.apache.org/r/60200/diff/1/
> 
> 
> Testing
> ---
> 
> GfshCommandOverHttpTest, GfshSecurityCommandOverHttpTest
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: [VOTE] Apache Geode release - v1.2.0 RC1

2017-06-19 Thread Jinmei Liao
-1, due the recent discovery of GEODE-3095
 and GEODE-3092



On Sat, Jun 17, 2017 at 2:57 PM, Anthony Baker  wrote:

> Thanks Mike!  Here’s the corrected text:
>
> This is the first release candidate for Apache Geode, version
> 1.2.0.  Thanks to all the community members for their
> contributions to this release!
>
> *** Please download, test and vote by Wednesday, June 19, 1700 hrs US
> Pacific. ***
>
> It fixes the following issues:
>https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420=12339257
>
> Note that we are voting upon the source tags:  rel/v1.2.0.RC1
>https://git-wip-us.apache.org/repos/asf?p=geode.git;a=commit;h=
> b6598890d41dc607ca4897ead04a75dd77f3e852
>https://git-wip-us.apache.org/repos/asf?p=geode-examples.
> git;a=commit;h=7f93d95ad06a6f2afee54312585f48435fff11e8
>
> Commit ID:
>b6598890d41dc607ca4897ead04a75dd77f3e852 (geode)
>7f93d95ad06a6f2afee54312585f48435fff11e8 (geode-examples)
>
> Source and binary files:
>  https://dist.apache.org/repos/dist/dev/geode/1.2.0.RC1
>
> Maven staging repo:
>https://repository.apache.org/content/repositories/orgapachegeode-1019
>
> Geode's KEYS file containing PGP keys we use to sign the release:
>https://git-wip-us.apache.org/repos/asf?p=geode.git;a=blob_
> plain;f=KEYS;hb=HEAD
>
> pub  4096R/C72CFB64 2015-10-01
>Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64
>
> Anthony
>
>
>
> > On Jun 17, 2017, at 2:35 PM, Michael Stolz  wrote:
> >
> > That shouldn't say incubating should  it?
> >
> > --
> > Mike Stolz
> > Principal Engineer - Gemfire Product Manager
> > Mobile: 631-835-4771
> >
> > On Jun 17, 2017 12:01 PM, "Anthony Baker"  wrote:
> >
>
>


-- 
Cheers

Jinmei


Review Request 60202: GEODE-3056: fix the message for invalid partition-resolver

2017-06-19 Thread Jinmei Liao

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

Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description
---

GEODE-3056: fix the message for invalid partition-resolver


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
 842802ba7b7ed34aa52974dcf327015051f22e1b 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
 47d150d180dada8066f1e644b293c56096b2a969 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsTest.java
 PRE-CREATION 


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


Testing
---

newly added test and precheckin running


Thanks,

Jinmei Liao



Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Bruce Schuchardt

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



I just have a few small things.


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


It seems to me that a protobuf client connection should be reported as such 
and not as an empty string.



geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryIntegrationTest.java
Lines 45 (patched)


Use the RestoreSystemProperties junit rule here & remove the try/finally



geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
Lines 47 (patched)


Since you're testing both with & without the system property being set you 
should add a try/finally to this test to clear the property.


- Bruce Schuchardt


On June 16, 2017, 5:14 p.m., Galen O'Sullivan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60157/
> ---
> 
> (Updated June 16, 2017, 5:14 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Create `ServerConnectionFactory`, which creates either instances of 
> `LegacyServerConnection` (identical in functionality to the old 
> `ServerConnection`) or `NewProtocolServerConnection` (which is currently 
> basically a stub, but will never get called unless a feature flag is set).
> 
> This is the initial work for GEODE-3074, and will allow us to continue to 
> work on further tasks for that project.
> 
> An explicit goal with this PR is to not disturb any existing functionality. 
> Unless a feature flag is set and a connection with a certain magic byte is 
> received, server connections will work as before. If you see something that 
> you think may break, please let me know.
> 
> Have a nice day!
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> 9a3241b05 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2a8818cef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e0b5ab8b6 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolMessageHandler.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/LegacyServerConnection.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  947b83652 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  794c61097 
> 
> 
> Diff: https://reviews.apache.org/r/60157/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin passed (on a version without the integration test, but I'd 
> consider it pretty much equivalent. If you want me to run again, I will.)
> 
> 
> Thanks,
> 
> Galen O'Sullivan
> 
>



[GitHub] geode issue #592: GEODE-2707: Removing TXLockToken

2017-06-19 Thread galen-pivotal
Github user galen-pivotal commented on the issue:

https://github.com/apache/geode/pull/592
  
👍  thanks for pushing this through! It had completely fallen off my 
radar.


---
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 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

2017-06-19 Thread Jinmei Liao

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

Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description
---

GEODE-3095: fix parameter type mismatch between the diskstore command and 
controller


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java
 c613a8a1d5cca93ce7be785b58f6502d96752d8c 


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


Testing
---

GfshCommandOverHttpTest, GfshSecurityCommandOverHttpTest


Thanks,

Jinmei Liao



[GitHub] geode pull request #592: GEODE-2707: Removing TXLockToken

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

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

GEODE-2707: Removing TXLockToken

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

It seems that there are no other classes that use the `TXLockToken` class, 
so it was safely removed. It's also an internal class, so it doesn't seem 
likely that the removal of this class will break any backwards compatibility.

**_Precheckin status: In progress_**

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/YehEmily/geode GEODE-2707

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

https://github.com/apache/geode/pull/592.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 #592


commit e51f2047136ea4cdaa09c22a3cc06290e7dd0a01
Author: YehEmily 
Date:   2017-06-19T17:36:53Z

GEODE-2707: Removing TXLockToken (it doesn't appear to be used anywhere, 
although I still need to check for backwards compatibility issues)




---
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 60199: GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter

2017-06-19 Thread Ken Howe

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


Ship it!




Ship It!

- Ken Howe


On June 19, 2017, 4:09 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60199/
> ---
> 
> (Updated June 19, 2017, 4:09 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  842802ba7b7ed34aa52974dcf327015051f22e1b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  47d150d180dada8066f1e644b293c56096b2a969 
> 
> 
> Diff: https://reviews.apache.org/r/60199/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



[GitHub] geode pull request #582: GEODE-2601: Fix banner being logged twice

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

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


---
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 60199: GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter

2017-06-19 Thread Emily Yeh

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


Ship it!




Ship It!

- Emily Yeh


On June 19, 2017, 4:09 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60199/
> ---
> 
> (Updated June 19, 2017, 4:09 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  842802ba7b7ed34aa52974dcf327015051f22e1b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  47d150d180dada8066f1e644b293c56096b2a969 
> 
> 
> Diff: https://reviews.apache.org/r/60199/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Udo Kohlmeyer

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




geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java
Lines 77 (patched)


Could we rename this to PROTOBUF_CLIENT_SERVER_PROTOCOL?



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


pls rename to s = socket



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


please rename to useful field. sc = socketChannel



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


st = timerTask.. Pls rename



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


this conditional is becoming exponentially worse. Could we maybe extract 
this to a method/function?



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


This does not seem to be used anywhere anymore. Could you please check and 
remove



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


Doubt that this only needs to happen inside of this conditional. Maybe we 
extract this out into its own method and then call it outside of the method



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


should we have a similar setting for the CLIENT_TO_SERVER_NEW_PROTOCOL?



geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
Lines 32 (patched)


Maybe we rename this to GenericProtocolServerConnection



geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
Lines 35 (patched)


clientProtocolMessageHandler



geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
Lines 54 (patched)


could we rename this to clientProtocolHandler or 
clientProtocolMessageHandler instead of newClientProtocol



geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NewProtocolServerConnection.java
Lines 58 (patched)


surely we don't need this here... How do we handle the case if we have 
another client protocol handler... this would fail.


- Udo Kohlmeyer


On June 16, 2017, 5:14 p.m., Galen O'Sullivan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60157/
> ---
> 
> (Updated June 16, 2017, 5:14 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Create `ServerConnectionFactory`, which creates either instances of 
> `LegacyServerConnection` (identical in functionality to the old 
> `ServerConnection`) or `NewProtocolServerConnection` (which is currently 
> basically a stub, but will never get called unless a feature flag is set).
> 
> This is the initial work for GEODE-3074, and will allow us to continue to 
> work on further tasks for that project.
> 
> An explicit goal with this PR is to not disturb any existing functionality. 
> Unless a feature flag is set and a connection with a certain magic byte is 
> received, server connections will work as before. If you see something that 
> you think may break, please let me know.
> 
> Have a nice day!
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> 9a3241b05 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2a8818cef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e0b5ab8b6 
>   
> 

Re: Review Request 60199: GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter

2017-06-19 Thread Jared Stewart

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


Ship it!




Ship It!

- Jared Stewart


On June 19, 2017, 4:09 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60199/
> ---
> 
> (Updated June 19, 2017, 4:09 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  842802ba7b7ed34aa52974dcf327015051f22e1b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  47d150d180dada8066f1e644b293c56096b2a969 
> 
> 
> Diff: https://reviews.apache.org/r/60199/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Review Request 60199: GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter

2017-06-19 Thread Jinmei Liao

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

Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description
---

GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
 842802ba7b7ed34aa52974dcf327015051f22e1b 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
 47d150d180dada8066f1e644b293c56096b2a969 


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


Testing
---

precheckin running


Thanks,

Jinmei Liao



Re: Review Request 60106: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

2017-06-19 Thread Bruce Schuchardt

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

(Updated June 19, 2017, 4:09 p.m.)


Review request for geode and Hitesh Khamesra.


Changes
---

This revision fixes race conditions in encrypted UDP messaging that we started 
seeing with the previous revision.  The cluster secret key was not initialized 
by the View Creator thread fast enough, causing NPEs when the key was needed to 
send a JoinResponse in GMSJoinLeave.  We now initialize the cluster secret key 
in GMSJoinLeave.recordViewRequest.


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


Repository: geode


Description
---

There were four problems that new unit tests hit:
1. when recovering a view from disk we were treating it as a definitive (live) 
view.  I've moved it to a new variable in GMSLocator and set its viewId to -1.  
At the same time I set the initial GMSJoinLeave SearchState.viewId to -100 so 
it will be overridden by the one returned by the locator.  These changes allow 
GmsJoinLeave to know that the potential coordinator is from a recovered view.
2. when trying to join with a recovered view GMSJoinLeave.join() was giving up 
after the second ID in the view and becoming the coordinator.  It needs to keep 
trying until the list is exhausted, and it shouldn't sleep between attempts.
3. GMSLocator wasn't returning registrants for use in 
findCoordinatorFromView().  This was causing it to choose itself as the 
coordinator instead of using registrant sort order and choosing a different 
registrant as the coordinator.
4. During concurrent startup GMSLocator didn't know when the decision was made 
to become coordinator.  It is now notified of this decision and 
processRequest() uses this flag to have it override anything in the registrants 
set or in the recovered view.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
 26b03276b0abbf6210a5602a8c551abe38edc261 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java
 c6bef571134c6444a297cc8fe0bb0b7eb95f41f4 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java
 c5fdf45411581a36feca220e14a0551f3197d368 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
 edfaf625e6c652f46d9323c1116791f1c69fda59 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
 93fa9dab4ec2c8e43fc41cfd3b8ad986f96cf00f 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 8abcc456e42ad00a558a93f87bd3ae74ce88d146 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java
 c7b1a26b47cf2c913d9de30d6934ad5b3ac49840 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
 390824eb11a2e72a21d951539c2e03ed8025be82 
  geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
7ecca6146f6b7a542ae9864d7fabd48c9794ecac 
  
geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
 df1d8d1101a5f9d04c402922955a283353aa3b7c 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java
 19cee066a488198471ebf4093045853e36d5ba78 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java
 7f64c670400464aa8e6a73405516bd6e891a006b 
  
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java
 b35270e2d97930cee68d8c54221a04c20dfb96de 


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

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


Testing
---

New unit tests, regression testing (under way), precheckin (under way)


Thanks,

Bruce Schuchardt



Re: Review Request 60142: GEODE-3071: Provide capability to parallelize distributedTests

2017-06-19 Thread Jens Deppe

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

(Updated June 19, 2017, 3:19 p.m.)


Review request for geode, Anthony Baker, Mark Bretl, and Dan Smith.


Changes
---

Pull some of the config into the top-level build script


Repository: geode


Description
---

Herewith the ability to leverage Gradle's parallel test execution
capability to run dunits in parallel. This is combined with launching
tests in Docker containers to provide process, network and filesystem
isolation. Depending on the size of your system, this can speed up
running the distributedTest task 2-5 times.

The capability is enabled by launching gradle with '-PparallelDunit'

Tunables, enabled as gradle parametrs (-P option) are:

- dunitDockerImage: The docker image which will be used to launch
  tests. The image must have the JAVA_HOME environment variable set. The
  image must be pulled locally before starting the tests.
- dunitParallelForks: The number of parallel docker containers to be
  launched.
- dunitDockerUser: The docker user which will run the tests. Because of
  the way that the containers map the build directory into them, the
  test artifacts, will be written with this user id. By default this is
  'root'.

Remove debug println

NOTE: There are problems running this on MacOS which will require a bit more 
work.


Diffs (updated)
-

  build.gradle ec6b920c825491030120e99d288067d476d1fdcb 
  gradle.properties ca79a3816bffa56e412d82dd570f1e0e445592be 
  gradle/docker.gradle PRE-CREATION 


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

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


Testing
---

Manual test on linux


Thanks,

Jens Deppe