Re: Review Request 61143: GEODE-3310 Set target node in TXStateProxy during TXFailover if necessary

[Darrel Schneider]

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


Ship it!




Ship It!

- Darrel Schneider


On July 26, 2017, 10:37 a.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61143/
> ---
> 
> (Updated July 26, 2017, 10:37 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
> and Nick Reich.
> 
> 
> Bugs: GEODE-3310
> https://issues.apache.org/jira/browse/GEODE-3310
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Set target node in TXStateProxy during TXFailover if necessary
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommand.java
>  6bd00c0 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/MyTransactionFunction.java
>  0970836 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
>  7a56644 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommandTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61143/diff/1/
> 
> 
> Testing
> ---
> 
> Ongoing precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

Review Request 61166: GEODE-3313: Test utility supports building jar files with multiple classes

[Jared Stewart]

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

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


Repository: geode


Description
---

GEODE-3313: Test utility supports building jar files with multiple classes


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractor.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractorTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/CompiledSourceCode.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilder.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilderTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompiler.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompilerTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/UncompiledSourceCode.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractExtendsFunctionAdapter.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractFunction.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ConcreteExtendsAbstractExtendsFunctionAdapter.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsAbstractFunction.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsFunctionAdapter.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ImplementsFunction.java
 PRE-CREATION 


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


Testing
---

Precheckin running


Thanks,

Jared Stewart

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

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #628 was successful.
---
Scheduled
1989 tests in total.

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





--
This message is automatically generated by Atlassian Bamboo

[GitHub] geode pull request #647: GEODE-3271: Refactor WanCommands

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[kirklund]

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

https://github.com/apache/geode/pull/652#discussion_r129713502
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExitCodeStatusCommandsDUnitTest.java
 ---
@@ -0,0 +1,396 @@
+/*
+ * 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.management.internal.cli.shell;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.ExitCode;
+import org.apache.geode.internal.process.PidFile;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
+import org.apache.geode.test.dunit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.dunit.rules.gfsh.GfshRule;
+import org.apache.geode.test.dunit.rules.gfsh.GfshScript;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+
+// Originally created in response to GEODE-2971
+
+@Category(AcceptanceTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class GfshExitCodeStatusCommandsDUnitTest {
+  private static File toolsJar;
+  private final static ThreePhraseGenerator nameGenerator = new 
ThreePhraseGenerator();
+  private final static String memberControllerName = "member-controller";
+
+  @Rule
+  public GfshRule gfsh = new GfshRule();
+  private String locatorName;
+  private String serverName;
+
+  private int locatorPort;
+
+  // Some test configuration shorthands
+  private TestConfiguration LOCATOR_ONLINE_BUT_NOT_CONNECTED =
+  new TestConfiguration(true, false, false);
+  private TestConfiguration LOCATOR_ONLINE_AND_CONNECTED = new 
TestConfiguration(true, false, true);
+  private TestConfiguration BOTH_ONLINE_BUT_NOT_CONNECTED =
+  new TestConfiguration(true, true, false);
+  private TestConfiguration BOTH_ONLINE_AND_CONNECTED = new 
TestConfiguration(true, true, true);
+
+  @BeforeClass
+  public static void classSetup() {
+File javaHome = new File(System.getProperty("java.home"));
+String toolsPath =
+javaHome.getName().equalsIgnoreCase("jre") ? "../lib/tools.jar" : 
"lib/tools.jar";
+toolsJar = new File(javaHome, toolsPath);
+  }
+
+  @Before
+  public void setup() {
+locatorName = "locator-" + nameGenerator.generate('-');
+serverName = "server-" + nameGenerator.generate('-');
+locatorPort = 
AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);
+  }
+
+  @Test
+  @Parameters(
+  value = {"status locator --port=-10", "status locator --pid=-1", 
"status server --pid=-1"})
+  public void statusCommandWithInvalidOptionValueShouldFail(String cmd) {
+GfshScript.of(cmd).withName("test-frame").awaitAtMost(1, MINUTES)
+.expectExitCode(ExitCode.FATAL.getValue()).execute(gfsh);
+  }
+
+
+  @Test
+  @Parameters(value = {"status locator --host=somehostname", "status 
locator --port=10334",
+  "status locator --dir=.", "status server --dir=.", "status locator 
--name=some-locator-name",
+  "status server --name=some-server-name", "status locator --pid=-1", 
"status server --pid=-1"})
+  public void 
statusCommandWithValidOptionValueShouldFailWithNoMembers(String cmd) {
+GfshScript.of(cmd).withName("test-frame").awaitAtMost(1, MINUTES)
+.expectExitCode(ExitCode.FATAL.getValue()).execute(gfsh);
+  }
+
+
+  @Test
+  public void 

[GitHub] geode pull request #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[kirklund]

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

https://github.com/apache/geode/pull/652#discussion_r129713345
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExitCodeStatusCommandsDUnitTest.java
 ---
@@ -0,0 +1,396 @@
+/*
+ * 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.management.internal.cli.shell;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.ExitCode;
+import org.apache.geode.internal.process.PidFile;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
+import org.apache.geode.test.dunit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.dunit.rules.gfsh.GfshRule;
+import org.apache.geode.test.dunit.rules.gfsh.GfshScript;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+
+// Originally created in response to GEODE-2971
+
+@Category(AcceptanceTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class GfshExitCodeStatusCommandsDUnitTest {
+  private static File toolsJar;
+  private final static ThreePhraseGenerator nameGenerator = new 
ThreePhraseGenerator();
+  private final static String memberControllerName = "member-controller";
+
+  @Rule
+  public GfshRule gfsh = new GfshRule();
+  private String locatorName;
+  private String serverName;
+
+  private int locatorPort;
+
+  // Some test configuration shorthands
+  private TestConfiguration LOCATOR_ONLINE_BUT_NOT_CONNECTED =
+  new TestConfiguration(true, false, false);
+  private TestConfiguration LOCATOR_ONLINE_AND_CONNECTED = new 
TestConfiguration(true, false, true);
+  private TestConfiguration BOTH_ONLINE_BUT_NOT_CONNECTED =
+  new TestConfiguration(true, true, false);
+  private TestConfiguration BOTH_ONLINE_AND_CONNECTED = new 
TestConfiguration(true, true, true);
+
+  @BeforeClass
+  public static void classSetup() {
+File javaHome = new File(System.getProperty("java.home"));
+String toolsPath =
+javaHome.getName().equalsIgnoreCase("jre") ? "../lib/tools.jar" : 
"lib/tools.jar";
+toolsJar = new File(javaHome, toolsPath);
+  }
+
+  @Before
+  public void setup() {
+locatorName = "locator-" + nameGenerator.generate('-');
+serverName = "server-" + nameGenerator.generate('-');
+locatorPort = 
AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);
+  }
+
+  @Test
+  @Parameters(
+  value = {"status locator --port=-10", "status locator --pid=-1", 
"status server --pid=-1"})
+  public void statusCommandWithInvalidOptionValueShouldFail(String cmd) {
+GfshScript.of(cmd).withName("test-frame").awaitAtMost(1, MINUTES)
+.expectExitCode(ExitCode.FATAL.getValue()).execute(gfsh);
+  }
+
+
+  @Test
+  @Parameters(value = {"status locator --host=somehostname", "status 
locator --port=10334",
+  "status locator --dir=.", "status server --dir=.", "status locator 
--name=some-locator-name",
+  "status server --name=some-server-name", "status locator --pid=-1", 
"status server --pid=-1"})
+  public void 
statusCommandWithValidOptionValueShouldFailWithNoMembers(String cmd) {
+GfshScript.of(cmd).withName("test-frame").awaitAtMost(1, MINUTES)
+.expectExitCode(ExitCode.FATAL.getValue()).execute(gfsh);
+  }
+
+
+  @Test
+  public void 

[GitHub] geode pull request #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[kirklund]

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

https://github.com/apache/geode/pull/652#discussion_r129712861
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExitCodeStatusCommandsDUnitTest.java
 ---
@@ -0,0 +1,396 @@
+/*
+ * 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.management.internal.cli.shell;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.ExitCode;
+import org.apache.geode.internal.process.PidFile;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
+import org.apache.geode.test.dunit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.dunit.rules.gfsh.GfshRule;
+import org.apache.geode.test.dunit.rules.gfsh.GfshScript;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+
+// Originally created in response to GEODE-2971
+
+@Category(AcceptanceTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class GfshExitCodeStatusCommandsDUnitTest {
+  private static File toolsJar;
+  private final static ThreePhraseGenerator nameGenerator = new 
ThreePhraseGenerator();
--- End diff --

Correct keyword ordering (according to the Google style guide we're using) 
is "static 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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[kirklund]

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

https://github.com/apache/geode/pull/652#discussion_r129712972
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExitCodeStatusCommandsDUnitTest.java
 ---
@@ -0,0 +1,396 @@
+/*
+ * 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.management.internal.cli.shell;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.ExitCode;
+import org.apache.geode.internal.process.PidFile;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
+import org.apache.geode.test.dunit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.dunit.rules.gfsh.GfshRule;
+import org.apache.geode.test.dunit.rules.gfsh.GfshScript;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+
+// Originally created in response to GEODE-2971
+
+@Category(AcceptanceTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class GfshExitCodeStatusCommandsDUnitTest {
+  private static File toolsJar;
+  private final static ThreePhraseGenerator nameGenerator = new 
ThreePhraseGenerator();
+  private final static String memberControllerName = "member-controller";
+
+  @Rule
+  public GfshRule gfsh = new GfshRule();
+  private String locatorName;
+  private String serverName;
+
+  private int locatorPort;
+
+  // Some test configuration shorthands
+  private TestConfiguration LOCATOR_ONLINE_BUT_NOT_CONNECTED =
--- End diff --

These get re-instantiated between each test method. Was that the intention 
here? Or do you want these to be static 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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[kirklund]

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

https://github.com/apache/geode/pull/652#discussion_r129712729
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExitCodeStatusCommandsDUnitTest.java
 ---
@@ -0,0 +1,396 @@
+/*
+ * 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.management.internal.cli.shell;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.ExitCode;
+import org.apache.geode.internal.process.PidFile;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
+import org.apache.geode.test.dunit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.dunit.rules.gfsh.GfshRule;
+import org.apache.geode.test.dunit.rules.gfsh.GfshScript;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+
+// Originally created in response to GEODE-2971
+
+@Category(AcceptanceTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class GfshExitCodeStatusCommandsDUnitTest {
--- End diff --

I recommend removing "DUnit" from the name especially since it's not 
categorized as DistributedTest.


---
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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[kirklund]

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

https://github.com/apache/geode/pull/652#discussion_r129714136
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java
 ---
@@ -102,35 +107,52 @@ protected ProcessBuilder toProcessBuilder(GfshScript 
gfshScript, Path gfshPath,
   commandsToExecute.add("-e " + command);
 }
 
-return new ProcessBuilder(commandsToExecute).directory(workingDir);
+ProcessBuilder p = new ProcessBuilder(commandsToExecute);
+p.directory(workingDir);
+
+// TODO PSRhomberg -- Input requested: Is this OS agnostic
+List extendedClasspath = gfshScript.getExtendedClasspath();
+if (!extendedClasspath.isEmpty()) {
+  Map m = p.environment();
+  String classpathKey = "CLASSPATH";
+  String existingJavaArgs = m.get(classpathKey);
+  String specified = String.join(":", extendedClasspath);
--- End diff --

Linux and Mac use ":" as path separator. Use this instead:

org.apache.commons.lang.SystemUtils.PATH_SEPARATOR


---
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 #580: GEODE-2936: Refactoring OrderByComparator

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #653: GEODE-3290: Removed effectively-dead classes Filter...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #650: GEODE-3253: Refactoring ClientCommands and related ...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #580: GEODE-2936: Refactoring OrderByComparator

[YehEmily]

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

https://github.com/apache/geode/pull/580#discussion_r129695770
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
 ---
@@ -173,36 +157,54 @@ public void testUnsupportedOrderByForPR() throws 
Exception {
 
   @Test
   public void testSupportedOrderByForRR() throws Exception {
-
 String unsupportedQueries[] =
-{"select distinct p.status from /portfolio1 p order by p.status, 
p.ID",
-
-};
+{"select distinct p.status from /portfolio1 p order by p.status, 
p.ID"};
 Object r[][] = new Object[unsupportedQueries.length][2];
-QueryService qs;
-qs = CacheUtils.getQueryService();
 Position.resetCounter();
-// Create Regions
 
+// Create Regions
 Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
 
 for (int i = 0; i < 50; i++) {
   r1.put(new Portfolio(i), new Portfolio(i));
 }
 
 for (int i = 0; i < unsupportedQueries.length; i++) {
-  Query q = null;
-
+  Query q;
   CacheUtils.getLogger().info("Executing query: " + 
unsupportedQueries[i]);
   q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
   try {
 r[i][0] = q.execute();
-
   } catch (QueryInvalidException qe) {
 qe.printStackTrace();
 fail(qe.toString());
   }
 }
   }
 
+  // The following tests cover edge cases in OrderByComparator.
+  @Test
+  public void testCompareTwoNulls() throws Exception {
+assert (createComparator().compare(null, null) == 0);
--- End diff --

Thanks for catching this! I've fixed it and updated the PR.


---
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 #580: GEODE-2936: Refactoring OrderByComparator

[kirklund]

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

https://github.com/apache/geode/pull/580#discussion_r129693040
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
 ---
@@ -173,36 +157,54 @@ public void testUnsupportedOrderByForPR() throws 
Exception {
 
   @Test
   public void testSupportedOrderByForRR() throws Exception {
-
 String unsupportedQueries[] =
-{"select distinct p.status from /portfolio1 p order by p.status, 
p.ID",
-
-};
+{"select distinct p.status from /portfolio1 p order by p.status, 
p.ID"};
 Object r[][] = new Object[unsupportedQueries.length][2];
-QueryService qs;
-qs = CacheUtils.getQueryService();
 Position.resetCounter();
-// Create Regions
 
+// Create Regions
 Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
 
 for (int i = 0; i < 50; i++) {
   r1.put(new Portfolio(i), new Portfolio(i));
 }
 
 for (int i = 0; i < unsupportedQueries.length; i++) {
-  Query q = null;
-
+  Query q;
   CacheUtils.getLogger().info("Executing query: " + 
unsupportedQueries[i]);
   q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
   try {
 r[i][0] = q.execute();
-
   } catch (QueryInvalidException qe) {
 qe.printStackTrace();
 fail(qe.toString());
   }
 }
   }
 
+  // The following tests cover edge cases in OrderByComparator.
+  @Test
+  public void testCompareTwoNulls() throws Exception {
+assert (createComparator().compare(null, null) == 0);
--- End diff --

"assert" should be used in product code instead of tests. These are the 
optional assertions that are enabled when running a Java product with -ea 
(enable assertions).

You should use assertThat instead:
```java
assertThat(createComparator().compare(null, null).isEqualTo(0);
```


---
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 #580: GEODE-2936: Refactoring OrderByComparator

[kirklund]

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

https://github.com/apache/geode/pull/580#discussion_r129693128
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
 ---
@@ -173,36 +157,54 @@ public void testUnsupportedOrderByForPR() throws 
Exception {
 
   @Test
   public void testSupportedOrderByForRR() throws Exception {
-
 String unsupportedQueries[] =
-{"select distinct p.status from /portfolio1 p order by p.status, 
p.ID",
-
-};
+{"select distinct p.status from /portfolio1 p order by p.status, 
p.ID"};
 Object r[][] = new Object[unsupportedQueries.length][2];
-QueryService qs;
-qs = CacheUtils.getQueryService();
 Position.resetCounter();
-// Create Regions
 
+// Create Regions
 Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
 
 for (int i = 0; i < 50; i++) {
   r1.put(new Portfolio(i), new Portfolio(i));
 }
 
 for (int i = 0; i < unsupportedQueries.length; i++) {
-  Query q = null;
-
+  Query q;
   CacheUtils.getLogger().info("Executing query: " + 
unsupportedQueries[i]);
   q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
   try {
 r[i][0] = q.execute();
-
   } catch (QueryInvalidException qe) {
 qe.printStackTrace();
 fail(qe.toString());
   }
 }
   }
 
+  // The following tests cover edge cases in OrderByComparator.
+  @Test
+  public void testCompareTwoNulls() throws Exception {
+assert (createComparator().compare(null, null) == 0);
+  }
+
+  @Test
+  public void testCompareTwoObjectArrays() throws Exception {
+String[] arrString1 = {"elephant"};
+String[] arrString2 = {"elephants"};
+assert (createComparator().compare(arrString2, arrString1) == 1);
--- End diff --

Same as previous comment. Please use assertThat.


---
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 #113: (no ticket) Capitalize C# client member func...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode-native/pull/113


---
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 #113: (no ticket) Capitalize C# client member functions

[echobravopapa]

Github user echobravopapa commented on the issue:

https://github.com/apache/geode-native/pull/113
  
+1



---
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 #657: GEODE-3286: Failing to cleanup connections from Con...

[pivotal-amurmann]

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

https://github.com/apache/geode/pull/657#discussion_r129689404
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java ---
@@ -279,26 +280,29 @@ protected void acceptConnection(Socket sock) throws 
IOException, ConnectionExcep
   // in our caller.
   // no need to log error here since caller will log warning
 
-  if (conn != null && !finishedConnecting) {
+  if (connection != null && !finishedConnecting) {
 // we must be throwing from checkCancelInProgress so close the 
connection
-
closeCon(LocalizedStrings.ConnectionTable_CANCEL_AFTER_ACCEPT.toLocalizedString(),
 conn);
-conn = null;
+
closeCon(LocalizedStrings.ConnectionTable_CANCEL_AFTER_ACCEPT.toLocalizedString(),
+connection);
+connection = null;
   }
 }
 
-if (conn != null) {
+if (connection != null) {
   synchronized (this.receivers) {
-this.owner.stats.incReceivers();
+this.owner.getStats().incReceivers();
 if (this.closed) {
   
closeCon(LocalizedStrings.ConnectionTable_CONNECTION_TABLE_NO_LONGER_IN_USE
-  .toLocalizedString(), conn);
+  .toLocalizedString(), connection);
   return;
 }
-this.receivers.add(conn);
+if (!connection.isSocketClosed()) {
--- End diff --

Maybe once this gets squashed into a single commit the explanation can be 
added to the commit message to be preserved with the code? It's always awesome 
to do a `git annotate` and get a great explanation


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

[kirklund]

Github user kirklund closed the pull request at:

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


---
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 #657: GEODE-3286: Failing to cleanup connections from Con...

[WireBaron]

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

https://github.com/apache/geode/pull/657#discussion_r129683975
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java ---
@@ -1322,6 +1328,14 @@ private void createBatchSendBuffer() {
 this.batchFlusher.start();
   }
 
+  public void onIdleCancel() {
--- End diff --

We'll change this to cleanUpOnIdleTaskCancel.  The connection will always 
be closed when we reach this code.  We debated adding an assert here, but 
decided against 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 pull request #657: GEODE-3286: Failing to cleanup connections from Con...

[WireBaron]

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

https://github.com/apache/geode/pull/657#discussion_r129683563
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java ---
@@ -568,6 +568,12 @@ protected Connection(ConnectionTable t, Socket socket) 
throws IOException, Conne
 }
   }
 
+  protected void initRecevier() {
--- End diff --

fixed


---
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 61143: GEODE-3310 Set target node in TXStateProxy during TXFailover if necessary

[Nick Reich]

> On July 26, 2017, 8:11 p.m., Nick Reich wrote:
> >

All my comments are nits and only suggestions / thoughts and not barriers to 
accepting this review and shipping it.


- Nick


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


On July 26, 2017, 5:37 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61143/
> ---
> 
> (Updated July 26, 2017, 5:37 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
> and Nick Reich.
> 
> 
> Bugs: GEODE-3310
> https://issues.apache.org/jira/browse/GEODE-3310
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Set target node in TXStateProxy during TXFailover if necessary
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommand.java
>  6bd00c0 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/MyTransactionFunction.java
>  0970836 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
>  7a56644 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommandTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61143/diff/1/
> 
> 
> Testing
> ---
> 
> Ongoing precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

Re: Review Request 61143: GEODE-3310 Set target node in TXStateProxy during TXFailover if necessary

[Nick Reich]

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




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


This does not really get a TXId, it creates one. Maybe getNewTXId() or 
createTXId() would be better names?



geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Lines 178 (patched)


With "e" or "ex" being the most common names for Exceptions in java catch 
blocks and "exp" being an abbreviation for "expression", maybe Execution exec 
and Exception e would be better variable names?



geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Lines 180 (patched)


assertFalse("Expected exception was not thrown", isFirstFunc)?



geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Lines 184 (patched)


Possible alternative:
catch (TransactionException e) {
assertFalse("Unexpected exception was thrown", isFirstFunction)
assertTrue(e.getMessage().startsWith("Function execution is not colocated 
with transaction.")
}

Letting unexpected exceptions be thrown from the test will result in them 
being logged and simplify the logic of the test (and not require manually 
logging that information as well). You could arguably remove the check for the 
contents of the exception message, as that makes the test brittle to changes 
that do not effect behavior (i.e. error message text changes). If the type of 
exception thrown is not sufficient, does this suggest we need a new exception 
that inherits from TransactionException?



geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
Line 149 (original), 223 (patched)


Replacing all these anonymous classes with lambdas really helps clean up 
the code: I am glad we are doing more of this.



geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommandTest.java
Lines 64 (patched)


Ouch! That is a massive amount of mocking and even power mocking. I do not 
envy you in trying to cobble that together. Is this easier/possible to do in an 
integration test (dunit or otherwise)? Whenever I create tests, the more 
mocking (and any power mocking) I do, the more I wonder how much I am really 
testing the code and not my ability to string together the right mock call and 
response sequences, though if it is the only way to get it done, it is still 
better than nothing.


- Nick Reich


On July 26, 2017, 5:37 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61143/
> ---
> 
> (Updated July 26, 2017, 5:37 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
> and Nick Reich.
> 
> 
> Bugs: GEODE-3310
> https://issues.apache.org/jira/browse/GEODE-3310
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Set target node in TXStateProxy during TXFailover if necessary
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommand.java
>  6bd00c0 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/MyTransactionFunction.java
>  0970836 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
>  7a56644 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommandTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61143/diff/1/
> 
> 
> Testing
> ---
> 
> Ongoing precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

[GitHub] geode pull request #659: GEODE-3308: Lucene rolling upgrade and backwards co...

[nabarunnag]

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

https://github.com/apache/geode/pull/659#discussion_r129679647
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneSearchWithRollingUpgradeDUnit.java
 ---
@@ -0,0 +1,1044 @@
+/*
+ * 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.cache.lucene;
+
+import static org.apache.geode.test.dunit.Assert.fail;
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.GemFireCache;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.cache30.CacheSerializableRunnable;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.Version;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.dunit.DistributedTestUtils;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.Invoke;
+import org.apache.geode.test.dunit.NetworkUtils;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.standalone.DUnitLauncher;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+@Category({DistributedTest.class, BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)

+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class LuceneSearchWithRollingUpgradeDUnit extends 
JUnit4DistributedTestCase {
+
+
+  @Parameterized.Parameters
+  public static Collection data() {
+List result = 
VersionManager.getInstance().getVersionsWithoutCurrent();
+// Lucene Compatibility checks start with Apache Geode v1.2.0
+// Removing the versions older than v1.2.0
+result.removeIf(s -> Integer.parseInt(s) < 120);
+if (result.size() < 1) {
+  throw new RuntimeException("No older versions of Geode were found to 
test against");
+} else {
+  System.out.println("running against these versions: " + result);
+}
+return result;
+  }
+
+  private File[] testingDirs = new File[3];
+
+  private static String INDEX_NAME = "index";
+
+  private static String diskDir = "LuceneSearchWithRollingUpgradeDUnit";
+
+  // Each vm will have a cache object
+  private static Object cache;
+
+  // the old version of Geode we're testing against
+  private String oldVersion;
+
+  private void deleteVMFiles() throws Exception {
+System.out.println("deleting files in vm" + VM.getCurrentVMNum());
+File pwd = new 

[GitHub] geode pull request #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[jinmeiliao]

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

https://github.com/apache/geode/pull/652#discussion_r129673457
  
--- Diff: geode-core/src/main/java/org/apache/geode/internal/ExitCode.java 
---
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal;
+
+import java.util.Arrays;
+
+import org.springframework.shell.core.ExitShellRequest;
+
+
+public enum ExitCode {
+
+  // The choice of redundant values here is to be consistent with existing 
behavior
+  // while allowing for future extensibility. JVM_TERMINATED_EXIT(99) 
exists for coverage of
+  // Spring's ExitShellRequest values in fromSpring.
+  NORMAL(0),
+  FATAL(1),
+  DEPENDENCY_GRAPH_FAILURE(-1),
+  COULD_NOT_EXECUTE_COMMAND(1),
+  INVALID_COMMAND(1),
--- End diff --

It's the exit code that matters. As long as you are not changing the code 
that each command gives when it's calling system.exit(), you are not changing 
existing behavior. Or Am I missing anything 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 #659: GEODE-3308: Lucene rolling upgrade and backwards co...

[ladyVader]

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

https://github.com/apache/geode/pull/659#discussion_r129672503
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneSearchWithRollingUpgradeDUnit.java
 ---
@@ -0,0 +1,1044 @@
+/*
+ * 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.cache.lucene;
+
+import static org.apache.geode.test.dunit.Assert.fail;
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.GemFireCache;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.cache30.CacheSerializableRunnable;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.Version;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.dunit.DistributedTestUtils;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.Invoke;
+import org.apache.geode.test.dunit.NetworkUtils;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.standalone.DUnitLauncher;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+@Category({DistributedTest.class, BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)

+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class LuceneSearchWithRollingUpgradeDUnit extends 
JUnit4DistributedTestCase {
+
+
+  @Parameterized.Parameters
+  public static Collection data() {
+List result = 
VersionManager.getInstance().getVersionsWithoutCurrent();
+// Lucene Compatibility checks start with Apache Geode v1.2.0
+// Removing the versions older than v1.2.0
+result.removeIf(s -> Integer.parseInt(s) < 120);
+if (result.size() < 1) {
+  throw new RuntimeException("No older versions of Geode were found to 
test against");
+} else {
+  System.out.println("running against these versions: " + result);
+}
+return result;
+  }
+
+  private File[] testingDirs = new File[3];
+
+  private static String INDEX_NAME = "index";
+
+  private static String diskDir = "LuceneSearchWithRollingUpgradeDUnit";
+
+  // Each vm will have a cache object
+  private static Object cache;
+
+  // the old version of Geode we're testing against
+  private String oldVersion;
+
+  private void deleteVMFiles() throws Exception {
+System.out.println("deleting files in vm" + VM.getCurrentVMNum());
+File pwd = new 

[GitHub] geode issue #659: GEODE-3308: Lucene rolling upgrade and backwards compatibi...

[nabarunnag]

Github user nabarunnag commented on the issue:

https://github.com/apache/geode/pull/659
  
* These tests were ported from RollingUpgradeDUnitTest and 
RollingUpgrade2DUnitTest
* Instead of put and get verification, it is now lucent query result size 
verification

Potential Reviewers
@ladyVader @jhuynh1 @DivineEnder


---
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 #659: GEODE-3308: Lucene rolling upgrade and backwards co...

[nabarunnag]

GitHub user nabarunnag opened a pull request:

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

GEODE-3308: Lucene rolling upgrade and backwards compatibility tests …

…added
 

Thank you for submitting a contribution to Apache Geode.

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

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

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

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

- [ ] Does `gradlew build` run cleanly?

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

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

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


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

$ git pull https://github.com/nabarunnag/incubator-geode feature/GEODE-3308

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

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


commit c9d9e71e8d31feccfd1c7d0ba2ac83d7296fbcbf
Author: nabarun 
Date:   2017-07-18T23:09:28Z

GEODE-3308: Lucene rolling upgrade and backwards compatibility tests added




---
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 #652: Geode-2971: Introduce ExitCode to resolve inconsistency of...

[PurelyApplied]

Github user PurelyApplied commented on the issue:

https://github.com/apache/geode/pull/652
  
@jaredjstewart I don't know why I didn't think to put my questions here 
instead of imbedding them in my code, just to be removed later.  I clearly 
wasn't on my A-game.

1.  In my addition to `GfshRule` to extend the classpath, it was unclear to 
me whether using `ProcessBuilder`'s `environment` method was properly OS 
agnostic.  (Looking at it again, it also appears that I left variable names `p` 
and `m` in place instead of more meaningful names.  Ugh.)

2. In the `GfshExitCodeStatuscommandsDUnitTest`, I couldn't decide if the 
utility functions generating the actual `gfsh` command would look better 
inlined into the test function or as a function call.

3.  Akin to the "Should `gfsh help` actually be returning non-zero", 
existing behavior is to exit with a zero when gfsh is interrupted 
(`Launcher.java:227`).  Is an interruption assumed to be intentional and we 
should return a zero, or should that actually be returning a nonzero?


---
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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[PurelyApplied]

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

https://github.com/apache/geode/pull/652#discussion_r129656012
  
--- Diff: geode-core/src/main/java/org/apache/geode/internal/ExitCode.java 
---
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal;
+
+import java.util.Arrays;
+
+import org.springframework.shell.core.ExitShellRequest;
+
+
+public enum ExitCode {
+
+  // The choice of redundant values here is to be consistent with existing 
behavior
+  // while allowing for future extensibility. JVM_TERMINATED_EXIT(99) 
exists for coverage of
+  // Spring's ExitShellRequest values in fromSpring.
+  NORMAL(0),
+  FATAL(1),
+  DEPENDENCY_GRAPH_FAILURE(-1),
+  COULD_NOT_EXECUTE_COMMAND(1),
+  INVALID_COMMAND(1),
--- End diff --

I agree in principle, but was hesitant to alter existing behavior for 
system exit calls.  I didn't gain any traction in my message to the dev list 
about using any exit codes other than `0` and `1`, but at the same time felt 
that some distinction was warranted in the different places that `System.exit` 
had been called.

Then again, I just finished ripping out dead code that had been in place 
"for future development," so perhaps I should keep it simple for now and not do 
the work of future extensibility that may or may not ever arrive.


---
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 #643: GEODE-3192,GEODE-3229: Change API and implementatio...

[galen-pivotal]

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

https://github.com/apache/geode/pull/643#discussion_r129649690
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetRequestOperationHandler.java
 ---
@@ -53,10 +52,10 @@
   ProtobufUtilities.createEncodedValue(serializationService, 
resultValue);
   return 
Success.of(RegionAPI.GetResponse.newBuilder().setResult(encodedValue).build());
 } catch (UnsupportedEncodingTypeException ex) {
-  return Failure.of(
-  ClientProtocol.ErrorResponse.newBuilder().setMessage("Encoding 
not supported.").build());
+  return Failure
--- End diff --

We're not entirely consistent with when we log errors and when we don't. 
Probably these should be logged like the others?


---
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 #651: GEODE-3230: Cleaning up unused (Cli)Strings

[jaredjstewart]

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

https://github.com/apache/geode/pull/651#discussion_r129647633
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 ---
@@ -639,29 +639,34 @@ public Result compactOfflineDiskStore(
   String errorString = CliStrings.format(
   
CliStrings.COMPACT_OFFLINE_DISK_STORE__MSG__ERROR_WHILE_COMPACTING_DISKSTORE_0_WITH_1_REASON_2,
   diskStoreName, fieldsMessage);
-  result = ResultBuilder.createUserErrorResult(errorString);
+  result = ResultBuilder.createUserErrorResult(CliStrings.format(
--- End diff --

This looks to me like it's going to build a strange error message that I 
expect will look something like this:: 

```
An error occurred while doing compaction: "While compacting disk store={0} 
{1}. Reason: {2}"
``` 

I think {0} and {1} will be filled in but not {2}.


---
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 #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[jinmeiliao]

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

https://github.com/apache/geode/pull/652#discussion_r129647221
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExitCodeStatusCommandsDUnitTest.java
 ---
@@ -0,0 +1,396 @@
+/*
+ * 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.management.internal.cli.shell;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.ExitCode;
+import org.apache.geode.internal.process.PidFile;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.management.internal.cli.util.ThreePhraseGenerator;
+import org.apache.geode.test.dunit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.dunit.rules.gfsh.GfshRule;
+import org.apache.geode.test.dunit.rules.gfsh.GfshScript;
+import org.apache.geode.test.junit.categories.AcceptanceTest;
+
+// Originally created in response to GEODE-2971
+
+@Category(AcceptanceTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class GfshExitCodeStatusCommandsDUnitTest {
+  private static File toolsJar;
+  private final static ThreePhraseGenerator nameGenerator = new 
ThreePhraseGenerator();
+  private final static String memberControllerName = "member-controller";
+
+  @Rule
+  public GfshRule gfsh = new GfshRule();
+  private String locatorName;
+  private String serverName;
+
+  private int locatorPort;
+
+  // Some test configuration shorthands
+  private TestConfiguration LOCATOR_ONLINE_BUT_NOT_CONNECTED =
+  new TestConfiguration(true, false, false);
+  private TestConfiguration LOCATOR_ONLINE_AND_CONNECTED = new 
TestConfiguration(true, false, true);
+  private TestConfiguration BOTH_ONLINE_BUT_NOT_CONNECTED =
+  new TestConfiguration(true, true, false);
+  private TestConfiguration BOTH_ONLINE_AND_CONNECTED = new 
TestConfiguration(true, true, true);
+
+  @BeforeClass
+  public static void classSetup() {
+File javaHome = new File(System.getProperty("java.home"));
+String toolsPath =
+javaHome.getName().equalsIgnoreCase("jre") ? "../lib/tools.jar" : 
"lib/tools.jar";
+toolsJar = new File(javaHome, toolsPath);
+  }
+
+  @Before
+  public void setup() {
+locatorName = "locator-" + nameGenerator.generate('-');
+serverName = "server-" + nameGenerator.generate('-');
+locatorPort = 
AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);
+  }
+
+  @Test
+  @Parameters(
+  value = {"status locator --port=-10", "status locator --pid=-1", 
"status server --pid=-1"})
+  public void statusCommandWithInvalidOptionValueShouldFail(String cmd) {
+GfshScript.of(cmd).withName("test-frame").awaitAtMost(1, MINUTES)
+.expectExitCode(ExitCode.FATAL.getValue()).execute(gfsh);
+  }
+
+
+  @Test
+  @Parameters(value = {"status locator --host=somehostname", "status 
locator --port=10334",
+  "status locator --dir=.", "status server --dir=.", "status locator 
--name=some-locator-name",
+  "status server --name=some-server-name", "status locator --pid=-1", 
"status server --pid=-1"})
+  public void 
statusCommandWithValidOptionValueShouldFailWithNoMembers(String cmd) {
--- End diff --

this test failed for me when I run it locally:

[INFO  10:33:30.713 PDT] (test-frame): (1) Executing - status server --dir=.
[INFO  10:33:30.713 PDT] 

[GitHub] geode pull request #652: Geode-2971: Introduce ExitCode to resolve inconsist...

[jinmeiliao]

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

https://github.com/apache/geode/pull/652#discussion_r129646761
  
--- Diff: geode-core/src/main/java/org/apache/geode/internal/ExitCode.java 
---
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal;
+
+import java.util.Arrays;
+
+import org.springframework.shell.core.ExitShellRequest;
+
+
+public enum ExitCode {
+
+  // The choice of redundant values here is to be consistent with existing 
behavior
+  // while allowing for future extensibility. JVM_TERMINATED_EXIT(99) 
exists for coverage of
+  // Spring's ExitShellRequest values in fromSpring.
+  NORMAL(0),
+  FATAL(1),
+  DEPENDENCY_GRAPH_FAILURE(-1),
+  COULD_NOT_EXECUTE_COMMAND(1),
+  INVALID_COMMAND(1),
--- End diff --

I would think for each enum, the value passed to it would be distinct. 
What's the reason for multiple enum value have the same shellExitCode? Would 
this be more concise?

enum ExitCode{
  DEPENDENCY_GRAPH_FAILURE(-1),
  NORMAL(0),
  FATAL(1),
  INSTALL_FAILURE(2),
}

I am hesitant to put the exit code used only for test in here. But that's 
debatable.


---
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 #657: GEODE-3286: Failing to cleanup connections from Con...

[galen-pivotal]

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

https://github.com/apache/geode/pull/657#discussion_r129640056
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java ---
@@ -279,26 +280,29 @@ protected void acceptConnection(Socket sock) throws 
IOException, ConnectionExcep
   // in our caller.
   // no need to log error here since caller will log warning
 
-  if (conn != null && !finishedConnecting) {
+  if (connection != null && !finishedConnecting) {
 // we must be throwing from checkCancelInProgress so close the 
connection
-
closeCon(LocalizedStrings.ConnectionTable_CANCEL_AFTER_ACCEPT.toLocalizedString(),
 conn);
-conn = null;
+
closeCon(LocalizedStrings.ConnectionTable_CANCEL_AFTER_ACCEPT.toLocalizedString(),
+connection);
+connection = null;
   }
 }
 
-if (conn != null) {
+if (connection != null) {
   synchronized (this.receivers) {
-this.owner.stats.incReceivers();
+this.owner.getStats().incReceivers();
 if (this.closed) {
   
closeCon(LocalizedStrings.ConnectionTable_CONNECTION_TABLE_NO_LONGER_IN_USE
-  .toLocalizedString(), conn);
+  .toLocalizedString(), connection);
   return;
 }
-this.receivers.add(conn);
+if (!connection.isSocketClosed()) {
--- End diff --

It looks like this is the critical line.  What is it that makes the 
connections get removed from the table when they are unexpectedly closed? What 
happens if the connection is closed after being added to the table?


---
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 #657: GEODE-3286: Failing to cleanup connections from Con...

[galen-pivotal]

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

https://github.com/apache/geode/pull/657#discussion_r129640243
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java ---
@@ -568,6 +568,12 @@ protected Connection(ConnectionTable t, Socket socket) 
throws IOException, Conne
 }
   }
 
+  protected void initRecevier() {
--- End diff --

typo: recevier -> receiver.


---
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 #657: GEODE-3286: Failing to cleanup connections from Con...

[galen-pivotal]

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

https://github.com/apache/geode/pull/657#discussion_r129645767
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java ---
@@ -1322,6 +1328,14 @@ private void createBatchSendBuffer() {
 this.batchFlusher.start();
   }
 
+  public void onIdleCancel() {
--- End diff --

Should this be closing and cleaning up the connection as well?


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

Review Request 61143: GEODE-3310 Set target node in TXStateProxy during TXFailover if necessary

[Eric Shu]

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

Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
and Nick Reich.


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


Repository: geode


Description
---

Set target node in TXStateProxy during TXFailover if necessary


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommand.java
 6bd00c0 
  
geode-core/src/test/java/org/apache/geode/internal/cache/execute/MyTransactionFunction.java
 0970836 
  
geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
 7a56644 
  
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/TXFailoverCommandTest.java
 PRE-CREATION 


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


Testing
---

Ongoing precheckin.


Thanks,

Eric Shu

Re: JIRA Assignment Permissions Request

[Anthony Baker]

Welcome!  What is your JIRA username?

Anthony

> On Jul 26, 2017, at 3:43 AM, Ju@N  wrote:
> 
> Hello team,
> 
> Can I get permissions to assign Geode Tickets to myself in JIRA?.
> Cheers.
> 
> -- 
> Ju@N

JIRA Assignment Permissions Request

[Ju@N]

Hello team,

Can I get permissions to assign Geode Tickets to myself in JIRA?.
Cheers.

-- 
Ju@N

Build failed in Jenkins: Geode-nightly #907

[Apache Jenkins Server]

See 


Changes:

[bschuchardt] GEODE-3175 backward-compatibility tests fail with bad classpath

[bschuchardt] GEODE-3175 backward-compatibility tests fail with bad classpath

[Anil] GEODE-3115 Added changes to check for persistent region during pdx type

[jiliao] GEODE-3231: withLogFile does not imply withWorkingDir anymore

[jiliao] GEODE-3031: Extracting startLocator and startServer from

[jiliao] GEODE-3097: GFSH works over HTTP with SSL

[dschneider] GEODE-2986: Remove redundant log message

[dschneider] GEODE-240: Remove deprecated methods on TransactionEvent

--
[...truncated 132.95 KB...]
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
:geode-junit:testClasses
:geode-junit:acceptanceTest
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
: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: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:acceptanceTest
:geode-lucene:assemble
: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: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:acceptanceTest
:geode-old-client-support:assemble
: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:compileTestJava UP-TO-DATE
:geode-old-versions:processTestResources UP-TO-DATE
:geode-old-versions:testClasses UP-TO-DATE
:geode-old-versions:acceptanceTest UP-TO-DATE
: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: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: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:acceptanceTest
:geode-protobuf:assemble
: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: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:acceptanceTest
:geode-pulse:assemble
: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:compileTestJava
:geode-rebalancer:processTestResources UP-TO-DATE
:geode-rebalancer:testClasses
:geode-rebalancer:acceptanceTest
:geode-rebalancer:assemble
:geode-rebalancer:checkMissedTests
:geode-rebalancer:spotlessJavaCheck
:geode-rebalancer:spotlessCheck
:geode-rebalancer:test
:geode-rebalancer:check
:geode-rebalancer:build
:geode-rebalancer:distributedTest

[GitHub] geode pull request #634: Feature/geode 3175

[bschuchardt]

Github user bschuchardt closed the pull request at:

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


---
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 #656: GEODE-3312 Update doc gfsh list members output

[asfgit]

Github user asfgit closed the pull request at:

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


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

[galen-pivotal]

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

https://github.com/apache/geode/pull/649#discussion_r129626464
  
--- 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 {
+int32 errorCode = 1;
+string message = 2;
+}
+
+message ErrorEntry {
--- End diff --

That's fine too. I don't have a real strong preference.


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

[galen-pivotal]

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

https://github.com/apache/geode/pull/649#discussion_r129626228
  
--- Diff: geode-protobuf/src/main/proto/clientProtocol.proto ---
@@ -66,7 +66,7 @@ message Response {
 RemoveAllResponse removeAllResponse = 7;
 ListKeysResponse listKeysResponse = 8;
 
-ErrorResponse errorResponse = 13;
+Error error = 13;
--- End diff --

The reasoning is, we're not just using it as an ErrorResponse at the same 
level as a PutResponse or GetResponse -- it gets embedded in other responses 
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 #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_r129624935
  
--- 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 --

So the end result would be the wait for flush function would return false 
(it did not flush) and a message would be logged on the server side.  Based on 
the proposed fix of returning the exception, we would have been doing something 
similar anyways.  The client would have seen a false (because the function 
handled the exception and returned false)

So I guess having the test check for false would be just fine after 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.
---