Re: Request for Jenkins config ability

[Jens Deppe]

Thanks Mark - I'll test it out tomorrow.

On Thu, Aug 10, 2017 at 3:42 PM, Mark Bretl  wrote:

> Hi Jens,
>
> Skillz have been given.
>
> Let me know if you have any issues,
>
> --Mark
>
> On Thu, Aug 10, 2017 at 10:46 AM, Jens Deppe  wrote:
>
> > Hi,
> >
> > I'd like to request the ability to make Jenkins config changes for Geode
> > builds; I want to start implementing parallel dockerized dunit tests.
> >
> > Could someone with the appropriate skillz set me up please?
> >
> > Thanks
> > --Jens
> >
>

Re: PdxInitializationException: PDX registry could not be initialized

[Dan Smith]

Hi Aashita,

Do you have a stack trace for that PdxInitializationException? Maybe there
is a cause that has a bit more information?

-Dan

On Thu, Aug 10, 2017 at 4:24 PM, Mark Bretl  wrote:

> + user
>
> --Mark
>
> On Thu, Aug 10, 2017 at 4:12 PM, Aashita Sharma <
> aashitasha...@yahoo.co.in.invalid> wrote:
>
> > Hello Team,
> >
> > We recently got whole gemfire bounced, and every region lost its data.
> >
> >
> >
> > Later, we were told to populate data to every region through code. Now
> > while populating we got following error:
> >
> >
> >
> > PdxInitializationException: PDX registry
> > could not be initialized
> >
> >
> >
> > It is occurring for all regions.
> >
> >
> >
> > Request you to please help me understand what it actually means and how
> > should I avoid it to occur in future whenever bouncing of server happens.
> >
> >
> >
> >  Regards,  Aashita
>

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132600143
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/LocalProcessLauncher.java
 ---
@@ -96,33 +117,55 @@ void close() {
* @throws IOException if unable to create or write to the file
*/
   private void writePid(final boolean force) throws 
FileAlreadyExistsException, IOException {
-final boolean created = this.pidFile.createNewFile();
-if (!created && !force) {
-  int otherPid = 0;
-  try {
-otherPid = ProcessUtils.readPid(this.pidFile);
-  } catch (IOException e) {
-// suppress
-  } catch (NumberFormatException e) {
-// suppress
-  }
-  boolean ignorePidFile = false;
-  if (otherPid != 0 && !ignoreIsPidAlive()) {
-ignorePidFile = !ProcessUtils.isProcessAlive(otherPid);
-  }
-  if (!ignorePidFile) {
-throw new FileAlreadyExistsException("Pid file already exists: " + 
this.pidFile + " for "
-+ (otherPid > 0 ? "process " + otherPid : "unknown process"));
+if (this.pidFile.exists()) {
+  if (!force) {
+checkOtherPid(readOtherPid());
   }
+  this.pidFile.delete();
+}
+
+assert !this.pidFile.exists();
--- End diff --

I deleted them.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132599897
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/LocalProcessLauncher.java
 ---
@@ -96,33 +117,55 @@ void close() {
* @throws IOException if unable to create or write to the file
*/
   private void writePid(final boolean force) throws 
FileAlreadyExistsException, IOException {
-final boolean created = this.pidFile.createNewFile();
-if (!created && !force) {
-  int otherPid = 0;
-  try {
-otherPid = ProcessUtils.readPid(this.pidFile);
-  } catch (IOException e) {
-// suppress
-  } catch (NumberFormatException e) {
-// suppress
-  }
-  boolean ignorePidFile = false;
-  if (otherPid != 0 && !ignoreIsPidAlive()) {
-ignorePidFile = !ProcessUtils.isProcessAlive(otherPid);
-  }
-  if (!ignorePidFile) {
-throw new FileAlreadyExistsException("Pid file already exists: " + 
this.pidFile + " for "
-+ (otherPid > 0 ? "process " + otherPid : "unknown process"));
+if (this.pidFile.exists()) {
+  if (!force) {
+checkOtherPid(readOtherPid());
   }
+  this.pidFile.delete();
+}
+
+assert !this.pidFile.exists();
--- End diff --

Did you like the use of Validate? I'm not sure how I feel about Validate 
after using it heavily in this package.

Jianxia added the asserts when he fixed a bug in this method and I just 
left them in place. I think we could easily change them to Validate calls or 
remove them altogether.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132599768
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
 ---
@@ -112,56 +119,43 @@ private void stop(final File workingDir, final String 
stopRequestFileName) throw
   private String status(final File workingDir, final String 
statusRequestFileName,
   final String statusFileName) throws IOException, 
InterruptedException, TimeoutException {
 // monitor for statusFile
-final File statusFile = new File(workingDir, statusFileName);
-final AtomicReference statusRef = new AtomicReference<>();
-
-final ControlRequestHandler statusHandler = new 
ControlRequestHandler() {
-  @Override
-  public void handleRequest() throws IOException {
-// read the statusFile
-final BufferedReader reader = new BufferedReader(new 
FileReader(statusFile));
-final StringBuilder lines = new StringBuilder();
-try {
-  String line = null;
-  while ((line = reader.readLine()) != null) {
-lines.append(line);
-  }
-} finally {
-  statusRef.set(lines.toString());
-  reader.close();
+File statusFile = new File(workingDir, statusFileName);
+AtomicReference statusRef = new AtomicReference<>();
+
+ControlRequestHandler statusHandler = () -> {
--- End diff --

Done.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132599627
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/AttachProcessUtils.java
 ---
@@ -14,21 +14,28 @@
  */
 package org.apache.geode.internal.process;
 
-import org.apache.geode.internal.process.ProcessUtils.InternalProcessUtils;
+import static org.apache.commons.lang.Validate.isTrue;
+
 import com.sun.tools.attach.VirtualMachine;
 import com.sun.tools.attach.VirtualMachineDescriptor;
 
+import org.apache.geode.internal.process.ProcessUtils.InternalProcessUtils;
+
 /**
  * Implementation of the {@link ProcessUtils} SPI that uses the JDK Attach 
API.
  * 
  * @since GemFire 8.0
  */
 class AttachProcessUtils implements InternalProcessUtils {
 
-  AttachProcessUtils() {}
+  AttachProcessUtils() {
--- End diff --

Deleted ctor from AttachProcessUtils and NativeProcessUtils.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132598899
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -104,6 +109,8 @@
 helpMap.put("bind-address",
--- End diff --

I'd love to break these Launcher classes up into smaller classes. I'd also 
like to move them from org.apache.geode.distributed -- I don't think that pkg 
really makes sense for these classes to live in.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132598795
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java ---
@@ -795,25 +785,25 @@ protected String toString(final Date dateTime) {
 
 // the value of a Number as a String, or "" if null
 protected String toString(final Number value) {
-  return StringUtils.defaultString(value);
+  return defaultString(value);
 }
 
 // a String concatenation of all values separated by " "
 protected String toString(final Object... values) {
-  return values == null ? "" : StringUtils.join(values, " ");
+  return values == null ? "" : join(values, " ");
 }
 
 // the value of the String, or "" if value is null
 protected String toString(final String value) {
-  return ObjectUtils.defaultIfNull(value, "");
+  return defaultIfNull(value, "");
 }
   }
 
   /**
* The Status enumerated type represents the various lifecycle states of 
a GemFire service (such
* as a Cache Server, a Locator or a Manager).
*/
-  public static enum Status {
--- End diff --

It's static with or without the keyword. IntelliJ grays it out so I just 
deleted it. Similar to public on methods defined in an Interface.


---
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: PdxInitializationException: PDX registry could not be initialized

[Mark Bretl]

+ user

--Mark

On Thu, Aug 10, 2017 at 4:12 PM, Aashita Sharma <
aashitasha...@yahoo.co.in.invalid> wrote:

> Hello Team,
>
> We recently got whole gemfire bounced, and every region lost its data.
>
>
>
> Later, we were told to populate data to every region through code. Now
> while populating we got following error:
>
>
>
> PdxInitializationException: PDX registry
> could not be initialized
>
>
>
> It is occurring for all regions.
>
>
>
> Request you to please help me understand what it actually means and how
> should I avoid it to occur in future whenever bouncing of server happens.
>
>
>
>  Regards,  Aashita

[GitHub] geode pull request #700: GEODE-3386 - Make KeyedErrorResponse & ErrorRespons...

[WireBaron]

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

https://github.com/apache/geode/pull/700#discussion_r132591028
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAllRequestOperationHandler.java
 ---
@@ -41,9 +42,8 @@
 String regionName = request.getRegionName();
 Region region = cache.getRegion(regionName);
 if (region == null) {
-  return Failure.of(BasicTypes.ErrorResponse.newBuilder()
-  
.setErrorCode(ProtocolErrorCode.REGION_NOT_FOUND.codeValue).setMessage("Region 
not found")
-  .build());
+  return Failure.of(ProtobufResponseUtilities
+  .makeErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND.codeValue, 
"Region not found"));
--- End diff --

Thank you for making this change, this has been bothering me since I added 
the error code. Sorry I didn't do this sooner.


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

PdxInitializationException: PDX registry could not be initialized

[Aashita Sharma]

Hello Team,

We recently got whole gemfire bounced, and every region lost its data. 


 
Later, we were told to populate data to every region through code. Now while 
populating we got following error:


 
    PdxInitializationException: PDX registry could 
not be initialized


 
It is occurring for all regions.


 
Request you to please help me understand what it actually means and how should 
I avoid it to occur in future whenever bouncing of server happens.


 
 Regards,  Aashita

[GitHub] geode pull request #700: GEODE-3386 - Make KeyedErrorResponse & ErrorRespons...

[WireBaron]

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

https://github.com/apache/geode/pull/700#discussion_r132591701
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/PutAllRequestOperationHandler.java
 ---
@@ -28,6 +28,8 @@
 import org.apache.geode.serialization.SerializationService;
 import 
org.apache.geode.serialization.exception.UnsupportedEncodingTypeException;
 import 
org.apache.geode.serialization.registry.exception.CodecNotRegisteredForTypeException;
+
+import com.fasterxml.jackson.databind.introspect.TypeResolutionContext;
--- End diff --

I don't see how this new import is being used?


---
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 #700: GEODE-3386 - Make KeyedErrorResponse & ErrorRespons...

[WireBaron]

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

https://github.com/apache/geode/pull/700#discussion_r132591361
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAllRequestOperationHandler.java
 ---
@@ -59,13 +59,14 @@
   }
   return 
Success.of(RegionAPI.GetAllResponse.newBuilder().addAllEntries(entries).build());
 } catch (UnsupportedEncodingTypeException ex) {
-  return Failure.of(BasicTypes.ErrorResponse.newBuilder()
-  .setErrorCode(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue)
-  .setMessage("Encoding not supported.").build());
+  int errorCode = ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue;
+  String message = "Encoding not supported.";
+  return 
Failure.of(ProtobufResponseUtilities.makeErrorResponse(errorCode, message));
--- End diff --

Having a local variable for the error code and message or inlining them are 
both fine approaches, but it's a bit jarring to see both approaches used in two 
adjacent code blocks.  Please use a consistent approach.


---
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 #700: GEODE-3386 - Make KeyedErrorResponse & ErrorRespons...

[WireBaron]

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

https://github.com/apache/geode/pull/700#discussion_r132591888
  
--- Diff: 
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/PutAllRequestOperationHandler.java
 ---
@@ -79,9 +81,10 @@
   private BasicTypes.KeyedErrorResponse 
buildAndLogKeyedError(BasicTypes.Entry entry,
   ProtocolErrorCode errorCode, String message, Exception ex) {
 logger.error(message, ex);
-BasicTypes.ErrorResponse errorResponse = 
BasicTypes.ErrorResponse.newBuilder()
-.setErrorCode(errorCode.codeValue).setMessage(message).build();
-return 
BasicTypes.KeyedErrorResponse.newBuilder().setKey(entry.getKey()).setError(errorResponse)
+
+return 
BasicTypes.KeyedErrorResponse.newBuilder().setKey(entry.getKey())
+.setError(
+
BasicTypes.Error.newBuilder().setErrorCode(errorCode.codeValue).setMessage(message))
--- End diff --

Ugh...is this how spotless formatted 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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132592102
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/NonBlockingProcessStreamReaderIntegrationTest.java
 ---
@@ -0,0 +1,182 @@
+/*
+ * 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.process;
+
+import static 
org.apache.geode.internal.process.ProcessStreamReader.ReadingMode.NON_BLOCKING;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.process.ProcessStreamReader.Builder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+/**
+ * Functional integration tests for NonBlockingProcessStreamReader which 
was introduced to fix TRAC
+ * #51967.
+ *
+ * @see BlockingProcessStreamReaderIntegrationTest
+ * @see BlockingProcessStreamReaderWindowsTest
+ *
+ * @since GemFire 8.2
+ */
+@Category(IntegrationTest.class)
+public class NonBlockingProcessStreamReaderIntegrationTest
+extends BaseProcessStreamReaderIntegrationTest {
+
+  private StringBuffer stdoutBuffer;
+  private StringBuffer stderrBuffer;
+
+  @Before
+  public void before() {
+stdoutBuffer = new StringBuffer();
+stderrBuffer = new StringBuffer();
+  }
+
+  /**
+   * This test hangs on Windows if the implementation is blocking instead 
of non-blocking.
+   */
+  @Test
+  public void canCloseStreamsWhileProcessIsAlive() throws Exception {
+// arrange
+process = new 
ProcessBuilder(createCommandLine(ProcessSleeps.class)).start();
+stdout = new 
Builder(process).inputStream(process.getInputStream()).readingMode(NON_BLOCKING)
+.build().start();
+stderr = new 
Builder(process).inputStream(process.getErrorStream()).readingMode(NON_BLOCKING)
--- End diff --

Done.


---
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 #708: GEODE-3410 Doc update for gfsh query command change...

[jaredjstewart]

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

https://github.com/apache/geode/pull/708#discussion_r132591921
  
--- Diff: geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb ---
@@ -21,7 +21,11 @@ limitations under the License.
 
 Run queries against Geode regions.
 
-Run the specified OQL query as a single quoted string and displays results 
in pages allows to move between pages. If limit is not set in the query, then a 
default limit of 1000 (derived from GFSH environment variable APP\_FETCH\_SIZE) 
will be applied. Page size is derived from GFSH environment variable 
APP\_COLLECTION\_LIMIT (default value=20).
+If a limit restricting the result size is not set in the query,
+then a default limit of the gfsh environment variable `APP_FETCH_SIZE`
--- End diff --

Would it be much work to add a link to that section here in that case?  I 
just suspect if I was a user I would have no idea what "the gfsh environment 
variable `APP_FETCH_SIZE`" means.


---
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 #708: GEODE-3410 Doc update for gfsh query command change...

[karensmolermiller]

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

https://github.com/apache/geode/pull/708#discussion_r132590875
  
--- Diff: geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb ---
@@ -21,7 +21,11 @@ limitations under the License.
 
 Run queries against Geode regions.
 
-Run the specified OQL query as a single quoted string and displays results 
in pages allows to move between pages. If limit is not set in the query, then a 
default limit of 1000 (derived from GFSH environment variable APP\_FETCH\_SIZE) 
will be applied. Page size is derived from GFSH environment variable 
APP\_COLLECTION\_LIMIT (default value=20).
+If a limit restricting the result size is not set in the query,
+then a default limit of the gfsh environment variable `APP_FETCH_SIZE`
--- End diff --

The default size is given in the gfsh section titled 'Useful gfsh Shell 
Variables.'  I'd like to have that default value given in only 1 place in the 
documentation, such that if the value ever changes, we only need to update a 
single location.  If that is OK with you, I'll consider that you approve this 
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 #708: GEODE-3410 Doc update for gfsh query command change...

[jaredjstewart]

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

https://github.com/apache/geode/pull/708#discussion_r132589415
  
--- Diff: geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb ---
@@ -21,7 +21,11 @@ limitations under the License.
 
 Run queries against Geode regions.
 
-Run the specified OQL query as a single quoted string and displays results 
in pages allows to move between pages. If limit is not set in the query, then a 
default limit of 1000 (derived from GFSH environment variable APP\_FETCH\_SIZE) 
will be applied. Page size is derived from GFSH environment variable 
APP\_COLLECTION\_LIMIT (default value=20).
+If a limit restricting the result size is not set in the query,
+then a default limit of the gfsh environment variable `APP_FETCH_SIZE`
--- End diff --

It might be nice to also mention that `APP_FETCH_SIZE` has a default value 
of 100 if not explicitly set by the user.


---
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 > #643 was SUCCESSFUL (with 2027 tests)

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #643 was successful.
---
Scheduled
2029 tests in total.

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





--
This message is automatically generated by Atlassian Bamboo

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132571788
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/config/ClusterConfigurationNotAvailableException.java
 ---
@@ -0,0 +1,29 @@
+/*
+ * 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.config;
+
+/**
+ * Exception thrown during server startup when it requests the locators 
for shared configuration and
+ * does not receive it.
+ */
+public class ClusterConfigurationNotAvailableException
+extends 
org.apache.geode.internal.process.ClusterConfigurationNotAvailableException {
+
+  private static final long serialVersionUID = 771319836094239284L;
--- End diff --

Since this is a new class and there is no need to worry about backwards 
compatibility with an autogenerated `serialVersionUID`, I think you can just 
use `0L`.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132569537
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -1781,8 +1782,8 @@ protected void validate() {
  * @see org.apache.geode.distributed.LocatorLauncher.Command#START
  */
 protected void validateOnStart() {
-  if (Command.START.equals(getCommand())) {
-if (StringUtils.isBlank(getMemberName())
+  if (Command.START == getCommand()) {
--- End diff --

Cool, I didn't know that `==` could be used safely for enums.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132568184
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -1352,11 +1328,13 @@ protected void parseCommand(final String... args) {
  * @see 
org.apache.geode.distributed.LocatorLauncher.Command#isCommand(String)
  * @see #parseArguments(String...)
  */
-protected void parseMemberName(final String[] args) {
-  for (String arg : args) {
-if (!(arg.startsWith(OPTION_PREFIX) || Command.isCommand(arg))) {
-  setMemberName(arg);
-  break;
+protected void parseMemberName(final String... args) {
+  if (args != null) {
+for (String arg : args) {
+  if (!(arg.startsWith(OPTION_PREFIX) || Command.isCommand(arg))) {
--- End diff --

I believe the behavior of this method may contradicts its javadoc, which 
says that
> If the argument does **not** start with '-' or is **not** the name of a 
Locator launcher command, then the value is presumed to be the member name for 
the Locator in GemFire.

Whereas the actual implementation seems to find values which **are** the 
name of a Locator launcher command.  I would guess that the javadoc is correct 
about the intended behavior.


P.S. I can't resist writing this in a declarative style with streams :)
```
protected void parseMemberName(final String... args) {
  if (args == null) return;

  Arrays.stream(args)
  .filter(arg -> !(arg.startsWith(OPTION_PREFIX) || 
Command.isCommand(arg)))
  .findFirst()
  .ifPresent(this::setMemberName);
}
```


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132585045
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
 ---
@@ -112,56 +119,43 @@ private void stop(final File workingDir, final String 
stopRequestFileName) throw
   private String status(final File workingDir, final String 
statusRequestFileName,
   final String statusFileName) throws IOException, 
InterruptedException, TimeoutException {
 // monitor for statusFile
-final File statusFile = new File(workingDir, statusFileName);
-final AtomicReference statusRef = new AtomicReference<>();
-
-final ControlRequestHandler statusHandler = new 
ControlRequestHandler() {
-  @Override
-  public void handleRequest() throws IOException {
-// read the statusFile
-final BufferedReader reader = new BufferedReader(new 
FileReader(statusFile));
-final StringBuilder lines = new StringBuilder();
-try {
-  String line = null;
-  while ((line = reader.readLine()) != null) {
-lines.append(line);
-  }
-} finally {
-  statusRef.set(lines.toString());
-  reader.close();
+File statusFile = new File(workingDir, statusFileName);
+AtomicReference statusRef = new AtomicReference<>();
+
+ControlRequestHandler statusHandler = () -> {
--- End diff --

This might be a nice simplification:

```
ControlRequestHandler statusHandler = () -> {
  // read the statusFile
  StringBuilder lines = new StringBuilder();
  try (BufferedReader reader = new BufferedReader(new 
FileReader(statusFile))) {
reader.lines().forEach(lines::append);
  } finally {
statusRef.set(lines.toString());
  }
};
```


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132570355
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -104,6 +109,8 @@
 helpMap.put("bind-address",
--- End diff --

Since this class is rather large, maybe in the future we can extract the 
`helpMap` and `usageMap` to a separate class like `LocatorLauncherHelp` whose 
single responsibility is to provide the help and usage information.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132562988
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java ---
@@ -795,25 +785,25 @@ protected String toString(final Date dateTime) {
 
 // the value of a Number as a String, or "" if null
 protected String toString(final Number value) {
-  return StringUtils.defaultString(value);
+  return defaultString(value);
 }
 
 // a String concatenation of all values separated by " "
 protected String toString(final Object... values) {
-  return values == null ? "" : StringUtils.join(values, " ");
+  return values == null ? "" : join(values, " ");
 }
 
 // the value of the String, or "" if value is null
 protected String toString(final String value) {
-  return ObjectUtils.defaultIfNull(value, "");
+  return defaultIfNull(value, "");
 }
   }
 
   /**
* The Status enumerated type represents the various lifecycle states of 
a GemFire service (such
* as a Cache Server, a Locator or a Manager).
*/
-  public static enum Status {
--- End diff --

What is the motivation for removing static here?   I believe Effective Java 
advocates for favoring static member classes over non-static.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132572256
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/AttachProcessUtils.java
 ---
@@ -14,21 +14,28 @@
  */
 package org.apache.geode.internal.process;
 
-import org.apache.geode.internal.process.ProcessUtils.InternalProcessUtils;
+import static org.apache.commons.lang.Validate.isTrue;
+
 import com.sun.tools.attach.VirtualMachine;
 import com.sun.tools.attach.VirtualMachineDescriptor;
 
+import org.apache.geode.internal.process.ProcessUtils.InternalProcessUtils;
+
 /**
  * Implementation of the {@link ProcessUtils} SPI that uses the JDK Attach 
API.
  * 
  * @since GemFire 8.0
  */
 class AttachProcessUtils implements InternalProcessUtils {
 
-  AttachProcessUtils() {}
+  AttachProcessUtils() {
--- End diff --

Is there any reason to include this constructor 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 #699: GEODE-3413: overhaul launcher and process classes a...

[jaredjstewart]

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

https://github.com/apache/geode/pull/699#discussion_r132585334
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/LocalProcessLauncher.java
 ---
@@ -96,33 +117,55 @@ void close() {
* @throws IOException if unable to create or write to the file
*/
   private void writePid(final boolean force) throws 
FileAlreadyExistsException, IOException {
-final boolean created = this.pidFile.createNewFile();
-if (!created && !force) {
-  int otherPid = 0;
-  try {
-otherPid = ProcessUtils.readPid(this.pidFile);
-  } catch (IOException e) {
-// suppress
-  } catch (NumberFormatException e) {
-// suppress
-  }
-  boolean ignorePidFile = false;
-  if (otherPid != 0 && !ignoreIsPidAlive()) {
-ignorePidFile = !ProcessUtils.isProcessAlive(otherPid);
-  }
-  if (!ignorePidFile) {
-throw new FileAlreadyExistsException("Pid file already exists: " + 
this.pidFile + " for "
-+ (otherPid > 0 ? "process " + otherPid : "unknown process"));
+if (this.pidFile.exists()) {
+  if (!force) {
+checkOtherPid(readOtherPid());
   }
+  this.pidFile.delete();
+}
+
+assert !this.pidFile.exists();
--- End diff --

I saw that you used Apache common's `Validate` class in other places.  
Would that be good to use here since the default java `assert` won't do 
anything under normal circumstances?


---
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: Request for Jenkins config ability

[Mark Bretl]

Hi Jens,

Skillz have been given.

Let me know if you have any issues,

--Mark

On Thu, Aug 10, 2017 at 10:46 AM, Jens Deppe  wrote:

> Hi,
>
> I'd like to request the ability to make Jenkins config changes for Geode
> builds; I want to start implementing parallel dockerized dunit tests.
>
> Could someone with the appropriate skillz set me up please?
>
> Thanks
> --Jens
>

Failed: jinmeiliao/geode#15 (geode-3096-invoker - f789634)

[Travis CI]

Build Update for jinmeiliao/geode
-

Build: #15
Status: Failed

Duration: 17 minutes and 27 seconds
Commit: f789634 (geode-3096-invoker)
Author: Jinmei Liao
Message: GEODE-3096: pulling in refactoring work on HttpOperationInvoker

View the changeset: 
https://github.com/jinmeiliao/geode/compare/cfd971338811^...f789634515c9

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

--

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

[GitHub] geode-examples pull request #12: Add OQL example

[PivotalSarge]

GitHub user PivotalSarge opened a pull request:

https://github.com/apache/geode-examples/pull/12

Add OQL example

Perry Birch and I paired on an example for OQL. To prepare for that, I also 
created an example for Region.putAll().

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

$ git pull https://github.com/PivotalSarge/geode-examples develop

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

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


commit 98df7e42d0a4e390fd9114461af04a5cf9f8982e
Author: Sarge 
Date:   2017-08-04T22:42:07Z

Add example for Region.putAll().

commit 20a8c44a8a8068cb4abc2caca5fd278074cf8167
Author: Sarge 
Date:   2017-08-04T22:46:28Z

Fix sample gfsh command.

commit ac81624d7a809387068b0e0a71c0854c14b477ac
Author: Sarge 
Date:   2017-08-07T21:18:54Z

Replace foreach loop of put() with putAll().

commit 98d2325964145518f971793a67ea0c9a57509024
Author: Sarge 
Date:   2017-08-07T22:52:15Z

Add example for OQL.

commit b151f6285f7d3653d407cfb20ff85c22d6ee59af
Author: Sarge 
Date:   2017-08-08T18:44:46Z

Remove unused java.util.Random.

commit 06e82508d57050ec3760e6a0cb66164a2d330b95
Author: Sarge 
Date:   2017-08-08T18:45:34Z

Add Perry's changes.

commit 43dd33833c306de21aaaf4cfa723a5f0408bc573
Author: Sarge 
Date:   2017-08-10T20:27:29Z

Fix putAll example test.

commit eb747a6d359fb2853894db7b94e289c3c58be358
Author: Sarge 
Date:   2017-08-10T22:16:29Z

Reduce the OQL example down to the consecutive queries example.




---
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 61585: GEODE-3096: pulling in refactoring work on HttpOperationInvoker

[Jinmei Liao]

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

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


Repository: geode


Description
---

GEODE-3096: pulling in refactoring work on HttpOperationInvoker

These are the refactoring work we did on those three weeks, minus the query 
(which is already in develop), and minus pulling commands into it's own 
class(Emily's work is gonna help on that one). It's rebased on top of the 
current develop now.


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshExecution.java
 23f2a73acf2cf92a8b1c0c2ea2afd10392265628 
  geode-common/src/main/java/org/apache/geode/annotations/TestingOnly.java 
PRE-CREATION 
  geode-core/build.gradle 9ecb0f9cca43eba4904af6f656bf34551bdd5b15 
  
geode-core/src/main/java/org/apache/geode/management/cli/CommandStatement.java 
a01f08c2f09b9c762bbd4ef561ce0ba26d22dd73 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
 3c8f6cf235d0f1b34d948d23e39fddfbe306be2c 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java
 00a05872a512f294f914dbc8ac1c12b799a9145d 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponse.java
 81c49583b759ea815f6f20fb1c6da7edb7f99b2f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
 3f8f20d1055973f5af6b2a22d8656014cf2c704e 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 274f61c1f304576f8d8db1d5289875ecea706962 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 84326c09ff24d194469ce8de435d5cf640615d06 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
 83eddeebb472758944863cde098746c7ff8da5a4 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
 20757766ae8aebb2dffbc0b775d8689815ab3fd6 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
 c7f53b1552b45f340b244828cb76d09c8aaa83da 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
 2464b0065d61b1aafc3b933f5f1a04e90e95c689 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandStatementImpl.java
 ac510d1755c7dba1c2c7a772887a5c1b64cdcf57 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
 25ff549be2bf706c3e3a312fa1b6ce6b423996e7 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/RemoteExecutionStrategy.java
 75dce477ba6ce0352ff2575daa7b16f66f1acf18 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/AbstractResultData.java
 0d1d91bc150a364c97d7cd1c23e7ab51bfb41163 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/CommandResult.java
 00f1fd9376a144e32824384dab14bbb96ef93973 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/DownloadFileResult.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ErrorResultData.java
 7ae0d8083600bba89eb625b95408afc9b9896059 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/InfoResultData.java
 f399a54159d3e1d8b8a68ec59a8ee68c5ca4bbe7 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java
 94dbca07757fab196522eccc9e1239f6c6911b24 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 503c2522b1b9b492e6167940543669b560e490d1 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
 6794b3ba06c2e5dba944942f9ac099eb51930a80 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/ScriptExecutionDetails.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java
 0a18ec522884f5f737045b0da53a0e49e1cd6aa9 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ClientCommandsController.java
 e8df505f35bf751fa5a57acb3cd1d75ea2e0e35c 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ClusterCommandsController.java
 fae10debac618594d191f08e9b45b85e71769a32 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java
 

Re: Review Request 61562: GEODE-3423: Provide support for running parallel docker builds in Jenkins

[Jens Deppe]

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

(Updated Aug. 10, 2017, 9:16 p.m.)


Review request for geode, Anthony Baker and Mark Bretl.


Changes
---

Switched to openjdk as base image


Repository: geode


Description
---

- Also cleaning up other Dockerfiles which are unused

Signed-off-by: Scott Jewell 


Diffs (updated)
-

  dev-tools/docker/base/Dockerfile 1cce0ddb0d29c9e188d27481c82123e357c5b685 
  dev-tools/docker/base/build-base-docker.sh 
9aab72c45d63c519874ad39aaf74c8236d9671ed 
  dev-tools/docker/compile/Dockerfile 6ae343a70eccf7cc4b303228118cee0b8579e79a 
  dev-tools/docker/compile/start-compile-docker.sh 
9059c5b5bbffbadaa82277c05e2189360d94a484 
  gradle/docker.gradle 79719740bc85fd1939d2d7859a7c78c0a87dd26e 


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

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


Testing
---


Thanks,

Jens Deppe

[GitHub] geode issue #704: GEODE-3300: Complete and expose parallel export feature fo...

[nreich]

Github user nreich commented on the issue:

https://github.com/apache/geode/pull/704
  
The requirement for the ".gfd" extension currently resides in the 
ExportDataCommand from gfsh, the only non-internal entry point to 
create/restore a backup. This constraint could be also validated in 
RegionSnapshotServiceImpl before starting the local snapshot creation.

I can add a toString to SnapshotOptions, some of the contents will still 
return an only somewhat helpful Object.toString() value, but it should provide 
the desired information.


---
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 #688: GEODE-3397: Fixed issue with Tomcat locators in cac...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 61562: GEODE-3423: Provide support for running parallel docker builds in Jenkins

[Anthony Baker]

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




dev-tools/docker/base/Dockerfile
Line 27 (original), 27 (patched)


We can't embedd/redistribute the Oracle JDK due to licensing.  We should 
use the openjdk:8 as a base image (see https://hub.docker.com/_/openjdk/).


- Anthony Baker


On Aug. 10, 2017, 5:34 p.m., Jens Deppe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61562/
> ---
> 
> (Updated Aug. 10, 2017, 5:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Mark Bretl.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> - Also cleaning up other Dockerfiles which are unused
> 
> Signed-off-by: Scott Jewell 
> 
> 
> Diffs
> -
> 
>   dev-tools/docker/base/Dockerfile 1cce0ddb0d29c9e188d27481c82123e357c5b685 
>   dev-tools/docker/base/build-base-docker.sh 
> 9aab72c45d63c519874ad39aaf74c8236d9671ed 
>   dev-tools/docker/compile/Dockerfile 
> 6ae343a70eccf7cc4b303228118cee0b8579e79a 
>   dev-tools/docker/compile/start-compile-docker.sh 
> 9059c5b5bbffbadaa82277c05e2189360d94a484 
>   gradle/docker.gradle 79719740bc85fd1939d2d7859a7c78c0a87dd26e 
> 
> 
> Diff: https://reviews.apache.org/r/61562/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jens Deppe
> 
>

[GitHub] geode pull request #708: GEODE-3410 Doc update for gfsh query command change...

[karensmolermiller]

GitHub user karensmolermiller opened a pull request:

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

GEODE-3410 Doc update for gfsh query command changes

- the --interactive option is gone
- there is a new --file option
- correct default value for APP_FETCH_SIZE gfsh env var
- gfsh env var APP_COLLECTION_LIMIT is no longer used

@jaredjstewart @jinmeiliao @kirklund @davebarnes97 @joeymcallister Please 
review.

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

$ git pull https://github.com/karensmolermiller/geode feature/GEODE-3410

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

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


commit 75ca6ab8cc41db02d0240d62ac6c034d94727e76
Author: Karen Miller 
Date:   2017-08-10T19:53:25Z

GEODE-3410 Doc update for gfsh query command changes

- the --interactive option is gone
- there is a new --file option
- correct default value for APP_FETCH_SIZE gfsh env var
- gfsh env var APP_COLLECTION_LIMIT is no longer used




---
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 #699: GEODE-3413: overhaul launcher and process classes and test...

[kirklund]

Github user kirklund commented on the issue:

https://github.com/apache/geode/pull/699
  
These tests have also now been overhauled or rewritten:
* LauncherMemberMXBeanIntegrationTest
* ServerLauncherWaitOnServerMultiThreadedTest
* ServerLauncherWithProviderIntegrationTest -> 
ServerLauncherWithProviderRegressionTest


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542184
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTestCase.java
 ---
@@ -0,0 +1,270 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static 
org.apache.geode.distributed.internal.DistributionConfig.GEMFIRE_PREFIX;
+import static 
org.apache.geode.internal.cache.AbstractCacheServer.TEST_OVERRIDE_DEFAULT_PORT_PROPERTY;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.net.BindException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.junit.After;
+import org.junit.Before;
+
+import org.apache.geode.distributed.AbstractLauncher.Status;
+import org.apache.geode.internal.process.ProcessStreamReader;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+
+/**
+ * Abstract base class for integration tests of {@link ServerLauncher} as 
an application main in a
+ * forked JVM.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class ServerLauncherRemoteIntegrationTestCase
+extends ServerLauncherIntegrationTestCase implements UsesServerCommand 
{
+
+  private final AtomicBoolean threwBindException = new AtomicBoolean();
+
+  protected volatile Process process;
+  protected volatile ProcessStreamReader processOutReader;
+  protected volatile ProcessStreamReader processErrReader;
+
+  private ServerCommand serverCommand;
+
+  @Before
+  public void setUp() throws Exception {
+serverCommand = new ServerCommand(this);
+  }
+
+  @After
+  public void tearDownAbstractServerLauncherRemoteIntegrationTestCase() 
throws Exception {
+if (this.process != null) {
+  this.process.destroy();
+  this.process = null;
+}
+if (this.processOutReader != null && 
this.processOutReader.isRunning()) {
+  this.processOutReader.stop();
+}
+if (this.processErrReader != null && 
this.processErrReader.isRunning()) {
+  this.processErrReader.stop();
+}
+  }
+
+  @Override
+  public List getJvmArguments() {
+List jvmArguments = new ArrayList<>();
+jvmArguments.add("-D" + GEMFIRE_PREFIX + LOG_LEVEL + "=config");
+jvmArguments.add("-D" + GEMFIRE_PREFIX + MCAST_PORT + "=0");
+jvmArguments
+.add("-D" + TEST_OVERRIDE_DEFAULT_PORT_PROPERTY + "=" + 
String.valueOf(defaultServerPort));
+return jvmArguments;
+  }
+
+  @Override
+  public String getName() {
+return getUniqueName();
+  }
+
+  @Override
+  public boolean getDisableDefaultServer() {
+return false;
+  }
+
+  protected void assertThatServerThrewBindException() {
+assertThat(threwBindException.get()).isTrue();
+  }
+
+  protected void assertThatServerThrew(final Class 
throwableClass) {
+assertThat(threwBindException.get()).isTrue();
+  }
+
+  protected ServerLauncher startServer(final ServerCommand command) {
+return awaitStart(command);
+  }
+
+  protected ServerLauncher awaitStart(final ServerCommand command,
+  final ProcessStreamReader.InputListener outListener,
+  final ProcessStreamReader.InputListener errListener) throws 
Exception {
+executeCommandWithReaders(command.create(), outListener, errListener);
+ServerLauncher launcher = awaitStart(getWorkingDirectory());
  

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542091
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/LauncherIntegrationTestCase.java
 ---
@@ -0,0 +1,505 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.internal.AvailablePort.SOCKET;
+import static org.apache.geode.internal.AvailablePort.isPortAvailable;
+import static org.apache.geode.internal.process.ProcessUtils.identifyPid;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.management.ManagementFactory;
+import java.net.ServerSocket;
+import java.nio.file.Files;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
+
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.process.PidUnavailableException;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+import org.apache.geode.internal.process.ProcessType;
+import org.apache.geode.internal.process.ProcessUtils;
+import org.apache.geode.internal.process.lang.AvailablePid;
+import org.apache.geode.internal.util.StopWatch;
+import org.apache.geode.test.dunit.IgnoredException;
+
+/**
+ * Abstract base class for integration tests of both {@link 
LocatorLauncher} and
+ * {@link ServerLauncher}.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class LauncherIntegrationTestCase {
+  protected static final Logger logger = LogService.getLogger();
+
+  protected static final int WAIT_FOR_PROCESS_TO_DIE_TIMEOUT = 5 * 60 * 
1000; // 5 minutes
+  protected static final int TIMEOUT_MILLISECONDS = 
WAIT_FOR_PROCESS_TO_DIE_TIMEOUT;
+  protected static final int WAIT_FOR_FILE_CREATION_TIMEOUT = 10 * 1000; 
// 10s
+  protected static final int WAIT_FOR_FILE_DELETION_TIMEOUT = 10 * 1000; 
// 10s
+  protected static final int WAIT_FOR_MBEAN_TIMEOUT = 10 * 1000; // 10s
--- End diff --

All unused fields deleted.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542146
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/LauncherIntegrationTestCase.java
 ---
@@ -0,0 +1,505 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.internal.AvailablePort.SOCKET;
+import static org.apache.geode.internal.AvailablePort.isPortAvailable;
+import static org.apache.geode.internal.process.ProcessUtils.identifyPid;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.management.ManagementFactory;
+import java.net.ServerSocket;
+import java.nio.file.Files;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
+
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.process.PidUnavailableException;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+import org.apache.geode.internal.process.ProcessType;
+import org.apache.geode.internal.process.ProcessUtils;
+import org.apache.geode.internal.process.lang.AvailablePid;
+import org.apache.geode.internal.util.StopWatch;
+import org.apache.geode.test.dunit.IgnoredException;
+
+/**
+ * Abstract base class for integration tests of both {@link 
LocatorLauncher} and
+ * {@link ServerLauncher}.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class LauncherIntegrationTestCase {
+  protected static final Logger logger = LogService.getLogger();
+
+  protected static final int WAIT_FOR_PROCESS_TO_DIE_TIMEOUT = 5 * 60 * 
1000; // 5 minutes
+  protected static final int TIMEOUT_MILLISECONDS = 
WAIT_FOR_PROCESS_TO_DIE_TIMEOUT;
+  protected static final int WAIT_FOR_FILE_CREATION_TIMEOUT = 10 * 1000; 
// 10s
+  protected static final int WAIT_FOR_FILE_DELETION_TIMEOUT = 10 * 1000; 
// 10s
+  protected static final int WAIT_FOR_MBEAN_TIMEOUT = 10 * 1000; // 10s
+  protected static final int INTERVAL_MILLISECONDS = 100;
+
+  protected static final int PREFERRED_FAKE_PID = 42;
+
+  private static final String EXPECTED_EXCEPTION_ADD =
+  "{}";
+  private static final String EXPECTED_EXCEPTION_REMOVE =
+  "{}";
+  private static final String EXPECTED_EXCEPTION_MBEAN_NOT_REGISTERED =
+  "MBean Not Registered In GemFire Domain";
+
+  private IgnoredException ignoredException;
+
+  protected int localPid;
+  protected int fakePid;
+
+  protected volatile ServerSocket socket;
+
+  protected volatile File pidFile;
+  protected volatile File stopRequestFile;
+  protected volatile File statusRequestFile;
+  protected volatile File statusFile;
+
+  @Rule
+  public TestName testName = new TestName();
+
+  @Rule
 

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542169
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTestCase.java
 ---
@@ -0,0 +1,270 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static 
org.apache.geode.distributed.internal.DistributionConfig.GEMFIRE_PREFIX;
+import static 
org.apache.geode.internal.cache.AbstractCacheServer.TEST_OVERRIDE_DEFAULT_PORT_PROPERTY;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.net.BindException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.junit.After;
+import org.junit.Before;
+
+import org.apache.geode.distributed.AbstractLauncher.Status;
+import org.apache.geode.internal.process.ProcessStreamReader;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+
+/**
+ * Abstract base class for integration tests of {@link ServerLauncher} as 
an application main in a
+ * forked JVM.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class ServerLauncherRemoteIntegrationTestCase
+extends ServerLauncherIntegrationTestCase implements UsesServerCommand 
{
+
+  private final AtomicBoolean threwBindException = new AtomicBoolean();
+
+  protected volatile Process process;
+  protected volatile ProcessStreamReader processOutReader;
+  protected volatile ProcessStreamReader processErrReader;
+
+  private ServerCommand serverCommand;
+
+  @Before
+  public void setUp() throws Exception {
+serverCommand = new ServerCommand(this);
+  }
+
+  @After
+  public void tearDownAbstractServerLauncherRemoteIntegrationTestCase() 
throws Exception {
+if (this.process != null) {
+  this.process.destroy();
+  this.process = null;
+}
+if (this.processOutReader != null && 
this.processOutReader.isRunning()) {
+  this.processOutReader.stop();
+}
+if (this.processErrReader != null && 
this.processErrReader.isRunning()) {
+  this.processErrReader.stop();
+}
+  }
+
+  @Override
+  public List getJvmArguments() {
+List jvmArguments = new ArrayList<>();
+jvmArguments.add("-D" + GEMFIRE_PREFIX + LOG_LEVEL + "=config");
+jvmArguments.add("-D" + GEMFIRE_PREFIX + MCAST_PORT + "=0");
+jvmArguments
+.add("-D" + TEST_OVERRIDE_DEFAULT_PORT_PROPERTY + "=" + 
String.valueOf(defaultServerPort));
+return jvmArguments;
+  }
+
+  @Override
+  public String getName() {
+return getUniqueName();
+  }
+
+  @Override
+  public boolean getDisableDefaultServer() {
+return false;
+  }
+
+  protected void assertThatServerThrewBindException() {
+assertThat(threwBindException.get()).isTrue();
+  }
+
+  protected void assertThatServerThrew(final Class 
throwableClass) {
+assertThat(threwBindException.get()).isTrue();
+  }
+
+  protected ServerLauncher startServer(final ServerCommand command) {
+return awaitStart(command);
+  }
+
+  protected ServerLauncher awaitStart(final ServerCommand command,
+  final ProcessStreamReader.InputListener outListener,
+  final ProcessStreamReader.InputListener errListener) throws 
Exception {
+executeCommandWithReaders(command.create(), outListener, errListener);
+ServerLauncher launcher = awaitStart(getWorkingDirectory());
  

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542242
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -1075,8 +1082,7 @@ private LocatorState stopWithWorkingDirectory() {
 try {
   final ProcessController controller =
   new 
ProcessControllerFactory().createProcessController(this.controllerParameters,
-  new File(getWorkingDirectory()), 
ProcessType.LOCATOR.getPidFileName(),
-  READ_PID_FILE_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
+  new File(getWorkingDirectory()), 
ProcessType.LOCATOR.getPidFileName());
   parsedPid = controller.getProcessId();
 
   // NOTE in-process request will go infinite loop unless we do the 
following
--- End diff --

Done.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542157
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTestCase.java
 ---
@@ -0,0 +1,270 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static 
org.apache.geode.distributed.internal.DistributionConfig.GEMFIRE_PREFIX;
+import static 
org.apache.geode.internal.cache.AbstractCacheServer.TEST_OVERRIDE_DEFAULT_PORT_PROPERTY;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.net.BindException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.junit.After;
+import org.junit.Before;
+
+import org.apache.geode.distributed.AbstractLauncher.Status;
+import org.apache.geode.internal.process.ProcessStreamReader;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+
+/**
+ * Abstract base class for integration tests of {@link ServerLauncher} as 
an application main in a
+ * forked JVM.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class ServerLauncherRemoteIntegrationTestCase
+extends ServerLauncherIntegrationTestCase implements UsesServerCommand 
{
+
+  private final AtomicBoolean threwBindException = new AtomicBoolean();
+
+  protected volatile Process process;
+  protected volatile ProcessStreamReader processOutReader;
+  protected volatile ProcessStreamReader processErrReader;
+
+  private ServerCommand serverCommand;
+
+  @Before
+  public void setUp() throws Exception {
+serverCommand = new ServerCommand(this);
+  }
+
+  @After
+  public void tearDownAbstractServerLauncherRemoteIntegrationTestCase() 
throws Exception {
+if (this.process != null) {
+  this.process.destroy();
+  this.process = null;
+}
+if (this.processOutReader != null && 
this.processOutReader.isRunning()) {
+  this.processOutReader.stop();
+}
+if (this.processErrReader != null && 
this.processErrReader.isRunning()) {
+  this.processErrReader.stop();
+}
+  }
+
+  @Override
+  public List getJvmArguments() {
+List jvmArguments = new ArrayList<>();
+jvmArguments.add("-D" + GEMFIRE_PREFIX + LOG_LEVEL + "=config");
+jvmArguments.add("-D" + GEMFIRE_PREFIX + MCAST_PORT + "=0");
+jvmArguments
+.add("-D" + TEST_OVERRIDE_DEFAULT_PORT_PROPERTY + "=" + 
String.valueOf(defaultServerPort));
+return jvmArguments;
+  }
+
+  @Override
+  public String getName() {
+return getUniqueName();
+  }
+
+  @Override
+  public boolean getDisableDefaultServer() {
+return false;
+  }
+
+  protected void assertThatServerThrewBindException() {
+assertThat(threwBindException.get()).isTrue();
+  }
+
+  protected void assertThatServerThrew(final Class 
throwableClass) {
+assertThat(threwBindException.get()).isTrue();
+  }
+
+  protected ServerLauncher startServer(final ServerCommand command) {
+return awaitStart(command);
+  }
+
+  protected ServerLauncher awaitStart(final ServerCommand command,
+  final ProcessStreamReader.InputListener outListener,
+  final ProcessStreamReader.InputListener errListener) throws 
Exception {
+executeCommandWithReaders(command.create(), outListener, errListener);
+ServerLauncher launcher = awaitStart(getWorkingDirectory());
  

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542106
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/LauncherIntegrationTestCase.java
 ---
@@ -0,0 +1,505 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.internal.AvailablePort.SOCKET;
+import static org.apache.geode.internal.AvailablePort.isPortAvailable;
+import static org.apache.geode.internal.process.ProcessUtils.identifyPid;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.management.ManagementFactory;
+import java.net.ServerSocket;
+import java.nio.file.Files;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
+
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.process.PidUnavailableException;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+import org.apache.geode.internal.process.ProcessType;
+import org.apache.geode.internal.process.ProcessUtils;
+import org.apache.geode.internal.process.lang.AvailablePid;
+import org.apache.geode.internal.util.StopWatch;
+import org.apache.geode.test.dunit.IgnoredException;
+
+/**
+ * Abstract base class for integration tests of both {@link 
LocatorLauncher} and
+ * {@link ServerLauncher}.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class LauncherIntegrationTestCase {
+  protected static final Logger logger = LogService.getLogger();
+
+  protected static final int WAIT_FOR_PROCESS_TO_DIE_TIMEOUT = 5 * 60 * 
1000; // 5 minutes
+  protected static final int TIMEOUT_MILLISECONDS = 
WAIT_FOR_PROCESS_TO_DIE_TIMEOUT;
+  protected static final int WAIT_FOR_FILE_CREATION_TIMEOUT = 10 * 1000; 
// 10s
+  protected static final int WAIT_FOR_FILE_DELETION_TIMEOUT = 10 * 1000; 
// 10s
+  protected static final int WAIT_FOR_MBEAN_TIMEOUT = 10 * 1000; // 10s
+  protected static final int INTERVAL_MILLISECONDS = 100;
+
+  protected static final int PREFERRED_FAKE_PID = 42;
+
+  private static final String EXPECTED_EXCEPTION_ADD =
+  "{}";
+  private static final String EXPECTED_EXCEPTION_REMOVE =
--- End diff --

All unused fields deleted.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542063
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/LauncherIntegrationTestCase.java
 ---
@@ -0,0 +1,505 @@
+/*
+ * 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.distributed;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.internal.AvailablePort.SOCKET;
+import static org.apache.geode.internal.AvailablePort.isPortAvailable;
+import static org.apache.geode.internal.process.ProcessUtils.identifyPid;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.management.ManagementFactory;
+import java.net.ServerSocket;
+import java.nio.file.Files;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
+
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.process.PidUnavailableException;
+import org.apache.geode.internal.process.ProcessStreamReader.InputListener;
+import org.apache.geode.internal.process.ProcessType;
+import org.apache.geode.internal.process.ProcessUtils;
+import org.apache.geode.internal.process.lang.AvailablePid;
+import org.apache.geode.internal.util.StopWatch;
+import org.apache.geode.test.dunit.IgnoredException;
+
+/**
+ * Abstract base class for integration tests of both {@link 
LocatorLauncher} and
+ * {@link ServerLauncher}.
+ *
+ * @since GemFire 8.0
+ */
+public abstract class LauncherIntegrationTestCase {
+  protected static final Logger logger = LogService.getLogger();
+
+  protected static final int WAIT_FOR_PROCESS_TO_DIE_TIMEOUT = 5 * 60 * 
1000; // 5 minutes
+  protected static final int TIMEOUT_MILLISECONDS = 
WAIT_FOR_PROCESS_TO_DIE_TIMEOUT;
--- End diff --

All unused fields deleted.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542016
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/BlockingProcessStreamReaderIntegrationTest.java
 ---
@@ -0,0 +1,179 @@
+/*
+ * 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.process;
+
+import static org.apache.geode.internal.lang.SystemUtils.isWindows;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assume.assumeFalse;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.process.ProcessStreamReader.Builder;
+import org.apache.geode.internal.process.ProcessStreamReader.ReadingMode;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+/**
+ * Functional integration tests for BlockingProcessStreamReader. All tests 
are skipped on Windows
+ * due to TRAC bug #51967 which is caused by a JDK bug. See
--- End diff --

I added description.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132542008
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/BlockingProcessStreamReaderWindowsTest.java
 ---
@@ -0,0 +1,97 @@
+/*
+ * 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.process;
+
+import static org.apache.geode.internal.lang.SystemUtils.isWindows;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.Assume.assumeTrue;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+/**
+ * Functional integration test {@link #hangsOnWindows} for 
BlockingProcessStreamReader which
+ * verifies TRAC #51967 on Windows. The hang is caused by a JDK bug in 
which a thread invoking
--- End diff --

I added description.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132541957
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java ---
@@ -1848,8 +1884,7 @@ public LocatorLauncher build() {
 private final String name;
 
 Command(final String name, final String... options) {
-  assert !StringUtils
-  .isBlank(name) : "The name of the locator launcher command must 
be specified!";
+  assert !isBlank(name) : "The name of the locator launcher command 
must be specified!";
--- End diff --

Changed.


---
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 #703: GEODE-3328 Properties to set Key/Trust Store Type f...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132541795
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/MBeanProcessController.java
 ---
@@ -228,9 +230,9 @@ private void disconnect() {
* @throws PidUnavailableException if parsing the pid from the 
RuntimeMXBean name fails
*/
   boolean checkPidMatches() throws IllegalStateException, IOException, 
PidUnavailableException {
--- End diff --

Deleted.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132541592
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java ---
@@ -502,7 +507,7 @@ protected static Integer identifyPid() {
   }
 }
 
-// TODO refactor the logic in this method into a DateTimeFormatUtils 
class
+// consider refactoring the logic in this method into a 
DateTimeFormatUtils class
--- End diff --

Deleted.


---
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 61540: GEODE-3403: Modify rolling upgrade test configuration for 1.2.x release

[Udo Kohlmeyer]

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




geode-core/src/main/java/org/apache/geode/internal/Version.java
Line 205 (original), 210 (patched)


what is the correlation between this and HIGHED_VERSION?
Could we automate this? External file? etc


- Udo Kohlmeyer


On Aug. 9, 2017, 8:45 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61540/
> ---
> 
> (Updated Aug. 9, 2017, 8:45 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Udo Kohlmeyer, 
> and Brian Rowe.
> 
> 
> Bugs: GEODE-3403
> https://issues.apache.org/jira/browse/GEODE-3403
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Nabarun has already added a test120 source set in geode-old-versions.  This 
> commit will roll the version on develop to 1.3.0 so that it will have a 
> different version ordinal than test120.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 39e3a3f18d90567a1e3564884760014f6daf1f4c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  9995e4658e5afcb5eb9823913e73aa04d1cdbdbd 
> 
> 
> Diff: https://reviews.apache.org/r/61540/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132535229
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java ---
@@ -157,7 +162,7 @@ protected static Properties loadGemFireProperties(final 
URL url) {
 if (url != null) {
   try {
 properties.load(new FileReader(new File(url.toURI(;
-  } catch (Exception e) {
+  } catch (Exception ignored) {
--- End diff --

Done.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132533643
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 ---
@@ -31,7 +31,7 @@
 import org.apache.geode.internal.admin.remote.DistributionLocatorId;
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.logging.LogService;
-import 
org.apache.geode.internal.process.ClusterConfigurationNotAvailableException;
+import 
org.apache.geode.config.internal.ClusterConfigurationNotAvailableException;
--- End diff --

Done.


---
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 61540: GEODE-3403: Modify rolling upgrade test configuration for 1.2.x release

[Brian Rowe]

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


Ship it!




Ship It!

- Brian Rowe


On Aug. 9, 2017, 8:45 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61540/
> ---
> 
> (Updated Aug. 9, 2017, 8:45 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Udo Kohlmeyer, 
> and Brian Rowe.
> 
> 
> Bugs: GEODE-3403
> https://issues.apache.org/jira/browse/GEODE-3403
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Nabarun has already added a test120 source set in geode-old-versions.  This 
> commit will roll the version on develop to 1.3.0 so that it will have a 
> different version ordinal than test120.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 39e3a3f18d90567a1e3564884760014f6daf1f4c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  9995e4658e5afcb5eb9823913e73aa04d1cdbdbd 
> 
> 
> Diff: https://reviews.apache.org/r/61540/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132532669
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/AbstractProcessStreamReaderIntegrationTest.java
 ---
@@ -0,0 +1,234 @@
+/*
+ * 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.process;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.apache.commons.lang.SystemUtils.LINE_SEPARATOR;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.junit.After;
+
+import org.apache.geode.internal.util.StopWatch;
+
+/**
+ * Abstract base class for functional integration testing of {@link 
ProcessStreamReader}.
+ */
+public abstract class AbstractProcessStreamReaderIntegrationTest {
+
+  /** Timeout to join to a running ProcessStreamReader thread */
+  protected static final int READER_JOIN_TIMEOUT_MILLIS = 20 * 1000;
+
+  /** Sleep timeout for {@link ProcessSleeps} instead of sleeping 
Long.MAX_VALUE */
+  private static final int PROCESS_FAILSAFE_TIMEOUT_MILLIS = 10 * 60 * 
1000;
+
+  /** Additional time for launched processes to live before terminating */
+  private static final int PROCESS_TIME_TO_LIVE_MILLIS = 3 * 500;
+
+  /** Timeout to wait for a new {@link ProcessStreamReader} to be running 
*/
+  private static final int WAIT_FOR_READER_IS_RUNNING_TIMEOUT_MILLIS = 20 
* 1000;
+
+  protected Process process;
+  protected ProcessStreamReader stderr;
+  protected ProcessStreamReader stdout;
+
+  @After
+  public void afterProcessStreamReaderTestCase() throws Exception {
+if (stderr != null) {
+  stderr.stop();
+}
+if (stdout != null) {
+  stdout.stop();
+}
+if (process != null) {
+  try {
+process.getErrorStream().close();
+process.getInputStream().close();
+process.getOutputStream().close();
+  } finally {
+// this is async and can require more than 10 seconds on slower 
machines
+process.destroy();
+  }
+}
+  }
+
+  protected ConditionFactory await() {
+return 
Awaitility.await().atMost(WAIT_FOR_READER_IS_RUNNING_TIMEOUT_MILLIS, 
MILLISECONDS);
+  }
+
+  protected static boolean isAlive(final Process process) {
--- End diff --

Done.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132532612
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/config/internal/ClusterConfigurationNotAvailableException.java
 ---
@@ -0,0 +1,29 @@
+/*
+ * 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.config.internal;
--- End diff --

Changed.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

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

https://github.com/apache/geode/pull/699#discussion_r132532641
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/FileProcessControllerIntegrationTest.java
 ---
@@ -0,0 +1,243 @@
+/*
+ * 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.process;
+
+import static com.googlecode.catchexception.CatchException.caughtException;
+import static com.googlecode.catchexception.CatchException.verifyException;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.ErrorCollector;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
+
+import org.apache.geode.distributed.AbstractLauncher.Status;
+import org.apache.geode.distributed.LocatorLauncher;
+import org.apache.geode.distributed.LocatorLauncher.Builder;
+import org.apache.geode.distributed.LocatorLauncher.LocatorState;
+import org.apache.geode.internal.process.io.EmptyFileWriter;
+import org.apache.geode.internal.process.io.IntegerFileWriter;
+import org.apache.geode.internal.process.io.StringFileWriter;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+/**
+ * Integration tests for {@link FileProcessController}.
+ */
+@Category(IntegrationTest.class)
+public class FileProcessControllerIntegrationTest {
+
+  private static final String STATUS_JSON = generateStatusJson();
+
+  private final AtomicReference statusRef = new 
AtomicReference<>();
+
+  private File pidFile;
+  private File statusFile;
+  private File statusRequestFile;
+  private File stopRequestFile;
+  private int pid;
+  private FileControllerParameters params;
+  private ExecutorService executor;
+
+  @Rule
+  public ErrorCollector errorCollector = new ErrorCollector();
+
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Rule
+  public TestName testName = new TestName();
+
+  @Before
+  public void setUp() throws Exception {
+ProcessType processType = ProcessType.LOCATOR;
+File directory = temporaryFolder.getRoot();
+pidFile = new File(directory, processType.getPidFileName());
+statusFile = new File(directory, processType.getStatusFileName());
+statusRequestFile = new File(directory, 
processType.getStatusRequestFileName());
+stopRequestFile = new File(directory, 
processType.getStopRequestFileName());
+pid = ProcessUtils.identifyPid();
+
+params = mock(FileControllerParameters.class);
+when(params.getPidFile()).thenReturn(pidFile);
+when(params.getProcessId()).thenReturn(pid);
+when(params.getProcessType()).thenReturn(processType);
+when(params.getDirectory()).thenReturn(temporaryFolder.getRoot());
+
+executor = Executors.newSingleThreadExecutor();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+assertThat(executor.shutdownNow()).isEmpty();
+  }
+
+  @Test
+  public void statusShouldAwaitTimeoutWhileFileIsEmpty() throws Exception {
+// given: FileProcessController with empty pidFile
+FileProcessController controller 

[GitHub] geode issue #688: GEODE-3397: Fixed issue with Tomcat locators in cache-clie...

[jhuynh1]

Github user jhuynh1 commented on the issue:

https://github.com/apache/geode/pull/688
  
Looks ok to me...I'll merge this in.


---
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 #707: GEODE-3412: Add simple authentication flow to proto...

[WireBaron]

GitHub user WireBaron opened a pull request:

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

GEODE-3412: Add simple authentication flow to protobuf protocol.

@pivotal-amurmann @galen-pivotal @kohlmu-pivotal @bschuchardt @hiteshk25 

This change adds a simple username/password validation to the protobuf 
protocol.
It also adds a new configuration parameter to specify the type of 
authentication required.

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?

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

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

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

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

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

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


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

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

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

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


commit daa140875c4f65838dde4ae0f627a9551bea7516
Author: Brian Rowe 
Date:   2017-08-10T18:16:25Z

GEODE-3412: Add simple authentication flow to protobuf protocol.

This change adds a simple username/password validation to the protobuf 
protocol.
It also adds a new configuration parameter to specify the type of 
authentication required.

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: [DISCUSS] Improvements to backups

[Nick Reich]

Dan, you are correct on #3: there is one location where this appears to not
be the case, but it is unused and thus timestamped directories is currently
implemented and overwrites should not be possible. This therefore also
covers incremental backups and negates the need for change #3. However,
what this means is that incremental backups need to know the timestamped
directory of the last backup. This suggests a different potential
optimization: keeping the (timestamped) incremental backup dirs in a base
directory and either using a metadata file or the timestamps from directory
names to determine the last incremental backup and automatically using that
as the baseline for the current backup (instead of having to (manually)
know what that directory was from the previous backup to use in the current
backup command)

On Thu, Aug 10, 2017 at 10:37 AM, Dan Smith  wrote:

> +1 this all looks good to me. I think #2 in particular would probably
> simplify the incremental backup code.
>
> For #3, I could have sworn the backups were already going into timestamped
> directories and nothing got overwritten in an existing backup. If that is
> not already happening that definitely should change!
>
> -Dan
>
> On Thu, Aug 10, 2017 at 10:31 AM, Nick Reich  wrote:
>
> > There is a desire to improve backup creation and restore. The suggested
> > improvements are listed below and I am seeking feedback from the
> community:
> >
> > 1) Allow saving of backups to different locations/systems: currently,
> > backups are saved to a directory on each member. Users can manually or
> > through scripting move those backups elsewhere, but it would be
> > advantageous to allow direct backups to cloud storage providers (amazon,
> > google, azure, etc.) and possibly other systems. To make this possible,
> it
> > is proposed to refactor backups into a service style architecture with
> > backup location plugins that can be used to specify the target location.
> > This would allow creation of additional backup strategies as demand is
> > determined and allow users to create their own plugins for their own
> > special use cases.
> >
> > 2) Changing backup restore procedure: backups create a restore script per
> > member that must be run from each member to restore a backup to. The
> script
> > created is based on the OS of the machine the backup is created on (it
> > mainly moves files to the correct directories). A more flexible system
> > would be to instead create a metadata file (xml, yaml, etc.) which
> contains
> > information on the files in the backup. This would allow the logic for
> > moving files and other activities in the backup restore process to be
> > maintained in our codebase in an operating system agnostic way. Because
> the
> > existing script is not dependent on geode code, old backups would not be
> > affected by this change, though the process for restoring new backups
> would
> > (likely using gfsh instead of sh or bat scripts).
> >
> > 3) Improved incremental backups: incremental backup allows for
> significant
> > space savings and is much quicker to run. However, it suffers from the
> > problem that you can only restore to the latest time the incremental
> backup
> > was run, as we overwrite user files, cache xml and properties, among
> other
> > files in the backup directory. By saving this information to timestamped
> > directories, restoring to a specific time point would be as simple as
> > choosing the newest point in the backup to include in the restore. Using
> > timestamped directories for normal backups as well would prevent
> successive
> > backups from overwriting each other.
> >
>

Request for Jenkins config ability

[Jens Deppe]

Hi,

I'd like to request the ability to make Jenkins config changes for Geode
builds; I want to start implementing parallel dockerized dunit tests.

Could someone with the appropriate skillz set me up please?

Thanks
--Jens

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[pdxrunner]

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

https://github.com/apache/geode/pull/699#discussion_r132483129
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java
 ---
@@ -14,745 +14,270 @@
  */
 package org.apache.geode.distributed;
 
-import org.apache.geode.distributed.AbstractLauncher.Status;
+import static 
org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
+import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
+import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
+import static 
org.apache.geode.distributed.ConfigurationProperties.DISABLE_AUTO_RECONNECT;
+import static 
org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static 
org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.File;
+import java.net.BindException;
+import java.net.InetAddress;
+import java.util.Collections;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
 import org.apache.geode.distributed.LocatorLauncher.Builder;
 import org.apache.geode.distributed.LocatorLauncher.LocatorState;
 import org.apache.geode.distributed.internal.InternalLocator;
-import org.apache.geode.internal.*;
-import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.GemFireVersion;
 import org.apache.geode.internal.process.ProcessControllerFactory;
 import org.apache.geode.internal.process.ProcessType;
-import org.apache.geode.internal.process.ProcessUtils;
-import org.apache.geode.internal.security.SecurableCommunicationChannel;
 import org.apache.geode.test.junit.categories.IntegrationTest;
-import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Ignore;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-import java.io.File;
-import java.lang.management.ManagementFactory;
-import java.net.BindException;
-import java.net.InetAddress;
-
-import static org.apache.geode.distributed.ConfigurationProperties.*;
-import static org.junit.Assert.*;
 
 /**
- * Tests usage of LocatorLauncher as a local API in existing JVM.
+ * Integration tests for using {@link LocatorLauncher} as an in-process 
API within an existing JVM.
  *
  * @since GemFire 8.0
  */
 @Category(IntegrationTest.class)
-@RunWith(Parameterized.class)

-@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
-public class LocatorLauncherLocalIntegrationTest
-extends AbstractLocatorLauncherIntegrationTestCase {
+public class LocatorLauncherLocalIntegrationTest extends 
LocatorLauncherIntegrationTestCase {
 
   @Before
-  public final void setUpLocatorLauncherLocalIntegrationTest() throws 
Exception {
+  public void setUpLocatorLauncherLocalIntegrationTest() throws Exception {
 disconnectFromDS();
-System.setProperty(ProcessType.TEST_PREFIX_PROPERTY, getUniqueName() + 
"-");
+System.setProperty(ProcessType.PROPERTY_TEST_PREFIX, getUniqueName() + 
"-");
+assertThat(new ProcessControllerFactory().isAttachAPIFound()).isTrue();
   }
 
   @After
-  public final void tearDownLocatorLauncherLocalIntegrationTest() throws 
Exception {
+  public void tearDownLocatorLauncherLocalIntegrationTest() throws 
Exception {
 disconnectFromDS();
   }
 
   @Test
-  public void testBuilderSetProperties() throws Throwable {
-this.launcher = new 
Builder().setForce(true).setMemberName(getUniqueName())
-
.setPort(this.locatorPort).setWorkingDirectory(this.workingDirectory)
-.set(CLUSTER_CONFIGURATION_DIR, this.clusterConfigDirectory)
-.set(DISABLE_AUTO_RECONNECT, "true").set(LOG_LEVEL, 
"config").set(MCAST_PORT, "0").build();
-
-try {
-  assertEquals(Status.ONLINE, this.launcher.start().getStatus());
-  waitForLocatorToStart(this.launcher, true);
-
-  final InternalLocator locator = this.launcher.getLocator();
-  assertNotNull(locator);
-
-  final DistributedSystem distributedSystem = 
locator.getDistributedSystem();
-
-  assertNotNull(distributedSystem);
-  assertEquals("true", 

Review Request 61563: GEODE-3383: Refactor deploy tests

[Jared Stewart]

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

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


Repository: geode


Description
---

GEODE-3383: Refactor deploy tests

- Refactor DeployedJarJUnitTest
- Move several tests into more appropriate locations


Diffs
-

  
geode-core/src/test/java/org/apache/geode/internal/ClassPathLoaderIntegrationTest.java
 34d8a2318422edbb3bbdc04920a41693f8d3fe69 
  geode-core/src/test/java/org/apache/geode/internal/DeployedJarJUnitTest.java 
178dbae2eaada0cc054502fdf4b6d2ff102b4ed6 
  
geode-core/src/test/java/org/apache/geode/internal/JarDeployerDeadlockTest.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/DeployJarTestSuite.java 
6dfab66926c7b246bf839632b293330959f1d728 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
 89148d7752ae1f69e25671ffc43101a63cf7dc64 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpDUnitTest.java
 7753aafbd7dc5ea4288e27f088400163f6739347 


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


Testing
---


Thanks,

Jared Stewart

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[pdxrunner]

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

https://github.com/apache/geode/pull/699#discussion_r132515331
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java ---
@@ -810,9 +819,6 @@ public void setStatus(final String statusMessage) {
 
LocalizedStrings.Launcher_Command_START_PID_UNAVAILABLE_ERROR_MESSAGE.toLocalizedString(
 getServiceName(), getId(), getWorkingDirectory(), 
e.getMessage()),
 e);
-  } catch (ClusterConfigurationNotAvailableException e) {
-failOnStart(e);
-throw e;
   } catch (RuntimeException e) {
 failOnStart(e);
--- End diff --

Opportunity to combine catch blocks?


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[pdxrunner]

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

https://github.com/apache/geode/pull/699#discussion_r132520010
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/process/StartupStatus.java 
---
@@ -14,29 +14,37 @@
  */
 package org.apache.geode.internal.process;
 
+import static org.apache.commons.lang.Validate.notNull;
+
 import org.apache.logging.log4j.Logger;
 
-import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.i18n.StringId;
+import org.apache.geode.internal.logging.LogService;
 
 /**
  * Extracted from LogWriterImpl and changed to static.
- * 
  */
--- End diff --

Is this javadoc necessary?


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[pdxrunner]

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

https://github.com/apache/geode/pull/699#discussion_r132501696
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/mbean/Process.java 
---
@@ -16,7 +16,6 @@
 
 /**
  * Extracted from LocalProcessControllerDUnitTest.
- * 
  */
--- End diff --

This javadoc doesn't seem meaningful.


---
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 #699: GEODE-3413: overhaul launcher and process classes a...

[pdxrunner]

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

https://github.com/apache/geode/pull/699#discussion_r132497014
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/NonBlockingProcessStreamReaderIntegrationTest.java
 ---
@@ -0,0 +1,182 @@
+/*
+ * 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.process;
+
+import static 
org.apache.geode.internal.process.ProcessStreamReader.ReadingMode.NON_BLOCKING;
+import static 
org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.process.ProcessStreamReader.Builder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+/**
+ * Functional integration tests for NonBlockingProcessStreamReader which 
was introduced to fix TRAC
+ * #51967.
+ *
+ * @see BlockingProcessStreamReaderIntegrationTest
+ * @see BlockingProcessStreamReaderWindowsTest
+ *
+ * @since GemFire 8.2
+ */
+@Category(IntegrationTest.class)
+public class NonBlockingProcessStreamReaderIntegrationTest
+extends BaseProcessStreamReaderIntegrationTest {
+
+  private StringBuffer stdoutBuffer;
+  private StringBuffer stderrBuffer;
+
+  @Before
+  public void before() {
+stdoutBuffer = new StringBuffer();
+stderrBuffer = new StringBuffer();
+  }
+
+  /**
+   * This test hangs on Windows if the implementation is blocking instead 
of non-blocking.
+   */
+  @Test
+  public void canCloseStreamsWhileProcessIsAlive() throws Exception {
+// arrange
+process = new 
ProcessBuilder(createCommandLine(ProcessSleeps.class)).start();
+stdout = new 
Builder(process).inputStream(process.getInputStream()).readingMode(NON_BLOCKING)
+.build().start();
+stderr = new 
Builder(process).inputStream(process.getErrorStream()).readingMode(NON_BLOCKING)
--- End diff --

It looks like there may be an opportunity to extract a method from the 
common code from the beginning of each test. Yes, there are some variations on 
setting stdout and stderr, so maybe a couple methods to avoid duplicated 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.
---

Re: [DISCUSS] Improvements to backups

[Dan Smith]

+1 this all looks good to me. I think #2 in particular would probably
simplify the incremental backup code.

For #3, I could have sworn the backups were already going into timestamped
directories and nothing got overwritten in an existing backup. If that is
not already happening that definitely should change!

-Dan

On Thu, Aug 10, 2017 at 10:31 AM, Nick Reich  wrote:

> There is a desire to improve backup creation and restore. The suggested
> improvements are listed below and I am seeking feedback from the community:
>
> 1) Allow saving of backups to different locations/systems: currently,
> backups are saved to a directory on each member. Users can manually or
> through scripting move those backups elsewhere, but it would be
> advantageous to allow direct backups to cloud storage providers (amazon,
> google, azure, etc.) and possibly other systems. To make this possible, it
> is proposed to refactor backups into a service style architecture with
> backup location plugins that can be used to specify the target location.
> This would allow creation of additional backup strategies as demand is
> determined and allow users to create their own plugins for their own
> special use cases.
>
> 2) Changing backup restore procedure: backups create a restore script per
> member that must be run from each member to restore a backup to. The script
> created is based on the OS of the machine the backup is created on (it
> mainly moves files to the correct directories). A more flexible system
> would be to instead create a metadata file (xml, yaml, etc.) which contains
> information on the files in the backup. This would allow the logic for
> moving files and other activities in the backup restore process to be
> maintained in our codebase in an operating system agnostic way. Because the
> existing script is not dependent on geode code, old backups would not be
> affected by this change, though the process for restoring new backups would
> (likely using gfsh instead of sh or bat scripts).
>
> 3) Improved incremental backups: incremental backup allows for significant
> space savings and is much quicker to run. However, it suffers from the
> problem that you can only restore to the latest time the incremental backup
> was run, as we overwrite user files, cache xml and properties, among other
> files in the backup directory. By saving this information to timestamped
> directories, restoring to a specific time point would be as simple as
> choosing the newest point in the backup to include in the restore. Using
> timestamped directories for normal backups as well would prevent successive
> backups from overwriting each other.
>

Build failed in Jenkins: Geode-nightly #919

[Apache Jenkins Server]

See 


Changes:

[jiliao] GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

[khowe] Revert "GEODE-3277: Fix error path constructors of inner State classes

[klund] GEODE-3407: fix deadlock between JMX and Membership

[jiliao] GEODE-3292: Embedded PULSE Connection Failure When

[lhughesgodfrey] GEODE-2226: SessionReplicationIntegrationTests do not run on 
windows

--
[...truncated 461.69 KB...]
:geode-cq:processTestResources
:geode-cq:testClasses
:geode-cq:checkMissedTests
:geode-cq:spotlessJavaCheck
:geode-cq:spotlessCheck
:geode-cq:test
:geode-cq:check
:geode-cq:build
:geode-cq:distributedTest
:geode-cq:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources
:geode-json:testClasses
:geode-json:checkMissedTests UP-TO-DATE
:geode-json:spotlessJavaCheck
:geode-json:spotlessCheck
:geode-json:test UP-TO-DATE
:geode-json:check
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
: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

Review Request 61562: GEODE-3423: Provide support for running parallel docker builds in Jenkins

[Jens Deppe]

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

Review request for geode, Anthony Baker and Mark Bretl.


Repository: geode


Description
---

- Also cleaning up other Dockerfiles which are unused

Signed-off-by: Scott Jewell 


Diffs
-

  dev-tools/docker/base/Dockerfile 1cce0ddb0d29c9e188d27481c82123e357c5b685 
  dev-tools/docker/base/build-base-docker.sh 
9aab72c45d63c519874ad39aaf74c8236d9671ed 
  dev-tools/docker/compile/Dockerfile 6ae343a70eccf7cc4b303228118cee0b8579e79a 
  dev-tools/docker/compile/start-compile-docker.sh 
9059c5b5bbffbadaa82277c05e2189360d94a484 
  gradle/docker.gradle 79719740bc85fd1939d2d7859a7c78c0a87dd26e 


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


Testing
---


Thanks,

Jens Deppe

[DISCUSS] Improvements to backups

[Nick Reich]

There is a desire to improve backup creation and restore. The suggested
improvements are listed below and I am seeking feedback from the community:

1) Allow saving of backups to different locations/systems: currently,
backups are saved to a directory on each member. Users can manually or
through scripting move those backups elsewhere, but it would be
advantageous to allow direct backups to cloud storage providers (amazon,
google, azure, etc.) and possibly other systems. To make this possible, it
is proposed to refactor backups into a service style architecture with
backup location plugins that can be used to specify the target location.
This would allow creation of additional backup strategies as demand is
determined and allow users to create their own plugins for their own
special use cases.

2) Changing backup restore procedure: backups create a restore script per
member that must be run from each member to restore a backup to. The script
created is based on the OS of the machine the backup is created on (it
mainly moves files to the correct directories). A more flexible system
would be to instead create a metadata file (xml, yaml, etc.) which contains
information on the files in the backup. This would allow the logic for
moving files and other activities in the backup restore process to be
maintained in our codebase in an operating system agnostic way. Because the
existing script is not dependent on geode code, old backups would not be
affected by this change, though the process for restoring new backups would
(likely using gfsh instead of sh or bat scripts).

3) Improved incremental backups: incremental backup allows for significant
space savings and is much quicker to run. However, it suffers from the
problem that you can only restore to the latest time the incremental backup
was run, as we overwrite user files, cache xml and properties, among other
files in the backup directory. By saving this information to timestamped
directories, restoring to a specific time point would be as simple as
choosing the newest point in the backup to include in the restore. Using
timestamped directories for normal backups as well would prevent successive
backups from overwriting each other.

[GitHub] geode pull request #705: GEODE-3111 GatewayReceiver - DEFAULT_MANUAL_START v...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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_r132497282
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
 ---
@@ -331,6 +331,29 @@ public void queryJsonObject() throws Exception {
   }
 
   @Test()
+  public void waitUntilFlushThrowsIllegalStateExceptionWhenAEQNotFound() 
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 result = false;
+try {
+  result = luceneService.waitUntilFlushed(nonCreatedIndex, 
REGION_NAME, 6,
+  TimeUnit.MILLISECONDS);
--- End diff --

I think there should be a fail() here,after the all to waitUntilFlushed, if 
we are expecting an exception to be thrown and it doesn't get thrown.


---
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 61506: GEODE-3328: refactor ConnectCommand

[Jinmei Liao]

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

(Updated Aug. 10, 2017, 3:59 p.m.)


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


Changes
---

update the properties file type in the command to be File instead of String


Repository: geode


Description
---

* connect command will prompt for missing ssl configs if ssl is implied.
* command ssl options will override the properties loaded in the file and 
prompt for missing ones if ssl is defined using the ssl-* prefix
* reworked the SSLConfigFactory to not use cached sslConfig when passing in a 
Properties object


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 91a6443b8fd98874feb9f17cf15b36826d7982c3 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 d4068910779c93b800d795d7f31f49a722ce6576 
  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 4b98617465d282fd9ff50cf551a66d4359b4111d 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38e4f893b0435d99b2192256fb559751b00 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
 16d45bc719817782d84ae7e3da876fb9ed4a77bb 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java
 3e1357dad711c134cc45e5415b132dfdde92d93f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 be556a447d862144e453f69809a2de67ee00b654 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 1aea253ff72dfd2bd89754e862cf222754286a94 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
 5b289a545e2d324bfa516cf1b05f8df641bc8a36 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartMemberUtils.java
 fd953878cdcd425bd1cd756ebe23b484f144c628 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java
 432a065ebb796028f73371e476502edca616369e 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UserInputProperty.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 58c8ef79c70f789d10d0a6e95cb24b510aa014a0 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 a69ce36afa6762890edcfb58f48bd8eddd27be65 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 7ae7c3b0f7d85aae1a8d16ba0fa7d9b80683bdfd 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/GfshConsoleReader.java
 9251d7e127a085e3bdb9bfcffc6c706b50b042dd 
  
geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
 308080919ba7e6e67dc0b77574c659bad2842bce 
  
geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
 31c2469a6ef7d9a1db9f2803b0b5bd9adfb5971c 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 866f4ef43dd1c583df210caa491cdd0ef8a3b84a 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorWithLegacySSLDUnitTest.java
 d7db4897d58d318496c1ba990c792ba94f77c81e 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GfshCommandJUnitTest.java
 67c3f5a212ab51a9f456a5f580c46d06cca7af84 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserInputPropertyTest.java
 PRE-CREATION 
  
geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java
 2cd69ddc619a68fb151e6574bace7418a7d58d10 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandIntegrationTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSSLTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSecurityTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectToLocatorSSLOverHttpTest.java
 e5b8d25ae4349e19c252c4d364d0a3c50edeeb54 


Diff: https://reviews.apache.org/r/61506/diff/4/

Changes: https://reviews.apache.org/r/61506/diff/3-4/


Testing
---

precheckin all Green again


Thanks,

Jinmei Liao

Re: Review Request 61506: GEODE-3328: refactor ConnectCommand

[Jinmei Liao]

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

(Updated Aug. 10, 2017, 3:35 p.m.)


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


Repository: geode


Description
---

* connect command will prompt for missing ssl configs if ssl is implied.
* command ssl options will override the properties loaded in the file and 
prompt for missing ones if ssl is defined using the ssl-* prefix
* reworked the SSLConfigFactory to not use cached sslConfig when passing in a 
Properties object


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 91a6443b8fd98874feb9f17cf15b36826d7982c3 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 d4068910779c93b800d795d7f31f49a722ce6576 
  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 4b98617465d282fd9ff50cf551a66d4359b4111d 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38e4f893b0435d99b2192256fb559751b00 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
 16d45bc719817782d84ae7e3da876fb9ed4a77bb 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java
 3e1357dad711c134cc45e5415b132dfdde92d93f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 be556a447d862144e453f69809a2de67ee00b654 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 1aea253ff72dfd2bd89754e862cf222754286a94 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
 5b289a545e2d324bfa516cf1b05f8df641bc8a36 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartMemberUtils.java
 fd953878cdcd425bd1cd756ebe23b484f144c628 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UserInputProperty.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 58c8ef79c70f789d10d0a6e95cb24b510aa014a0 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 a69ce36afa6762890edcfb58f48bd8eddd27be65 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 7ae7c3b0f7d85aae1a8d16ba0fa7d9b80683bdfd 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/GfshConsoleReader.java
 9251d7e127a085e3bdb9bfcffc6c706b50b042dd 
  
geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
 308080919ba7e6e67dc0b77574c659bad2842bce 
  
geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
 31c2469a6ef7d9a1db9f2803b0b5bd9adfb5971c 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 866f4ef43dd1c583df210caa491cdd0ef8a3b84a 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorWithLegacySSLDUnitTest.java
 d7db4897d58d318496c1ba990c792ba94f77c81e 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GfshCommandJUnitTest.java
 67c3f5a212ab51a9f456a5f580c46d06cca7af84 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserInputPropertyTest.java
 PRE-CREATION 
  
geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java
 2cd69ddc619a68fb151e6574bace7418a7d58d10 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandIntegrationTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSSLTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSecurityTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectToLocatorSSLOverHttpTest.java
 e5b8d25ae4349e19c252c4d364d0a3c50edeeb54 


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


Testing (updated)
---

precheckin all Green again


Thanks,

Jinmei Liao

[GitHub] geode-native pull request #114: GEODE-2729: Remove global variables

[dgkimura]

Github user dgkimura closed the pull request at:

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


---
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 #114: GEODE-2729: Remove global variables

[pivotal-jbarrett]

Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/114
  
Please fix up commit messages 


---
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 #529: GEODE-2980: All the methods in an interface are implicitly...

[davinash]

Github user davinash commented on the issue:

https://github.com/apache/geode/pull/529
  
Hi @kirklund Updated the PR and running the tests now again. Please review 
and you can merge the same.


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

[ameybarve15]

Github user ameybarve15 commented on the issue:

https://github.com/apache/geode/pull/609
  
@upthewaterspout , @jhuynh1 
I have updated PR, please review.



---
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 #677: GEODE-3038: A server process shuts down quietly when path ...

[anton-mironenko]

Github user anton-mironenko commented on the issue:

https://github.com/apache/geode/pull/677
  
Hello, is there anything I can do to proceed with this pull request?
I was asked to put RuntimeException instead of CacheXmlException. This is 
what I did. 

This PR contains not-squashed commits, also this PR is done in the main 
stream "develop" in my forked repository on GitHub, instead of being done in 
the proper branch "GEODE-3038".
May be these are the reasons, why my PR stuck? So I could create another PR 
with the only one squashed commit from the right branch.


---
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 61506: GEODE-3328: refactor ConnectCommand

[Jinmei Liao]

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

(Updated Aug. 10, 2017, 6:34 a.m.)


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


Changes
---

when start locator, auto connect will also prompt for missing ssl parameters. 
auto connect and shared config status retriever will use the appropriate 
socketcreator as well.


Repository: geode


Description
---

* connect command will prompt for missing ssl configs if ssl is implied.
* command ssl options will override the properties loaded in the file and 
prompt for missing ones if ssl is defined using the ssl-* prefix
* reworked the SSLConfigFactory to not use cached sslConfig when passing in a 
Properties object


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 91a6443b8fd98874feb9f17cf15b36826d7982c3 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 d4068910779c93b800d795d7f31f49a722ce6576 
  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 4b98617465d282fd9ff50cf551a66d4359b4111d 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38e4f893b0435d99b2192256fb559751b00 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
 16d45bc719817782d84ae7e3da876fb9ed4a77bb 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java
 3e1357dad711c134cc45e5415b132dfdde92d93f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 be556a447d862144e453f69809a2de67ee00b654 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 1aea253ff72dfd2bd89754e862cf222754286a94 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
 5b289a545e2d324bfa516cf1b05f8df641bc8a36 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartMemberUtils.java
 fd953878cdcd425bd1cd756ebe23b484f144c628 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UserInputProperty.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 58c8ef79c70f789d10d0a6e95cb24b510aa014a0 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 a69ce36afa6762890edcfb58f48bd8eddd27be65 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 7ae7c3b0f7d85aae1a8d16ba0fa7d9b80683bdfd 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/GfshConsoleReader.java
 9251d7e127a085e3bdb9bfcffc6c706b50b042dd 
  
geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
 308080919ba7e6e67dc0b77574c659bad2842bce 
  
geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
 31c2469a6ef7d9a1db9f2803b0b5bd9adfb5971c 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 866f4ef43dd1c583df210caa491cdd0ef8a3b84a 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorWithLegacySSLDUnitTest.java
 d7db4897d58d318496c1ba990c792ba94f77c81e 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GfshCommandJUnitTest.java
 67c3f5a212ab51a9f456a5f580c46d06cca7af84 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserInputPropertyTest.java
 PRE-CREATION 
  
geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java
 2cd69ddc619a68fb151e6574bace7418a7d58d10 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandIntegrationTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSSLTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSecurityTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectToLocatorSSLOverHttpTest.java
 e5b8d25ae4349e19c252c4d364d0a3c50edeeb54 


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

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


Testing
---

precheckin all Green


Thanks,

Jinmei Liao