RE: Geode-Native Windows build

[Daniel Farcovich]

In VS2017 installer I had to install the following components:
.NetFramework targeting pack 4.5.2
VC++ 2015.3 v140 toolset
VC++ 2017 v141 toolset
Windows 10 SDK for Desktop c++ x86 and x64
JavaScript and TypeScript language support
TypeScript 2.2 SDK

Daniel

-Original Message-
From: Jacob Barrett [mailto:jbarr...@amduat.net] 
Sent: Wednesday, July 19, 2017 3:09 PM
To: dev@geode.apache.org
Subject: Re: Geode-Native Windows build

Great! Any hiccups with 2017 you can share? Did you install the VC140 toolkit 
too?

Sent from my iPhone

> On Jul 19, 2017, at 1:45 AM, Daniel Farcovich  
> wrote:
> 
> Hi Jacob,
> I can confirm that build passed on VS2017 using generator "Visual Studio 15 
> 2017 Win64".
> 
> Thanks,
> Daniel
> 
> -Original Message-
> From: Jacob Barrett [mailto:jbarr...@pivotal.io]
> Sent: Thursday, July 13, 2017 4:20 PM
> To: dev@geode.apache.org
> Subject: Re: Geode-Native Windows build
> 
> Yup! It should match the the minimum requirements mentioned at the very 
> least. It should also compile fine under 2017 though I don't believe anyone 
> has yet.
> 
> Sent from my iPhone
> 
>> On Jul 13, 2017, at 6:08 AM, Daniel Farcovich  
>> wrote:
>> 
>> From BUILDING.md:
>> 
>> Windows Generator
>> 
>> The recommended generator on Windows is Visual Studio 12 2013 Win64:
>> 
>> $ cmake -G "Visual Studio 12 2013 Win64" ../src
>> 
>> 
>> Should we change that?
>> 
>> Daniel
>> 
>> -Original Message-
>> From: Jacob Barrett [mailto:jbarr...@pivotal.io]
>> Sent: Thursday, July 13, 2017 3:06 PM
>> To: dev@geode.apache.org
>> Subject: Re: Geode-Native Windows build
>> 
>> VS 2013 is not fully C++11 compliant. The minimum compiler on windows is VS 
>> 2015.
>> 
>> Sent from my iPhone
>> 
>>> On Jul 13, 2017, at 4:32 AM, Daniel Farcovich  
>>> wrote:
>>> 
>>> As I mentioned in the first email I receive compilation errors, for example:
>>> 31>C:\Users\Fdaniel\Source\Repos\geode-native2\src\cppcache\include\geode/utils.hpp(57):
>>>  error C2144: syntax error : 'bool' should be preceded by ';' 
>>> (C:\Users\Fdaniel\Source\Repos\geode-native2\src\cppcache\src\FunctionServiceImpl.
>>> 
>>> The root cause of this error is in the use of "constexpr" in utils.hpp and 
>>> build in VS2013.
>>> There are features of c++11 which are not implemented in various VS 
>>> versions.
>>> Here is a table with the unsupported features, where constexpr is one of 
>>> them:
>>> https://msdn.microsoft.com/en-us/library/hh567368.aspx#featurelist
>>> 
>>> I assume the rest of the errors are of the same reason.
>>> 
>>> Although boost has already macros to workaround constexpr 
>>> (BOOST_CONSTEXPR), my suggestion is to add a macro to geode_base.hpp for 
>>> removing constexpr when it's not supported:
>>> #ifdef _WIN32
>>> #if _MSC_VER > 1800 //VS2015 and higher #define GF_CONSTEXPR 
>>> constexpr #else #define GF_CONSTEXPR #endif #else #define 
>>> GF_CONSTEXPR constexpr #endif
>>> 
>>> Daniel
>>> 
>>> -Original Message-
>>> From: Daniel Farcovich
>>> Sent: Thursday, July 13, 2017 10:07 AM
>>> To: 'dev@geode.apache.org' 
>>> Subject: RE: Geode-Native Windows build
>>> 
>>> Hi Jake and Ernest,
>>> 
>>> I tried to build from c:\build
>>> Here is the config command and output:
>>> 
>>> c:\build>cmake  -DCMAKE_INSTALL_PREFIX=C:\build\out -G "Visual 
>>> Studio
>>> 12 2013 Win64" -Thost=x64
>>> C:\Users\Fdaniel\Source\Repos\geode-native2\src
>>> -- The C compiler identification is MSVC 18.0.21005.1
>>> -- The CXX compiler identification is MSVC 18.0.21005.1
>>> -- Check for working C compiler: C:/Program Files (x86)/Microsoft 
>>> Visual Studio 12.0/VC/bin/amd64/cl.exe
>>> -- Check for working C compiler: C:/Program Files (x86)/Microsoft 
>>> Visual Studio 12.0/VC/bin/amd64/cl.exe -- works
>>> -- Detecting C compiler ABI info
>>> -- Detecting C compiler ABI info - done
>>> -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft 
>>> Visual Studio 12.0/VC/bin/amd64/cl.exe
>>> -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft 
>>> Visual Studio 12.0/VC/bin/amd64/cl.exe -- works
>>> -- Detecting CXX compiler ABI info
>>> -- Detecting CXX compiler ABI info - done
>>> -- Detecting CXX compile features
>>> -- Detecting CXX compile features - done
>>> -- Performing Test CFLAGS_M64_ALLOWED
>>> -- Performing Test CFLAGS_M64_ALLOWED - Failed
>>> -- Performing Test CXXFLAGS_M64_ALLOWED
>>> -- Performing Test CXXFLAGS_M64_ALLOWED - Failed
>>> -- Performing Test CFLAGS_mt_ALLOWED
>>> -- Performing Test CFLAGS_mt_ALLOWED - Failed
>>> -- Found Java: C:/Program Files/Java/jdk1.8.0_74/bin/java.exe (found 
>>> suitable version "1.8.0.74", minimum required is "1.8.0.60") found
>>> components:  Development
>>> -- Found Geode: 
>>> C:\Users\Fdaniel\Source\Repos\geode\geode-assembly\build\install\apa
>>> c h e-geode\bin\gfsh.bat (found suitable version "1.1.1", minimum 
>>> required is "1.0")
>>> -- Found Doxygen: C:/Program Files/doxygen/bin/doxygen.exe (found 
>>> suitable version "1.8.13", minimum

RE: need information about SerialGatewaySenderQueue/ParallelGatewaySenderQueue Clear

[Dinesh Akhand]

Thanks Dan,

I have updated the Clear  method for parallel  queue.

@Override
public void clearQueue() {
this.sender.pause();
try{
this.sender.getLifeCycleLock().readLock().lock();
for (PartitionedRegion prQ : 
this.userRegionNameToshadowPRMap.values()) {
clearPartitionedRegion((PartitionedRegion) prQ);
}
this.sender.getLifeCycleLock().readLock().unlock();
}finally{
this.sender.resume();   
}
}

Thanks,
Dinesh Akhand 
   

-Original Message-
From: Dan Smith [mailto:dsm...@pivotal.io] 
Sent: Tuesday, July 18, 2017 3:59 AM
To: dev@geode.apache.org
Subject: Re: need information about 
SerialGatewaySenderQueue/ParallelGatewaySenderQueue Clear

Hi Dinesh,

I think we probably just never got around to adding a clear. I think you could 
probably clear your queues just stop stopping and starting the gateway sender, 
which might be the easiest thing to do here.

Regarding your code, for your parallel queue are you doing that inside of a 
function? The code you have will try to clear things on a single node. The 
queue also maintains some other metadata in memory. I'm not quite sure what the 
effect on the queue will be if you delete the region entries without changing 
that other metadata. I guess you could test it and find out.
You'll probably want to see what the effect is while the queue is actually 
dispatching entries as well, because it's possible you could catch the system 
in a state where it is trying to read entries from the region as you are 
deleting them. Or maybe pause the queue first in your clear method?

-Dan

On Fri, Jul 14, 2017 at 2:23 AM, Dinesh Akhand  wrote:

> Hi Team,
>
>
>
> Please reply . why we don't have implementation of clear method in 
> ParallelGatewaySenderQueue/ SerialGatewaySenderQueue in geode. Requirement:
> we want to clear the queue data.
>
>
>
> I have implement below method in our code.
>
> --
>
> Class ParallelGatewaySenderQueue.java
>
>
>
> //clear the partition region
>
> private void clearPartitionedRegion(PartitionedRegion 
> partitionedRegion)
>
> {
>
> LocalDataSet lds = (LocalDataSet) 
> PartitionRegionHelper.getLocalPrimaryData(partitionedRegion);
>
> Setset = lds.getBucketSet(); // this 
> returns bucket ids in the function context
>
>
>
> for (Integer bucketId : set) {
>
> Bucket bucket = partitionedRegion.
> getRegionAdvisor().getBucket(bucketId);
>
> if (bucket instanceof 
> ProxyBucketRegion == false) {
>
> if (bucket 
> instanceof BucketRegion) {
>
>
>   BucketRegion bucketRegion = (BucketRegion) bucket;
>
>
>   Set keySet = bucketRegion.keySet();
>
>
>   for (Iterator iterator = keySet.iterator(); iterator.hasNext();) 
> {
>
>
>   Object key = iterator.next();
>
>
>   bucketRegion.remove(key);
>
>
>   }
>
> }
>
> }
>
> }
>
> }
>
> -
>
> Class : SerialGatewaySenderQueue.java
>
>  @Override
>
>   public void clearQueue() {
>
>
>
> this.sender.getLifeCycleLock().readLock().lock();
>
> Set keys = this.region.keys();
>
> for (Long key : keys) {
>
>   this.region.remove(key);
>
> }
>
> this.sender.getLifeCycleLock().readLock().unlock();
>
>
>
>   }
>
> -
>
>
>
> Any comment in above code will welcome.
>
>
>
>
>
> Thanks,
>
> Dinesh Akhand
>
>
>
> -Original Message-
> From: Dinesh Akhand
> Sent: Monday, May 15, 2017 2:39 PM
> To: dev@geode.apache.org
> Subject: need information about RegionQueue
>
>
>
>
>
> Hi Team,
>
>
>
> Why we do't have support to clear complete queue.  Is there any 
> limitation for it?.
>
>
>
> public void clear(PartitionedRegion pr, int bucketId) {
>
>throw new RuntimeException("This method(clear)is not supported 
> by ParallelGatewaySenderQueue");
>
>   }
>
>
>
> Class : ParallelGatewaySenderQueue
>
> Class : SerialGatewaySenderQueue
>
>
>
> Thanks,
>
> Dinesh Akhand
>
>
>
> This message and the information contained herein is proprietary and 
> confidential and subject to the Amdocs policy statement,
>
>
>
> you may review at https://www.amdocs.com/about/email-disclaimer < 
> https://www.amdocs.com/about/email-disclaimer>
> This message and the information contained herein is proprietary and 
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer < 
> https://www.amdocs.com/about/email-disclaimer>
>
This message and

Re: Review Request 60985: GEODE-3217: Reimplement gfsh query as a single-step command

[Ken Howe]

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




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


Extra blank line. Check this in all of the new class files.



geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
Lines 24 (patched)


Seems there's opportunity for more tests in here, for instance, 
queryWithInvalidRegionName, queryInvalidExceptionThrown, etc.

Have you checked coverage, in particular for the new classes QueryCommand, 
and QueryInterceptor


- Ken Howe


On July 19, 2017, 9:06 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60985/
> ---
> 
> (Updated July 19, 2017, 9:06 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3217: Reimplement gfsh query as a single-step command
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  3f4397b3d025ee5453a6df338eed41589ff339c0 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommand.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryInterceptor.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
>  fe88fc98d397fa748f8f54b900b1511eb114c0b6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  41cc171dbabee2cf51a9e28d051b8365a3c44699 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  8ab7c9342ebd5370deba45a35dbf68201f2b7333 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
>  fb6318449e027c94ba4fd2fa87c84f8181ce61a4 
>   
> geode-core/src/test/java/org/apache/geode/management/DataCommandMBeanTest.java
>  e5d6ce8d5bfb35eb503c66c9650fde2a33a51315 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60985/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin passed (except for some flaky failures that appear to be unrelated)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

[GitHub] geode pull request #598: GEODE-2818: Completing implementation/testing of al...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128388403
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java
 ---
@@ -35,10 +37,9 @@
* @param errorMessage - description of the error
* @return An error response containing the above parameters
*/
-  public static ClientProtocol.Response createErrorResponse(String 
errorMessage) {
-ClientProtocol.ErrorResponse error =
-
ClientProtocol.ErrorResponse.newBuilder().setMessage(errorMessage).build();
-return 
ClientProtocol.Response.newBuilder().setErrorResponse(error).build();
+  public static Failure createFailureResult(String errorMessage) {
--- End diff --

I disagree with this method description or its implementation. Either the 
method name should state `createFailureResultWithErrorResponseForErrorMessage` 
OR the signature becomes `createFailureResult(ErrorResponse errorResponse)`.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128387458
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.operations;
+
+import org.apache.geode.protocol.protobuf.ClientProtocol;
+
+import java.util.function.Function;
+
+public class OperationContext {
--- End diff --

Would it make sense to maybe package restrict this class


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128386591
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandlerJUnitTest.java
 ---
@@ -64,11 +72,10 @@ public void setUp() throws Exception {
   public void test_puttingTheEncodedEntryIntoRegion() throws 
UnsupportedEncodingTypeException,
   CodecNotRegisteredForTypeException, 
CodecAlreadyRegisteredForTypeException {
 PutRequestOperationHandler operationHandler = new 
PutRequestOperationHandler();
-ClientProtocol.Response response =
+Result result =
 operationHandler.process(serializationServiceStub, 
generateTestRequest(), cacheStub);
 
-
Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.PUTRESPONSE,
-response.getResponseAPICase());
+Assert.assertNotNull(result);
--- End diff --

We should assert that we got back a Success Object rather than just result 
!= null


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128386349
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandlerJUnitTest.java
 ---
@@ -124,20 +128,19 @@ public void test_RegionThrowsClasscastException() 
throws CodecAlreadyRegisteredF
 when(regionMock.put(any(), any())).thenThrow(ClassCastException.class);
 
 PutRequestOperationHandler operationHandler = new 
PutRequestOperationHandler();
-ClientProtocol.Response response =
+Result result =
 operationHandler.process(serializationServiceStub, 
generateTestRequest(), cacheStub);
 
-
Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
-response.getResponseAPICase());
+Assert.assertTrue(result instanceof Failure);
--- End diff --

Do we need to assert at the error message


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128386432
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandlerJUnitTest.java
 ---
@@ -99,23 +105,21 @@ public void test_codecNotRegistered() throws 
CodecAlreadyRegisteredForTypeExcept
 TEST_KEY.getBytes(Charset.forName("UTF-8".thenThrow(exception);
 PutRequestOperationHandler operationHandler = new 
PutRequestOperationHandler();
 
-ClientProtocol.Response response =
+Result result =
 operationHandler.process(serializationServiceStub, 
generateTestRequest(), cacheStub);
 
-
Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
-response.getResponseAPICase());
+Assert.assertTrue(result instanceof Failure);
   }
 
   @Test
   public void test_RegionNotFound() throws 
CodecAlreadyRegisteredForTypeException,
   UnsupportedEncodingTypeException, CodecNotRegisteredForTypeException 
{
 when(cacheStub.getRegion(TEST_REGION)).thenReturn(null);
 PutRequestOperationHandler operationHandler = new 
PutRequestOperationHandler();
-ClientProtocol.Response response =
+Result result =
 operationHandler.process(serializationServiceStub, 
generateTestRequest(), cacheStub);
 
-
Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
-response.getResponseAPICase());
+Assert.assertTrue(result instanceof Failure);
--- End diff --

We need to assert at least the error message. Maybe later on the error code


---
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 #623: GEODE-3156: add AcceptanceTest gradle target and junit cat...

[kirklund]

Github user kirklund commented on the issue:

https://github.com/apache/geode/pull/623
  
Pulling this PR in now.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128567505
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java
 ---
@@ -67,30 +68,27 @@
*{@link ProtobufUtilities}
* @return A response indicating the passed value was found for a 
incoming GetRequest
*/
-  public static ClientProtocol.Response 
createGetResponse(BasicTypes.EncodedValue resultValue) {
-RegionAPI.GetResponse getResponse =
-RegionAPI.GetResponse.newBuilder().setResult(resultValue).build();
-return 
ClientProtocol.Response.newBuilder().setGetResponse(getResponse).build();
+  public static Success createGetResult(
+  BasicTypes.EncodedValue resultValue) {
+return new 
Success<>(RegionAPI.GetResponse.newBuilder().setResult(resultValue).build());
--- End diff --

could we replace these types of calls with inline `Success.of(EncodedValue)`


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

Geode PR pile up

[Udo Kohlmeyer]

Hi there fellow Geode devs.

I've just been cleaning up some PR's, and I'm seeing that we 33 odd PR's 
open, 20 of them older than 30 days.


Not beat on the pedantic drum, but it would be really nice to have the 
PR's either applied, addressed or closed(rejected).


Does anyone have any ideas how we can keep our PR backlog small and 
manageable vs a backlog that keeps on growing.


--Udo

[GitHub] geode pull request #623: GEODE-3156: add AcceptanceTest gradle target and ju...

[kirklund]

Github user kirklund closed the pull request at:

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


---
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 #450: GEODE-2632: create ClientCachePutBench

[kirklund]

Github user kirklund commented on the issue:

https://github.com/apache/geode/pull/450
  
I'll wrap up this PR soon.


---
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 #641: GetServersResponse number of servers was redundant.

[asfgit]

Github user asfgit closed the pull request at:

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


---
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: Proposal: Lucene indexing/searching for nested objects

[Dan Smith]

This proposal doesn't really talk about XML or gfsh support.

The XML should probably just be a nested xml element, like this. It should
have the same support for declarables that other callbacks in the xml do.


   com.mycompany.MySerializer 


The gfsh command to create an index should also accept a serializer, like
this

create lucene index --serializer=com.mycompany.MySerializer

If there are no objections I'll update the proposal.

-Dan

On Tue, Jul 18, 2017 at 10:38 AM, Dan Smith  wrote:

> I think this LuceneSerializer API needs a slight tweak. In order to
> implement the proposed FlatFormatSerializer, the serializer needs access to
> the index configuration to see what fields the user wants to index. We
> should also pass the LuceneIndex to the serializer.
>
> public interface LuceneSerializer {
>   Collection toDocuments(Object value, *LuceneIndex index*);
> }
>
> On Thu, Jul 13, 2017 at 2:19 PM, Dan Smith  wrote:
>
>> On Thu, Jul 13, 2017 at 11:26 AM, Jacob Barrett 
>> wrote:
>>
>>> Collections are really tough in Lucene because you have to flatten the
>>> document. I struggled against it for some time on a project a few years ago
>>> and ultimately decided to index the relationships separately and then merge
>>> the results.
>>>
>>
>> Yeah, this is part of the motivation for providing the LuceneSerializer
>> API. We can provide a built in serializer that just flattens all nested
>> collections into a single field, but users could also write their own
>> implementation that converts the nested objects into separate lucene
>> documents and use some of query classes in org.apache.lucene.search.join if
>> they really need to.
>>
>> It's not part of the goal here, but I think this LuceneSerializer API
>> could also make it easier to do spatial indexing, because users could
>> create a serializer that converts their gemfire object into a Lucene
>> document with GeoPointFields.
>>
>> -Dan
>>
>>
>

Review Request 61003: GEODE-393: GetRegionFunction uses the cache in the FunctionContext

[Jinmei Liao]

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

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


Repository: geode


Description
---

GEODE-393: GetRegionFunction uses the cache in the FunctionContext


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java
 d52492466d521fb25f410e7c03b3d9808aabe67c 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java
 PRE-CREATION 


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


Testing
---

added test and precheckin funning


Thanks,

Jinmei Liao

[GitHub] geode pull request #609: GEODE-2886 : sent IllegalStateException instead of ...

[jhuynh1]

Github user jhuynh1 commented on a diff in the pull request:

https://github.com/apache/geode/pull/609#discussion_r128586252
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
 ---
@@ -290,6 +295,24 @@ public void queryJsonObject() throws Exception {
   }
 
   @Test()
+  public void testWaitUntilFlushedForException() throws Exception {
+Map fields = new HashMap();
+fields.put("name", null);
+fields.put("lastName", null);
+fields.put("address", null);
+
luceneService.createIndexFactory().setFields(fields).create(INDEX_NAME, 
REGION_NAME);
+Region region = 
cache.createRegionFactory(RegionShortcut.PARTITION).create(REGION_NAME);
+final LuceneIndex index = luceneService.getIndex(INDEX_NAME, 
REGION_NAME);
+
+// This is to send IllegalStateException from WaitUntilFlushedFunction
+String nonCreatedIndex = "index2";
+
+boolean b =
+luceneService.waitUntilFlushed(nonCreatedIndex, REGION_NAME, 
6, TimeUnit.MILLISECONDS);
+assertFalse(b);
--- End diff --

We should check to make sure the exception is thrown instead of wait for 
flush returning false.  


---
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 #609: GEODE-2886 : sent IllegalStateException instead of ...

[jhuynh1]

Github user jhuynh1 commented on a diff in the pull request:

https://github.com/apache/geode/pull/609#discussion_r128584345
  
--- Diff: 
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java
 ---
@@ -85,8 +72,8 @@ public void execute(FunctionContext context) {
   }
 
 } else {
-  throw new IllegalArgumentException(
-  "The AEQ does not exist for the index " + indexName + " region " 
+ region.getFullPath());
+  resultSender.sendException(new IllegalStateException(
--- End diff --

I think we should just throw the IllegalStateException, that way it will 
propagate to the user and let them know that an AEQ does not exist for the 
index.


---
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 #609: GEODE-2886 : sent IllegalStateException instead of ...

[jhuynh1]

Github user jhuynh1 commented on a diff in the pull request:

https://github.com/apache/geode/pull/609#discussion_r128584677
  
--- Diff: 
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
 ---
@@ -472,12 +478,20 @@ public boolean waitUntilFlushed(String indexName, 
String regionPath, long timeou
 new WaitUntilFlushedFunctionContext(indexName, timeout, unit);
 Execution execution = FunctionService.onRegion(dataRegion);
 ResultCollector rs = 
execution.setArguments(context).execute(WaitUntilFlushedFunction.ID);
-List results = (List) rs.getResult();
-for (Boolean oneResult : results) {
-  if (oneResult == false) {
+List results = (List) rs.getResult();
+if (results != null) {
+  if (results.get(0) instanceof IllegalStateException) {
--- End diff --

Instead of checking types and silently handling this exception, it's 
probably better to just throw it and not have to modify this portion of code.


---
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 #609: GEODE-2886 : sent IllegalStateException instead of ...

[jhuynh1]

Github user jhuynh1 commented on a diff in the pull request:

https://github.com/apache/geode/pull/609#discussion_r128585366
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
 ---
@@ -290,6 +295,24 @@ public void queryJsonObject() throws Exception {
   }
 
   @Test()
+  public void testWaitUntilFlushedForException() throws Exception {
--- End diff --

Perhaps rename this be a bit more descriptive so a test maintainer can know 
what the test was expected to do when pass/failing.

Something like: waitUntilFlushThrowsIllegalStateExceptionWhenAEQNotFound


---
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 60985: GEODE-3217: Reimplement gfsh query as a single-step command

[Jared Stewart]

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

(Updated July 20, 2017, 5:54 p.m.)


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


Changes
---

Add more tests for QueryCommand


Repository: geode


Description
---

GEODE-3217: Reimplement gfsh query as a single-step command


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
 3f4397b 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommand.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryInterceptor.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
 fe88fc9 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
 41cc171 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 8ab7c93 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
 fb63184 
  
geode-core/src/test/java/org/apache/geode/management/DataCommandMBeanTest.java 
e5d6ce8 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
 PRE-CREATION 


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

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


Testing
---

Precheckin passed (except for some flaky failures that appear to be unrelated)


Thanks,

Jared Stewart

Re: Proposal: Lucene indexing/searching for nested objects

[Jacob Barrett]

I really feel like an annotation would make the most sense. How likely is it 
that the object could not be annotated or the serializer for the object is not 
tightly coupled with the object? Having to map objects to serializers 
externally then becomes a lot more complicated to keep consistent.

Sent from my iPhone

> On Jul 20, 2017, at 10:38 AM, Dan Smith  wrote:
> 
> This proposal doesn't really talk about XML or gfsh support.
> 
> The XML should probably just be a nested xml element, like this. It should
> have the same support for declarables that other callbacks in the xml do.
> 
> 
>   com.mycompany.MySerializer 
> 
> 
> The gfsh command to create an index should also accept a serializer, like
> this
> 
> create lucene index --serializer=com.mycompany.MySerializer
> 
> If there are no objections I'll update the proposal.
> 
> -Dan
> 
>> On Tue, Jul 18, 2017 at 10:38 AM, Dan Smith  wrote:
>> 
>> I think this LuceneSerializer API needs a slight tweak. In order to
>> implement the proposed FlatFormatSerializer, the serializer needs access to
>> the index configuration to see what fields the user wants to index. We
>> should also pass the LuceneIndex to the serializer.
>> 
>> public interface LuceneSerializer {
>>  Collection toDocuments(Object value, *LuceneIndex index*);
>> }
>> 
>>> On Thu, Jul 13, 2017 at 2:19 PM, Dan Smith  wrote:
>>> 
>>> On Thu, Jul 13, 2017 at 11:26 AM, Jacob Barrett 
>>> wrote:
>>> 
 Collections are really tough in Lucene because you have to flatten the
 document. I struggled against it for some time on a project a few years ago
 and ultimately decided to index the relationships separately and then merge
 the results.
 
>>> 
>>> Yeah, this is part of the motivation for providing the LuceneSerializer
>>> API. We can provide a built in serializer that just flattens all nested
>>> collections into a single field, but users could also write their own
>>> implementation that converts the nested objects into separate lucene
>>> documents and use some of query classes in org.apache.lucene.search.join if
>>> they really need to.
>>> 
>>> It's not part of the goal here, but I think this LuceneSerializer API
>>> could also make it easier to do spatial indexing, because users could
>>> create a serializer that converts their gemfire object into a Lucene
>>> document with GeoPointFields.
>>> 
>>> -Dan
>>> 
>>> 
>> 

Re: Review Request 61003: GEODE-393: GetRegionFunction uses the cache in the FunctionContext

[Ken Howe]

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


Ship it!





geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java
Lines 17 (patched)


Blank line is not needed between license and package.


- Ken Howe


On July 20, 2017, 5:40 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61003/
> ---
> 
> (Updated July 20, 2017, 5:40 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-393: GetRegionFunction uses the cache in the FunctionContext
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java
>  d52492466d521fb25f410e7c03b3d9808aabe67c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61003/diff/1/
> 
> 
> Testing
> ---
> 
> added test and precheckin funning
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 60985: GEODE-3217: Reimplement gfsh query as a single-step command

[Jared Stewart]

> On July 20, 2017, 2:52 p.m., Ken Howe wrote:
> > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
> > Lines 24 (patched)
> > 
> >
> > Seems there's opportunity for more tests in here, for instance, 
> > queryWithInvalidRegionName, queryInvalidExceptionThrown, etc.
> > 
> > Have you checked coverage, in particular for the new classes 
> > QueryCommand, and QueryInterceptor

I've updated with a revision to add some tests for the sad paths as you 
suggested.  Coverage is as follows: QueryCommand (100% method, 89% line), 
QueryInterceptor (100% method, 92%line).  Most of the lines without coverage 
are things like an extra-cautious 
```
} catch (IOException e) { 
throw new RuntimeException(e);
}
```


- Jared


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


On July 20, 2017, 5:54 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60985/
> ---
> 
> (Updated July 20, 2017, 5:54 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3217: Reimplement gfsh query as a single-step command
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  3f4397b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommand.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryInterceptor.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
>  fe88fc9 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  41cc171 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  8ab7c93 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
>  fb63184 
>   
> geode-core/src/test/java/org/apache/geode/management/DataCommandMBeanTest.java
>  e5d6ce8 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60985/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passed (except for some flaky failures that appear to be unrelated)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

Re: Review Request 60985: GEODE-3217: Reimplement gfsh query as a single-step command

[Ken Howe]

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


Ship it!




Ship It!

- Ken Howe


On July 20, 2017, 5:54 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60985/
> ---
> 
> (Updated July 20, 2017, 5:54 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3217: Reimplement gfsh query as a single-step command
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  3f4397b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommand.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryInterceptor.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
>  fe88fc9 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  41cc171 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  8ab7c93 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
>  fb63184 
>   
> geode-core/src/test/java/org/apache/geode/management/DataCommandMBeanTest.java
>  e5d6ce8 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60985/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passed (except for some flaky failures that appear to be unrelated)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

Re: Proposal: Lucene indexing/searching for nested objects

[Dan Smith]

On Thu, Jul 20, 2017 at 10:57 AM, Jacob Barrett  wrote:

> I really feel like an annotation would make the most sense. How likely is
> it that the object could not be annotated or the serializer for the object
> is not tightly coupled with the object? Having to map objects to
> serializers externally then becomes a lot more complicated to keep
> consistent.
>

Well, with PDX serialization there may not even be a java class, or it may
not be present on the server. So annotations don't really cover all of the
use cases. With the proposed API, you could plug in an annotation based
serializer, if you wanted to.

-Dan

Avoid mutating standard Java System Properties (even in tests)

[Kirk Lund]

We found the cause of DiskStoreMXBeanSecurityJUnitTest failing in CI:
https://issues.apache.org/jira/browse/GEODE-3251.

I think we should avoid setting or clearing any standard Java System
properties during runtime (even in tests). These are the properties that
JVM automatically sets:
https://docs.oracle.com/javase/tutorial/essential/environment/sysprop.html.

The alternative to swizzling something like "user.dir" at runtime is to
always fork a JVM for any test that needs "user.dir" to point somewhere
special.

Hope this helps someone else to avoid a similar bug!

GEODE-3251 is caused by:

1) The MemberStarterRule is swizzling the user.dir System property. Jinmei
has a pending change to not change the user.dir. In general, we should
avoid changing that System property at runtime and only fork JVMs for tests
that need processes to run in a specific user.dir.

2) FastClasspathScanner throws NoSuchFileException because
System.getProperty("user.dir") returns null. It's null because
MemberStarterRule set it to null.

3) The call stack for ManagementAdapter#handleCacheCreation(InternalCache)
catches the NoSuchFileException and swallows it without logging it.. The
ManagementAgent then closes itself in the finally block of
#handleCacheCreation.

4) When MBeanServerConnectionRule tries to connect to JMX RMI, it fails
because JMX RMI was closed in #3 above. This throws IOException "Failed to
retrieve RMIServer stub".

Here's the Exception stack trace that the Management Agent code was
swallowing:

java.lang.RuntimeException: java.lang.RuntimeException:
java.nio.file.NoSuchFileException:
/var/folders/28/m__9dv1906n60kmz7t71wm68gn/T/junit7797861898628691451
at
io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(FastClasspathScanner.java:1115)
at
io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(FastClasspathScanner.java:1143)
at
io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(FastClasspathScanner.java:1166)
at
org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper.scanPackageForClassesImplementing(ClasspathScanLoadHelper.java:35)
at
org.apache.geode.management.internal.cli.CommandManager.loadGeodeCommands(CommandManager.java:223)
at
org.apache.geode.management.internal.cli.CommandManager.loadCommands(CommandManager.java:176)
at
org.apache.geode.management.internal.cli.CommandManager.(CommandManager.java:83)
at
org.apache.geode.management.internal.cli.remote.CommandProcessor.(CommandProcessor.java:61)
at
org.apache.geode.management.internal.cli.remote.MemberCommandService.(MemberCommandService.java:36)
at
org.apache.geode.management.cli.CommandService.createLocalCommandService(CommandService.java:128)
at
org.apache.geode.management.internal.beans.MemberMBeanBridge.(MemberMBeanBridge.java:348)
at
org.apache.geode.management.internal.beans.ManagementAdapter.handleCacheCreation(ManagementAdapter.java:142)
at
org.apache.geode.management.internal.beans.ManagementListener.handleEvent(ManagementListener.java:106)
at
org.apache.geode.distributed.internal.InternalDistributedSystem.notifyResourceEventListeners(InternalDistributedSystem.java:2198)
at
org.apache.geode.distributed.internal.InternalDistributedSystem.handleResourceEvent(InternalDistributedSystem.java:585)
at
org.apache.geode.internal.cache.GemFireCacheImpl.initialize(GemFireCacheImpl.java:1198)
at
org.apache.geode.internal.cache.GemFireCacheImpl.basicCreate(GemFireCacheImpl.java:763)
at
org.apache.geode.internal.cache.GemFireCacheImpl.create(GemFireCacheImpl.java:750)
at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:175)
at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:222)
at
org.apache.geode.test.dunit.rules.ServerStarterRule.startServer(ServerStarterRule.java:175)
at
org.apache.geode.test.dunit.rules.ServerStarterRule.before(ServerStarterRule.java:91)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:46)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at
com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
at
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.jun

Re: Avoid mutating standard Java System Properties (even in tests)

[Kirk Lund]

Oops. Correction on #3:

Looks like the caller
of ManagementAdapter#handleCacheCreation(InternalCache) DOES log any caught
exceptions. But because the test uses a log-file that lives in a JUnit
TemporaryFolder, we have no visibility of the logged exception after the
test completes. This is one time when turning off stdout when a log-file is
specified hurts debugging of a CI failure.


On Thu, Jul 20, 2017 at 11:33 AM, Kirk Lund  wrote:

> We found the cause of DiskStoreMXBeanSecurityJUnitTest failing in CI:
> https://issues.apache.org/jira/browse/GEODE-3251.
>
> I think we should avoid setting or clearing any standard Java System
> properties during runtime (even in tests). These are the properties that
> JVM automatically sets: https://docs.oracle.com/javase/tutorial/essential/
> environment/sysprop.html.
>
> The alternative to swizzling something like "user.dir" at runtime is to
> always fork a JVM for any test that needs "user.dir" to point somewhere
> special.
>
> Hope this helps someone else to avoid a similar bug!
>
> GEODE-3251 is caused by:
>
> 1) The MemberStarterRule is swizzling the user.dir System property. Jinmei
> has a pending change to not change the user.dir. In general, we should
> avoid changing that System property at runtime and only fork JVMs for tests
> that need processes to run in a specific user.dir.
>
> 2) FastClasspathScanner throws NoSuchFileException because
> System.getProperty("user.dir") returns null. It's null because
> MemberStarterRule set it to null.
>
> 3) The call stack for ManagementAdapter#handleCacheCreation(InternalCache)
> catches the NoSuchFileException and swallows it without logging it.. The
> ManagementAgent then closes itself in the finally block of
> #handleCacheCreation.
>
> 4) When MBeanServerConnectionRule tries to connect to JMX RMI, it fails
> because JMX RMI was closed in #3 above. This throws IOException "Failed to
> retrieve RMIServer stub".
>
> Here's the Exception stack trace that the Management Agent code was
> swallowing:
>
> java.lang.RuntimeException: java.lang.RuntimeException: 
> java.nio.file.NoSuchFileException:
> /var/folders/28/m__9dv1906n60kmz7t71wm68gn/T/junit7797861898628691451
> at io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(
> FastClasspathScanner.java:1115)
> at io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(
> FastClasspathScanner.java:1143)
> at io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(
> FastClasspathScanner.java:1166)
> at org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper.
> scanPackageForClassesImplementing(ClasspathScanLoadHelper.java:35)
> at org.apache.geode.management.internal.cli.CommandManager.
> loadGeodeCommands(CommandManager.java:223)
> at org.apache.geode.management.internal.cli.CommandManager.
> loadCommands(CommandManager.java:176)
> at org.apache.geode.management.internal.cli.CommandManager.<
> init>(CommandManager.java:83)
> at org.apache.geode.management.internal.cli.remote.
> CommandProcessor.(CommandProcessor.java:61)
> at org.apache.geode.management.internal.cli.remote.
> MemberCommandService.(MemberCommandService.java:36)
> at org.apache.geode.management.cli.CommandService.
> createLocalCommandService(CommandService.java:128)
> at org.apache.geode.management.internal.beans.MemberMBeanBridge.(
> MemberMBeanBridge.java:348)
> at org.apache.geode.management.internal.beans.ManagementAdapter.
> handleCacheCreation(ManagementAdapter.java:142)
> at org.apache.geode.management.internal.beans.ManagementListener.
> handleEvent(ManagementListener.java:106)
> at org.apache.geode.distributed.internal.InternalDistributedSystem.
> notifyResourceEventListeners(InternalDistributedSystem.java:2198)
> at org.apache.geode.distributed.internal.InternalDistributedSystem.
> handleResourceEvent(InternalDistributedSystem.java:585)
> at org.apache.geode.internal.cache.GemFireCacheImpl.
> initialize(GemFireCacheImpl.java:1198)
> at org.apache.geode.internal.cache.GemFireCacheImpl.
> basicCreate(GemFireCacheImpl.java:763)
> at org.apache.geode.internal.cache.GemFireCacheImpl.create(
> GemFireCacheImpl.java:750)
> at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:175)
> at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:222)
> at org.apache.geode.test.dunit.rules.ServerStarterRule.
> startServer(ServerStarterRule.java:175)
> at org.apache.geode.test.dunit.rules.ServerStarterRule.
> before(ServerStarterRule.java:91)
> at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:46)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
> at org.junit.runners.BlockJUnit4ClassRunner.runChild(
> BlockJUnit4ClassRunner.java:78)
> at org.junit.runners.BlockJUnit4ClassRunner.runChild(
> BlockJUnit4ClassRunner.java:57)
> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)

Re: Avoid mutating standard Java System Properties (even in tests)

[Jared Stewart]

Indeed, this is frowned upon by the JDK authors: 
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4045688 


In the long term, we need to change our code to no longer assume that 
“user.dir” is the working directory of a given server/locator.  That current 
assumption is what necessitates setting “user.dir” for certain tests.  

In the interim, I added some time ago code (see MemberVM#stopMember) that will 
automatically bounce any DUnit VM’s started by LocatorServerStarterRule (the 
distributed version of MemberStarterRule) that have swizzled “user.dir”.

For non-dunit tests that change “user.dir”, I think the correct mitigation is 
to include a RestoreSystemProperties rule.

I completely agree that we should only dynamically change “user.dir” as an 
option of last resort.  

- Jared

> On Jul 20, 2017, at 11:33 AM, Kirk Lund  wrote:
> 
> We found the cause of DiskStoreMXBeanSecurityJUnitTest failing in CI:
> https://issues.apache.org/jira/browse/GEODE-3251.
> 
> I think we should avoid setting or clearing any standard Java System
> properties during runtime (even in tests). These are the properties that
> JVM automatically sets:
> https://docs.oracle.com/javase/tutorial/essential/environment/sysprop.html.
> 
> The alternative to swizzling something like "user.dir" at runtime is to
> always fork a JVM for any test that needs "user.dir" to point somewhere
> special.
> 
> Hope this helps someone else to avoid a similar bug!
> 
> GEODE-3251 is caused by:
> 
> 1) The MemberStarterRule is swizzling the user.dir System property. Jinmei
> has a pending change to not change the user.dir. In general, we should
> avoid changing that System property at runtime and only fork JVMs for tests
> that need processes to run in a specific user.dir.
> 
> 2) FastClasspathScanner throws NoSuchFileException because
> System.getProperty("user.dir") returns null. It's null because
> MemberStarterRule set it to null.
> 
> 3) The call stack for ManagementAdapter#handleCacheCreation(InternalCache)
> catches the NoSuchFileException and swallows it without logging it.. The
> ManagementAgent then closes itself in the finally block of
> #handleCacheCreation.
> 
> 4) When MBeanServerConnectionRule tries to connect to JMX RMI, it fails
> because JMX RMI was closed in #3 above. This throws IOException "Failed to
> retrieve RMIServer stub".
> 
> Here's the Exception stack trace that the Management Agent code was
> swallowing:
> 
> java.lang.RuntimeException: java.lang.RuntimeException:
> java.nio.file.NoSuchFileException:
> /var/folders/28/m__9dv1906n60kmz7t71wm68gn/T/junit7797861898628691451
> at
> io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(FastClasspathScanner.java:1115)
> at
> io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(FastClasspathScanner.java:1143)
> at
> io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(FastClasspathScanner.java:1166)
> at
> org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper.scanPackageForClassesImplementing(ClasspathScanLoadHelper.java:35)
> at
> org.apache.geode.management.internal.cli.CommandManager.loadGeodeCommands(CommandManager.java:223)
> at
> org.apache.geode.management.internal.cli.CommandManager.loadCommands(CommandManager.java:176)
> at
> org.apache.geode.management.internal.cli.CommandManager.(CommandManager.java:83)
> at
> org.apache.geode.management.internal.cli.remote.CommandProcessor.(CommandProcessor.java:61)
> at
> org.apache.geode.management.internal.cli.remote.MemberCommandService.(MemberCommandService.java:36)
> at
> org.apache.geode.management.cli.CommandService.createLocalCommandService(CommandService.java:128)
> at
> org.apache.geode.management.internal.beans.MemberMBeanBridge.(MemberMBeanBridge.java:348)
> at
> org.apache.geode.management.internal.beans.ManagementAdapter.handleCacheCreation(ManagementAdapter.java:142)
> at
> org.apache.geode.management.internal.beans.ManagementListener.handleEvent(ManagementListener.java:106)
> at
> org.apache.geode.distributed.internal.InternalDistributedSystem.notifyResourceEventListeners(InternalDistributedSystem.java:2198)
> at
> org.apache.geode.distributed.internal.InternalDistributedSystem.handleResourceEvent(InternalDistributedSystem.java:585)
> at
> org.apache.geode.internal.cache.GemFireCacheImpl.initialize(GemFireCacheImpl.java:1198)
> at
> org.apache.geode.internal.cache.GemFireCacheImpl.basicCreate(GemFireCacheImpl.java:763)
> at
> org.apache.geode.internal.cache.GemFireCacheImpl.create(GemFireCacheImpl.java:750)
> at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:175)
> at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:222)
> at
> org.apache.geode.test.dunit.rules.ServerStarterRule.startServer(ServerStarterRule.java:175)
> at
> org.apache.geode.test.dunit.rules.ServerStarterRule.before(ServerStarterRule.java:91)
> at org.junit.rules.ExternalResource$1.evaluate(Externa

[GitHub] geode pull request #646: GEODE-3213: Refactor ProtoBuf handler flow

[WireBaron]

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128600182
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
 ---
@@ -15,32 +15,35 @@
 package org.apache.geode.protocol.protobuf;
 
 import org.apache.geode.cache.Cache;
-import org.apache.geode.protocol.exception.InvalidProtocolMessageException;
-import org.apache.geode.protocol.operations.OperationHandler;
-import 
org.apache.geode.protocol.operations.registry.OperationsHandlerRegistry;
-import 
org.apache.geode.protocol.operations.registry.exception.OperationHandlerNotRegisteredException;
+import org.apache.geode.protocol.operations.OperationContext;
+import org.apache.geode.protocol.operations.Result;
+import 
org.apache.geode.protocol.operations.registry.OperationContextRegistry;
 import org.apache.geode.serialization.SerializationService;
 
 /**
  * This handles protobuf requests by determining the operation type of the 
request and dispatching
  * it to the appropriate handler.
  */
 public class ProtobufOpsProcessor {
-  private final OperationsHandlerRegistry opsHandlerRegistry;
+
+  private final OperationContextRegistry operationContextRegistry;
   private final SerializationService serializationService;
 
-  public ProtobufOpsProcessor(OperationsHandlerRegistry opsHandlerRegistry,
-  SerializationService serializationService) {
-this.opsHandlerRegistry = opsHandlerRegistry;
+  public ProtobufOpsProcessor(SerializationService serializationService,
+  OperationContextRegistry operationsContextRegistry) {
 this.serializationService = serializationService;
+this.operationContextRegistry = operationsContextRegistry;
   }
 
-  public ClientProtocol.Response process(ClientProtocol.Request request, 
Cache cache)
-  throws OperationHandlerNotRegisteredException, 
InvalidProtocolMessageException {
+  public ClientProtocol.Response process(ClientProtocol.Request request, 
Cache cache) {
 ClientProtocol.Request.RequestAPICase requestType = 
request.getRequestAPICase();
-OperationHandler opsHandler =
-
opsHandlerRegistry.getOperationHandlerForOperationId(requestType.getNumber());
+OperationContext operationContext = 
operationContextRegistry.getOperationContext(requestType);
--- End diff --

How are we dealing with missing registry entries now that we're no longer 
throwing OperationNotRegisteredException?


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[WireBaron]

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128597628
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/Failure.java 
---
@@ -0,0 +1,47 @@
+/*
+ * 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.protocol.operations;
--- End diff --

In terms of package layout, I had thought the intent was to have generic 
code that could be used for any protocol in this package, while protobuf 
specific code would be under org.apache.geode.protocol.protobuf.*  All of the 
Result and OperationContext code is now protobuf specific.  Should it be moved?


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[WireBaron]

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128591891
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/Result.java 
---
@@ -0,0 +1,28 @@
+/*
+ * 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.protocol.operations;
+
+import org.apache.geode.protocol.protobuf.ClientProtocol;
+
+import java.util.function.Function;
+
+public interface Result {
--- End diff --

This is tightly bound to the ClientProtocol.ErrorResponse, can we give it a 
less generic name?  Maybe ProtocolHandlerResponse?


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128618059
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
 ---
@@ -15,32 +15,35 @@
 package org.apache.geode.protocol.protobuf;
 
 import org.apache.geode.cache.Cache;
-import org.apache.geode.protocol.exception.InvalidProtocolMessageException;
-import org.apache.geode.protocol.operations.OperationHandler;
-import 
org.apache.geode.protocol.operations.registry.OperationsHandlerRegistry;
-import 
org.apache.geode.protocol.operations.registry.exception.OperationHandlerNotRegisteredException;
+import org.apache.geode.protocol.operations.OperationContext;
+import org.apache.geode.protocol.operations.Result;
+import 
org.apache.geode.protocol.operations.registry.OperationContextRegistry;
 import org.apache.geode.serialization.SerializationService;
 
 /**
  * This handles protobuf requests by determining the operation type of the 
request and dispatching
  * it to the appropriate handler.
  */
 public class ProtobufOpsProcessor {
-  private final OperationsHandlerRegistry opsHandlerRegistry;
+
+  private final OperationContextRegistry operationContextRegistry;
   private final SerializationService serializationService;
 
-  public ProtobufOpsProcessor(OperationsHandlerRegistry opsHandlerRegistry,
-  SerializationService serializationService) {
-this.opsHandlerRegistry = opsHandlerRegistry;
+  public ProtobufOpsProcessor(SerializationService serializationService,
+  OperationContextRegistry operationsContextRegistry) {
 this.serializationService = serializationService;
+this.operationContextRegistry = operationsContextRegistry;
   }
 
-  public ClientProtocol.Response process(ClientProtocol.Request request, 
Cache cache)
-  throws OperationHandlerNotRegisteredException, 
InvalidProtocolMessageException {
+  public ClientProtocol.Response process(ClientProtocol.Request request, 
Cache cache) {
 ClientProtocol.Request.RequestAPICase requestType = 
request.getRequestAPICase();
-OperationHandler opsHandler =
-
opsHandlerRegistry.getOperationHandlerForOperationId(requestType.getNumber());
+OperationContext operationContext = 
operationContextRegistry.getOperationContext(requestType);
--- End diff --

Currently we are getting a NPE for those which is awful and already lead to 
wasted time debugging the non-obvious error. Let's go back to throwing 
something specific.


---
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 #644: GEODE-3208 Revise docs with outdated JAR references

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #590: GEODE-3090: Fixing gfsh help message (and a lot of ...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 60974: GEODE-3231: do not log to file by default when using the member starter rules

[Jinmei Liao]

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

(Updated July 20, 2017, 9:51 p.m.)


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


Repository: geode


Description (updated)
---

* do not log to file by default, only when withLogInFile is called
* do not create a workingDir and sets the user.dir by default, only when 
withWorkingDir is called
* For LocatorServerStarterRule, also do not set the workingDir if not asked to, 
and only bounce the VM if the VM is started with a workingDir


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
 045e13edc798f2bd3460dc17bbfeec28ac5022e4 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java
 4ca625a436ad9d181f1fecd11afc7a0cf1157aa6 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java
 a78b26fb560c17c96b794ecedc09e10c87766ef0 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
 33439e6063150b87938f21f13e647d3b0ecc4896 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsWithMemberGroupDUnitTest.java
 e5cca279d8a8082726fc67eab8a5cfd91ce90529 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java
 766f25ada3b446df71d840d70bb6fae797a85a00 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java
 d98641849eed458b4c830e1fa198f4d986b0c4ce 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
 510a4e7fef1aff5a1aa8f00f569cd6422b7816ab 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java
 a8b498ddbecb9d8733ed4f98c1c80cb18da5c37e 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/util/MergeLogsDUnitTest.java
 9166642044055071b2cced88edfe3ae660976ef7 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
 066f8828f64b7caa9c9748683f9214868105fed8 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
 e6b50d210dad844aca5e37fcb3cf58d7aace4b3e 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  
geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
 c4a25470bab75a2fdfcdb55126564d21ecd9403d 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
 b977b50e28b8d518fd3d9da6c03b55f8b7cbff34 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 62ae2e223f019c78000ff7492510c24aba9c930c 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java
 226fd6c24bb1ef818fd2de2bd010d713b59872e8 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java 
7e5ce1f1e41eff125c5d2e75f389f5ea8b1f90ed 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
 6ea2d0336f74956ff81efee94fc1ac2504383520 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/test/MemberStarterRuleTest.java
 7dada04cc63aaa2668e103524bd63d3c58310257 
  
geode-web/src/test/java/org/apache/geode/management/internal/security/LogNoPasswordTest.java
 3176f540e5605989a125d9e3b533759ce009ed42 


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

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


Testing
---


Thanks,

Jinmei Liao

[GitHub] geode pull request #649: GEODE-2997: Change new protocol GetAllResponse.

[galen-pivotal]

GitHub user galen-pivotal opened a pull request:

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

GEODE-2997: Change new protocol GetAllResponse.

@kohlmu-pivotal @hiteshk25 @WireBaron @pivotal-amurmann @bschuchardt 

I think it's better to return the errors and the successful keys so that
one failed lookup doesn't mess up the whole request.

Imagining this as a conversational PR to see what the response is.

Signed-off-by: Galen O'Sullivan 

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?

- [NO.] 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/galen-pivotal/geode changeGetAllAPI

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

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


commit 2b07dfc3cae729b8e23655fa11f3e8d485c520c4
Author: Alexander Murmann 
Date:   2017-07-20T21:44:16Z

GEODE-2997: Change new protocol GetAllResponse.

I think it's better to return the errors and the successful keys so that
one failed lookup doesn't mess up the whole request.

Imagining this as a conversational PR to see what the response is.

Signed-off-by: Galen O'Sullivan 




---
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: Proposal: Lucene indexing/searching for nested objects

[Jacob Barrett]

Good point! Sounds good then.

Sent from my iPhone

> On Jul 20, 2017, at 11:15 AM, Dan Smith  wrote:
> 
>> On Thu, Jul 20, 2017 at 10:57 AM, Jacob Barrett  wrote:
>> 
>> I really feel like an annotation would make the most sense. How likely is
>> it that the object could not be annotated or the serializer for the object
>> is not tightly coupled with the object? Having to map objects to
>> serializers externally then becomes a lot more complicated to keep
>> consistent.
>> 
> 
> Well, with PDX serialization there may not even be a java class, or it may
> not be present on the server. So annotations don't really cover all of the
> use cases. With the proposed API, you could plug in an annotation based
> serializer, if you wanted to.
> 
> -Dan

[GitHub] geode issue #649: GEODE-2997: Change new protocol GetAllResponse.

[galen-pivotal]

Github user galen-pivotal commented on the issue:

https://github.com/apache/geode/pull/649
  
 Some other thoughts from today:
* make GetAllResponse return a collection of Entries and a collection of 
errors.
* make Get and GetAll return the same type of response, as mentioned above. 
Get will always return a single key in either the success or error collections.
   - maybe looking in a collection is extra overhead for a simple Get 
request?
   - bigger errors (like malformed messages) will still return errors at 
the Response level rather than errors nested in the response message.

Thoughts?


---
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 > #622 was SUCCESSFUL (with 1984 tests). Change made by John Blum.

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #622 was successful.
---
Scheduled with changes by John Blum.
1986 tests in total.

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




--
Code Changes
--
John Blum (852ee0020f434ce74032b58c5b24111cc9b67b27):

>DATAGEODE-23 - Add Annotation to configure and enable Spring's Transaction 
>Management with Local, GemFire Cache Transactions.

John Blum (8520b1bd570935be18c04cbb9190fe7c1eb54a06):

>DATAGEODE-22 - Add Annotation to configure and enable the Spring Cache 
>Abstraction with the GemfireCacheManager.



--
This message is automatically generated by Atlassian Bamboo

[GitHub] geode pull request #646: GEODE-3213: Refactor ProtoBuf handler flow

[pivotal-amurmann]

Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128648851
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/operations/Failure.java 
---
@@ -0,0 +1,47 @@
+/*
+ * 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.protocol.operations;
--- End diff --

We wrote a test that enforces that every enum value has a handler. Of 
course that test won't pass until we have implemented all operations which 
isn't going to happen any time soon. So we removed it again. By the time all 
operations are implemented the test would basically enforce that we don't 
delete any of the operations and hook them up correctly. However, that's 
already enforced via the individual integration tests. So it's a very low value 
test. 


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128649824
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.protobuf;
+
+import org.apache.geode.protocol.operations.OperationHandler;
+
+import java.util.function.Function;
+
+public class OperationContext {
+  private OperationHandler 
operationHandler;
--- End diff --

We could even just make instances of some interface and not have references 
to these Functions at all.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128648216
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufOpsProcessor.java
 ---
@@ -15,32 +15,33 @@
 package org.apache.geode.protocol.protobuf;
 
 import org.apache.geode.cache.Cache;
-import org.apache.geode.protocol.exception.InvalidProtocolMessageException;
-import org.apache.geode.protocol.operations.OperationHandler;
-import 
org.apache.geode.protocol.operations.registry.OperationsHandlerRegistry;
-import 
org.apache.geode.protocol.operations.registry.exception.OperationHandlerNotRegisteredException;
+import 
org.apache.geode.protocol.protobuf.registry.OperationContextRegistry;
 import org.apache.geode.serialization.SerializationService;
 
 /**
  * This handles protobuf requests by determining the operation type of the 
request and dispatching
  * it to the appropriate handler.
  */
 public class ProtobufOpsProcessor {
-  private final OperationsHandlerRegistry opsHandlerRegistry;
+
+  private final OperationContextRegistry operationContextRegistry;
   private final SerializationService serializationService;
 
-  public ProtobufOpsProcessor(OperationsHandlerRegistry opsHandlerRegistry,
-  SerializationService serializationService) {
-this.opsHandlerRegistry = opsHandlerRegistry;
+  public ProtobufOpsProcessor(SerializationService serializationService,
+  OperationContextRegistry operationsContextRegistry) {
--- End diff --

seems like a nitpick, but why are we using both operation and operations in 
names here?


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128649165
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.protobuf;
+
+import org.apache.geode.protocol.operations.OperationHandler;
+
+import java.util.function.Function;
+
+public class OperationContext {
+  private OperationHandler 
operationHandler;
+  private Function fromRequest;
+  private Function 
toResponse;
+  private Function toErrorResponse;
+
+  public OperationContext(Function 
fromRequest,
--- End diff --

Can we use `OperationRequest` and `OperationResponse`?


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128649664
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.protobuf;
+
+import org.apache.geode.protocol.operations.OperationHandler;
+
+import java.util.function.Function;
+
+public class OperationContext {
+  private OperationHandler 
operationHandler;
--- End diff --

Or we could even write a `fromRequest` (etc. family of) method that calls 
`fromRequest.apply()` on the arguments. Or we could have the context just have 
a `process` method that does things in the right order. I'm wondering what's 
the best way to write this.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128649345
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.protobuf;
+
+import org.apache.geode.protocol.operations.OperationHandler;
+
+import java.util.function.Function;
+
+public class OperationContext {
+  private OperationHandler 
operationHandler;
--- End diff --

These could be public final and the getters removed. `getFromRequest` and 
`getToResponse` make it sound like the builder is doing something, when all 
it's doing is returning an imutable final field.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128647952
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.protobuf;
+
+import org.apache.geode.protocol.operations.OperationHandler;
+
+import java.util.function.Function;
+
+public class OperationContext {
+  private OperationHandler 
operationHandler;
--- End diff --

I think these fields should be final.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128649982
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/OperationContext.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * 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.protocol.protobuf;
+
+import org.apache.geode.protocol.operations.OperationHandler;
+
+import java.util.function.Function;
+
+public class OperationContext {
+  private OperationHandler 
operationHandler;
--- End diff --

I don't think there's a clear best way to do this, feel free to ignore this 
comment chain and carry on if you want.


---
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 #639: GEODE-3112: Fixing improper ordering of client time...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #649: GEODE-2997: Change new protocol GetAllResponse.

[WireBaron]

Github user WireBaron commented on a diff in the pull request:

https://github.com/apache/geode/pull/649#discussion_r128652774
  
--- Diff: geode-protobuf/src/main/proto/basicTypes.proto ---
@@ -62,4 +62,14 @@ message Region {
 
 message Server {
 string url = 1;
-}
\ No newline at end of file
+}
+
+message Error {
--- End diff --

These are the same fields as ErrorResponse and KeyedErrorResponse in the 
outstanding PutAll pull request, can you comment in that PR to change the names 
if you prefer these?


---
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-native issue #107: GEODE-3019: Refactor Struct class

[echobravopapa]

Github user echobravopapa commented on the issue:

https://github.com/apache/geode-native/pull/107
  
@dgkimura @pivotal-jbarrett I think this is all ready for final inspection


---
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-native pull request #107: GEODE-3019: Refactor Struct class

[dgkimura]

Github user dgkimura commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/107#discussion_r128661186
  
--- Diff: src/cppcache/test/StructSetTest.cpp ---
@@ -0,0 +1,91 @@
+/*
+ * 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.
+ */
+
+#include 
+#include 
+
+#include 
+
+using namespace apache::geode::client;
+
+TEST(StructSetTest, Basic) {
--- End diff --

Woot more unittests!


---
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-native pull request #107: GEODE-3019: Refactor Struct class

[dgkimura]

Github user dgkimura commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/107#discussion_r128660879
  
--- Diff: src/cppcache/src/StructSetImpl.cpp ---
@@ -66,25 +66,22 @@ SelectResultsIterator StructSetImpl::getIterator() {
   return SelectResultsIterator(m_structVector, shared_from_this());
 }
 
-int32_t StructSetImpl::getFieldIndex(const char* fieldname) {
-  std::map::iterator iter =
+const int32_t StructSetImpl::getFieldIndex(const std::string& fieldname) {
+  const auto& iter =
   m_fieldNameIndexMap.find(fieldname);
   if (iter != m_fieldNameIndexMap.end()) {
 return iter->second;
   } else {
-// std::string tmp = "fieldname ";
-// tmp += fieldname + " not found";
-throw IllegalArgumentException("fieldname not found");
+throw std::invalid_argument("fieldname not found");
   }
 }
 
-const char* StructSetImpl::getFieldName(int32_t index) {
-  for (std::map::iterator iter =
-   m_fieldNameIndexMap.begin();
-   iter != m_fieldNameIndexMap.end(); ++iter) {
-if (iter->second == index) return iter->first.c_str();
+const std::string& StructSetImpl::getFieldName(int32_t index) {
+  for (const auto& iter : m_fieldNameIndexMap) {
+if (iter.second == index) return (iter.first);
   }
-  return nullptr;
+
+  throw std::out_of_range("Struct: fieldName not found.");
--- End diff --

Would it add any value having the index in the error message?


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

Build failed in Jenkins: Geode-nightly #902

[Apache Jenkins Server]

See 


Changes:

[jiliao] GEODE-2920: handle non-existent role in ExampleSecurityManager

[jiliao] GEODE-2707: Removing TXLockToken class and entry in

[kmiller] GEODE-3272 Doc update: .gfsh.history goes into .geode dir

[lhughesgodfrey] GEODE-3113: Modify HARegionQueue test to use Awaitility vs. 
timeouts

--
[...truncated 180.46 KB...]
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJavaNote: Some input files use or override a 
deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-lucene:processTestResources
:geode-lucene:testClasses
:geode-lucene:checkMissedTests
:geode-lucene:spotlessJavaCheck
:geode-lucene:spotlessCheck
:geode-lucene:test
:geode-lucene:check
:geode-lucene:build
:geode-lucene:distributedTest
:geode-lucene:integrationTest
:geode-old-client-support:assemble
:geode-old-client-support:compileTestJavaNote: 

 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:geode-old-client-support:processTestResources UP-TO-DATE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest
:geode-old-client-support:integrationTest
:geode-old-versions:javadoc UP-TO-DATE
:geode-old-versions:javadocJar
:geode-old-versions:sourcesJar
:geode-old-versions:signArchives SKIPPED
:geode-old-versions:assemble
:geode-old-versions:compileTestJava UP-TO-DATE
:geode-old-versions:processTestResources UP-TO-DATE
:geode-old-versions:testClasses UP-TO-DATE
:geode-old-versions:checkMissedTests UP-TO-DATE
:geode-old-versions:spotlessJavaCheck
:geode-old-versions:spotlessCheck
:geode-old-versions:test UP-TO-DATE
:geode-old-versions:check
:geode-old-versions:build
:geode-old-versions:distributedTest UP-TO-DATE
:geode-old-versions:integrationTest UP-TO-DATE
:geode-protobuf:assemble
:geode-protobuf:extractIncludeTestProto
:geode-protobuf:extractTestProto UP-TO-DATE
:geode-protobuf:generateTestProto UP-TO-DATE
:geode-protobuf:compileTestJavaNote: Some input files use unchecked or unsafe 
operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-protobuf:processTestResources
:geode-protobuf:testClasses
:geode-protobuf:checkMissedTests
:geode-protobuf:spotlessJavaCheck
:geode-protobuf:spotlessCheck
:geode-protobuf:test
:geode-protobuf:check
:geode-protobuf:build
:geode-protobuf:distributedTest
:geode-protobuf:integrationTest
:geode-pulse:assemble
:geode-pulse:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: 

 uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-pulse:processTestResources
:geode-pulse:testClasses
:geode-pulse:checkMissedTests
:geode-pulse:spotlessJavaCheck
:geode-pulse:spotlessCheck
:geode-pulse:test
:geode-pulse:check
:geode-pulse:build
:geode-pulse:distributedTest
:geode-pulse:integrationTest
:geode-rebalancer:assemble
:geode-rebalancer:compileTestJava
:geode-rebalancer:processTestResources UP-TO-DATE
:geode-rebalancer:testClasses
:geode-rebalancer:checkMissedTests
:geode-rebalancer:spotlessJavaCheck
:geode-rebalancer:spotlessCheck
:geode-rebalancer:test
:geode-rebalancer:check
:geode-rebalancer:build
:geode-rebalancer:distributedTest
:geode-rebalancer:integrationTest
:geode-wan:assemble
:geode-wan:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-wan:processTestResources
:geode-wan:testClasses
:geode-wan:checkMissedTests
:geode-wan:spotlessJavaCheck
:geode-wan:spotlessCheck
:geode-wan:test
:geode-wan:check
:geode-wan:build
:geode-wan:distributedTest
:geode-wan:integrationTest
:geode-web:assemble
:geode-web:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: 

Re: Avoid mutating standard Java System Properties (even in tests)

[Galen O'Sullivan]

I have had a lot of tests fail because mcast-port is nonzero. So now I set
that system property in a lot of new tests. Maybe the tests that set it to
something other than the default should be passing a Properties object to
CacheFactory rather than messing with system properties.

I think the Cache configuration step should happen before the Cache starts.
This way we can start the Cache with a configuration object and keep *all*
XML parsing, system property reading, and the like out of core code. This
way we can get system properties out of our tests, and have more confidence
in our ability to test clean and reliable configurations.

Granted, I'm not messing with core configuration, so it looks like the
problems you're seeing may be of a bit of a different class from the ones
I'm seeing.

On Thu, Jul 20, 2017 at 11:45 AM, Jared Stewart  wrote:

> Indeed, this is frowned upon by the JDK authors: http://bugs.java.com/
> bugdatabase/view_bug.do?bug_id=4045688  bugdatabase/view_bug.do?bug_id=4045688>
>
> In the long term, we need to change our code to no longer assume that
> “user.dir” is the working directory of a given server/locator.  That
> current assumption is what necessitates setting “user.dir” for certain
> tests.
>
> In the interim, I added some time ago code (see MemberVM#stopMember) that
> will automatically bounce any DUnit VM’s started by
> LocatorServerStarterRule (the distributed version of MemberStarterRule)
> that have swizzled “user.dir”.
>
> For non-dunit tests that change “user.dir”, I think the correct mitigation
> is to include a RestoreSystemProperties rule.
>
> I completely agree that we should only dynamically change “user.dir” as an
> option of last resort.
>
> - Jared
>
> > On Jul 20, 2017, at 11:33 AM, Kirk Lund  wrote:
> >
> > We found the cause of DiskStoreMXBeanSecurityJUnitTest failing in CI:
> > https://issues.apache.org/jira/browse/GEODE-3251.
> >
> > I think we should avoid setting or clearing any standard Java System
> > properties during runtime (even in tests). These are the properties that
> > JVM automatically sets:
> > https://docs.oracle.com/javase/tutorial/essential/
> environment/sysprop.html.
> >
> > The alternative to swizzling something like "user.dir" at runtime is to
> > always fork a JVM for any test that needs "user.dir" to point somewhere
> > special.
> >
> > Hope this helps someone else to avoid a similar bug!
> >
> > GEODE-3251 is caused by:
> >
> > 1) The MemberStarterRule is swizzling the user.dir System property.
> Jinmei
> > has a pending change to not change the user.dir. In general, we should
> > avoid changing that System property at runtime and only fork JVMs for
> tests
> > that need processes to run in a specific user.dir.
> >
> > 2) FastClasspathScanner throws NoSuchFileException because
> > System.getProperty("user.dir") returns null. It's null because
> > MemberStarterRule set it to null.
> >
> > 3) The call stack for ManagementAdapter#handleCacheCreation(
> InternalCache)
> > catches the NoSuchFileException and swallows it without logging it.. The
> > ManagementAgent then closes itself in the finally block of
> > #handleCacheCreation.
> >
> > 4) When MBeanServerConnectionRule tries to connect to JMX RMI, it fails
> > because JMX RMI was closed in #3 above. This throws IOException "Failed
> to
> > retrieve RMIServer stub".
> >
> > Here's the Exception stack trace that the Management Agent code was
> > swallowing:
> >
> > java.lang.RuntimeException: java.lang.RuntimeException:
> > java.nio.file.NoSuchFileException:
> > /var/folders/28/m__9dv1906n60kmz7t71wm68gn/T/
> junit7797861898628691451
> > at
> > io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(
> FastClasspathScanner.java:1115)
> > at
> > io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(
> FastClasspathScanner.java:1143)
> > at
> > io.github.lukehutch.fastclasspathscanner.FastClasspathScanner.scan(
> FastClasspathScanner.java:1166)
> > at
> > org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper.
> scanPackageForClassesImplementing(ClasspathScanLoadHelper.java:35)
> > at
> > org.apache.geode.management.internal.cli.CommandManager.
> loadGeodeCommands(CommandManager.java:223)
> > at
> > org.apache.geode.management.internal.cli.CommandManager.
> loadCommands(CommandManager.java:176)
> > at
> > org.apache.geode.management.internal.cli.CommandManager.<
> init>(CommandManager.java:83)
> > at
> > org.apache.geode.management.internal.cli.remote.CommandProcessor.(
> CommandProcessor.java:61)
> > at
> > org.apache.geode.management.internal.cli.remote.
> MemberCommandService.(MemberCommandService.java:36)
> > at
> > org.apache.geode.management.cli.CommandService.
> createLocalCommandService(CommandService.java:128)
> > at
> > org.apache.geode.management.internal.beans.MemberMBeanBridge.(
> MemberMBeanBridge.java:348)
> > at
> > org.apache.geode.management.internal.beans.ManagementAdapter.
> handleCacheCreation(Managemen

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

[Apache Jenkins Server]

See 


Changes:

[jiliao] GEODE-2818: Making "groups" a valid option when starting locators or

[klund] GEODE-3156: add AcceptanceTest gradle target and junit category

[ukohlmeyer] GetServersResponse number of servers was redundant. This now 
closes #641

[jiliao] GEODE-3108: update lucene security permission to use "LUCENE" target

[kmiller] GEODE-3208 Revise docs with outdated JAR references

[jiliao] GEODE-3090: Fixing gfsh help message (and a lot of other typos)

[lhughesgodfrey] GEODE-3112: Fixing improper ordering of client timeout setting

--
[...truncated 98.27 KB...]
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spring-web/2.6.1/springfox-spring-web-2.6.1.pom
Download 
https://repo1.maven.org/maven2/com/fasterxml/classmate/1.3.1/classmate-1.3.1.pom
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-core/1.2.0.RELEASE/spring-plugin-core-1.2.0.RELEASE.pom
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin/1.2.0.RELEASE/spring-plugin-1.2.0.RELEASE.pom
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-metadata/1.2.0.RELEASE/spring-plugin-metadata-1.2.0.RELEASE.pom
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct/1.0.0.Final/mapstruct-1.0.0.Final.pom
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct-parent/1.0.0.Final/mapstruct-parent-1.0.0.Final.pom
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-core/2.6.1/springfox-core-2.6.1.pom
Download 
https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-scala_2.10/2.8.6/jackson-module-scala_2.10-2.8.6.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger2/2.6.1/springfox-swagger2-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger-ui/2.6.1/springfox-swagger-ui-2.6.1.jar
Download 
https://repo1.maven.org/maven2/org/springframework/hateoas/spring-hateoas/0.23.0.RELEASE/spring-hateoas-0.23.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-paranamer/2.8.6/jackson-module-paranamer-2.8.6.jar
Download 
https://repo1.maven.org/maven2/io/swagger/swagger-annotations/1.5.10/swagger-annotations-1.5.10.jar
Download 
https://repo1.maven.org/maven2/io/swagger/swagger-models/1.5.10/swagger-models-1.5.10.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spi/2.6.1/springfox-spi-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-schema/2.6.1/springfox-schema-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger-common/2.6.1/springfox-swagger-common-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spring-web/2.6.1/springfox-spring-web-2.6.1.jar
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-core/1.2.0.RELEASE/spring-plugin-core-1.2.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-metadata/1.2.0.RELEASE/spring-plugin-metadata-1.2.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct/1.0.0.Final/mapstruct-1.0.0.Final.jar
Download 
https://repo1.maven.org/maven2/com/thoughtworks/paranamer/paranamer/2.8/paranamer-2.8.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-core/2.6.1/springfox-core-2.6.1.jar
Note: 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-protobuf:jar
:geode-protobuf:javadoc
:geode-protobuf:javadocJar
:geode-protobuf:sourcesJar
:geode-protobuf: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-w

[GitHub] geode pull request #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128677601
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java
 ---
@@ -50,47 +38,14 @@
* @param ex - exception which should be logged
* @return An error response containing the first three parameters.
*/
-  public static ClientProtocol.Response createAndLogErrorResponse(String 
errorMessage,
+  public static ClientProtocol.ErrorResponse 
createAndLogFailureResponse(String errorMessage,
--- End diff --

why'd the name change to `FailureResponse` when the type changed to 
`ErrorResponse`?


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128678905
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandlerJUnitTest.java
 ---
@@ -144,21 +138,22 @@ public void 
processReturnsErrorWhenUnableToDecodeRequest()
 when(serializationServiceStub.decode(BasicTypes.EncodingType.STRING,
 TEST_KEY.getBytes(Charset.forName("UTF-8".thenThrow(exception);
 
-ClientProtocol.Request getRequest = generateTestRequest(false, false, 
false);
-ClientProtocol.Response response = (ClientProtocol.Response) 
operationHandler
--- End diff --

👍  getting rid of typecasts.


---
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 #646: GEODE-3213: Refactor ProtoBuf handler flow

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/646#discussion_r128678978
  
--- Diff: 
geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/PutRequestOperationHandlerJUnitTest.java
 ---
@@ -124,20 +128,19 @@ public void test_RegionThrowsClasscastException() 
throws CodecAlreadyRegisteredF
 when(regionMock.put(any(), any())).thenThrow(ClassCastException.class);
 
 PutRequestOperationHandler operationHandler = new 
PutRequestOperationHandler();
-ClientProtocol.Response response =
+Result result =
 operationHandler.process(serializationServiceStub, 
generateTestRequest(), cacheStub);
 
-
Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
-response.getResponseAPICase());
+Assert.assertTrue(result instanceof Failure);
--- End diff --

no. Is it worth asserting part of it?


---
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 #515: GEODE-240: Remove deprecated methods on TransactionEvent

[shankarh]

Github user shankarh commented on the issue:

https://github.com/apache/geode/pull/515
  
Can this PR be merged? 


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