Re: Geode Module Configuration

[Jacob Barrett]

Yes there is an extension point on cache. Just follow the lucene as example but 
put your XML elements inside cache element.

Sent from my iPhone

> On Aug 9, 2017, at 4:38 PM, Dan Smith  wrote:
> 
> There is no global configuration for lucene right now.
> 
> Adding extensions to the cache.xml *looks* like it should be possible.
> InternalCache implements Extensible which means that theoretically
> you should be able to call
> InternalCache.getExtensionPoint.addExtension(yourXmlParser). That whole
> Extensible stuff is really just used for cache.xml parsing and generation.
> 
> The question is just how to add it before the cache actually parses the
> cache.xml file. I think if you implement CacheService and provide the
> appropriate configuration file in your META_INF directory, your service
> should get loaded early in cache creation and get a chance to register
> itself as an Extension.
> 
> -Dan
> 
>> On Wed, Aug 9, 2017 at 4:24 PM, Brian Rowe  wrote:
>> 
>> Hi all,
>> 
>> We are interested in adding some configuration parameters for the new
>> (pre-alpha) client-server protocol that lives in geode-protobuf. We're
>> curious if there is a good way to keep configuration out of geode-core
>> while still allowing users to set it conveniently.
>> 
>> It looks like for geode-lucene, there is some per-region configuration that
>> gets added to cache.xml files. Is there any global configuration?
>> 
>> Thanks,
>> Brian Rowe and Galen O'Sullivan
>> 

[GitHub] geode pull request #706: GEODE-3304: when colocated child bucket is destroye...

[gesterzhou]

GitHub user gesterzhou opened a pull request:

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

GEODE-3304: when colocated child bucket is destroyed, it should not i…

@upthewaterspout @boglesby 

…mpact its

parent bucket's creation.

Thank you for submitting a contribution to Apache Geode.

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

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

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

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

- [ ] Does `gradlew build` run cleanly?

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

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

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


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

$ git pull https://github.com/apache/geode feature/GEODE-3304

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

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


commit 53e27cf21f9c4285d2079ccc7f07fb688e097828
Author: zhouxh 
Date:   2017-08-09T23:55:41Z

GEODE-3304: when colocated child bucket is destroyed, it should not impact 
its
parent bucket's creation.




---
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 #705: GEODE-3111 GatewayReceiver - DEFAULT_MANUAL_START v...

[davebarnes97]

GitHub user davebarnes97 opened a pull request:

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

GEODE-3111 GatewayReceiver - DEFAULT_MANUAL_START value is ambiguous

Updated user guide and javadocs to agree with actual default setting 
(=false).

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

$ git pull https://github.com/davebarnes97/geode feature/GEODE-3111

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

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


commit 61e7c38d3e9daa2da992050d9de9e9ef1f4840e0
Author: Dave Barnes 
Date:   2017-08-09T23:17:14Z

GEODE-3111 GatewayReceiver - DEFAULT_MANUAL_START value is ambiguous




---
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: Geode Module Configuration

[Dan Smith]

There is no global configuration for lucene right now.

Adding extensions to the cache.xml *looks* like it should be possible.
InternalCache implements Extensible which means that theoretically
you should be able to call
InternalCache.getExtensionPoint.addExtension(yourXmlParser). That whole
Extensible stuff is really just used for cache.xml parsing and generation.

The question is just how to add it before the cache actually parses the
cache.xml file. I think if you implement CacheService and provide the
appropriate configuration file in your META_INF directory, your service
should get loaded early in cache creation and get a chance to register
itself as an Extension.

-Dan

On Wed, Aug 9, 2017 at 4:24 PM, Brian Rowe  wrote:

> Hi all,
>
> We are interested in adding some configuration parameters for the new
> (pre-alpha) client-server protocol that lives in geode-protobuf. We're
> curious if there is a good way to keep configuration out of geode-core
> while still allowing users to set it conveniently.
>
> It looks like for geode-lucene, there is some per-region configuration that
> gets added to cache.xml files. Is there any global configuration?
>
> Thanks,
> Brian Rowe and Galen O'Sullivan
>

Re: Geode Module Configuration

[Kirk Lund]

Not yet. I'd like to introduce org.apache.geode.config package and start
ripping out all of the configuration from DistributedSystem and Cache and
move them to something (a Configuration class?) that lives in
org.apache.geode.config. Would this live in a geode-config module...? I
don't think anyone has thought that far ahead yet, but it's certainly not
in place yet..

On Wed, Aug 9, 2017 at 4:24 PM, Brian Rowe  wrote:

> Hi all,
>
> We are interested in adding some configuration parameters for the new
> (pre-alpha) client-server protocol that lives in geode-protobuf. We're
> curious if there is a good way to keep configuration out of geode-core
> while still allowing users to set it conveniently.
>
> It looks like for geode-lucene, there is some per-region configuration that
> gets added to cache.xml files. Is there any global configuration?
>
> Thanks,
> Brian Rowe and Galen O'Sullivan
>

Geode Module Configuration

[Brian Rowe]

Hi all,

We are interested in adding some configuration parameters for the new
(pre-alpha) client-server protocol that lives in geode-protobuf. We're
curious if there is a good way to keep configuration out of geode-core
while still allowing users to set it conveniently.

It looks like for geode-lucene, there is some per-region configuration that
gets added to cache.xml files. Is there any global configuration?

Thanks,
Brian Rowe and Galen O'Sullivan

[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_r132325049
  
--- 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 --

There are several opportunities for collapsing multiple catch blocks in 
this class, such as the try/catch that starts at line 1082


---
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 #704: GEODE-3300: Complete and expose parallel export fea...

[nreich]

GitHub user nreich opened a pull request:

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

GEODE-3300: Complete and expose parallel export feature for use

This change exposes parallel export of snapshots. It provides a filename 
mapper for parallel exports that gives each snapshot file a unique names based 
on the host it was created on. It also enforces to use of the .gfd extension 
for snapshot files and allows for a directory of snapshot files to be imported 
together. Once this change is merged, additional work is required to update 
gfsh to support the commands necessary to allow users to make use of the 
parallel export feature.

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

$ git pull https://github.com/nreich/geode feature/GEODE-3300

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

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


commit 1844570f51fc6e6a1bcbedf5334a629958866426
Author: Nick Reich 
Date:   2017-07-28T23:47:10Z

GEODE-3300: Complete and expose parallel export feature for use




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

[Spring CI]

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

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





--
This message is automatically generated by Atlassian Bamboo

[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_r132326281
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
 ---
@@ -0,0 +1,74 @@
+/*
+ * 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.lang;
--- End diff --

```java
  @Test(timeout = DEFAULT_TIMEOUT_MILLIS)
  public void findAvailablePidShouldReturnRandomPid() throws Exception {
Random random = spy(new Random());
availablePid = new AvailablePid(random, DEFAULT_TIMEOUT_MILLIS);

availablePid.findAvailablePid();

verify(random, atLeastOnce()).nextInt(anyInt());
  }
```


---
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_r132324608
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
 ---
@@ -0,0 +1,74 @@
+/*
+ * 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.lang;
--- End diff --

Oops there it is in the javadocs of the methods.


---
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_r132324550
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
 ---
@@ -0,0 +1,74 @@
+/*
+ * 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.lang;
--- End diff --

Actually, just realized AvailablePid returns unused pids. Nothing in the 
javadoc or API says anything about it being "random" -- that's just how it's 
implemented. So the only really important details is that the pid is unused.


---
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_r132323674
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
 ---
@@ -0,0 +1,74 @@
+/*
+ * 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.lang;
--- End diff --

I'm not sure how to test this. For example, my first idea is to invoke it 
twice and make sure they're not the same but I don't think that's a safe 
assertion. It's also probably not safe to assert that they're not sequential. 
Hmm...


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

[davebarnes97]

GitHub user davebarnes97 opened a pull request:

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

GEODE-3328 Properties to set Key/Trust Store Type for SSL configurati…

…on - add to docs

Added documentation for ssl-keystore-type and ssl-truststore-type to the 
user manual in two locations: Implementing SSL and the Properties Reference. 


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

$ git pull https://github.com/davebarnes97/geode feature/GEODE-3328

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

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


commit e6fe1e69b104c62608f763054c92e89537012857
Author: Dave Barnes 
Date:   2017-08-09T22:22:22Z

GEODE-3328 Properties to set Key/Trust Store Type for SSL configuration - 
add to docs




---
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_r132322055
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 
---
@@ -14,253 +14,370 @@
  */
 package org.apache.geode.distributed;
 
+import static java.util.concurrent.TimeUnit.DAYS;
+import static java.util.concurrent.TimeUnit.HOURS;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static 
org.apache.geode.distributed.AbstractLauncher.ServiceState.toDaysHoursMinutesSeconds;
 import static org.apache.geode.distributed.ConfigurationProperties.NAME;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import java.net.URL;
+import java.util.Properties;
+
 import org.apache.commons.lang.StringUtils;
-import org.apache.geode.test.junit.categories.UnitTest;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.Properties;
-import java.util.concurrent.TimeUnit;
+import org.apache.geode.test.junit.categories.UnitTest;
 
 /**
- * The AbstractLauncherTest class is a test suite of unit tests testing 
the contract and
- * functionality of the AbstractLauncher class.
- * 
- * 
- * @see org.apache.geode.distributed.AbstractLauncher
- * @see org.junit.Assert
- * @see org.junit.Test
+ * Unit tests for {@link AbstractLauncher}.
+ *
  * @since GemFire 7.0
  */
 @Category(UnitTest.class)
 public class AbstractLauncherTest {
 
-  private AbstractLauncher createAbstractLauncher(final String 
memberName,
-  final String memberId) {
-return new FakeServiceLauncher(memberName, memberId);
-  }
-
   @Test
-  public void shouldBeMockable() throws Exception {
+  public void canBeMocked() throws Exception {
 AbstractLauncher mockAbstractLauncher = mock(AbstractLauncher.class);
 mockAbstractLauncher.setDebug(true);
 verify(mockAbstractLauncher, times(1)).setDebug(true);
   }
 
   @Test
-  public void testIsSet() {
-final Properties properties = new Properties();
+  public void isSetReturnsFalseIfPropertyDoesNotExist() throws Exception {
+assertThat(AbstractLauncher.isSet(new Properties(), NAME)).isFalse();
+  }
 
-assertFalse(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+  @Test
+  public void isSetReturnsFalseIfPropertyHasEmptyValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "");
 
-assertTrue(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
+  }
+
+  @Test
+  public void isSetReturnsFalseIfPropertyHasBlankValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "  ");
 
-assertTrue(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
+  }
+
+  @Test
+  public void isSetReturnsFalseIfPropertyHasRealValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "memberOne");
 
-assertTrue(AbstractLauncher.isSet(properties, NAME));
-assertFalse(AbstractLauncher.isSet(properties, "NaMe"));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isTrue();
   }
 
   @Test
-  public void testLoadGemFirePropertiesWithNullURL() {
-final Properties properties = 
AbstractLauncher.loadGemFireProperties(null);
-assertNotNull(properties);
-assertTrue(properties.isEmpty());
+  public void isSetKeyIsCaseSensitive() throws Exception {
+Properties properties = new Properties();
+
+properties.setProperty(NAME, "memberOne");
+
+assertThat(AbstractLauncher.isSet(properties, "NaMe")).isFalse();
   }
 
   @Test
-  public void testLoadGemFirePropertiesWithNonExistingURL() throws 

[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_r132321751
  
--- 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_r132321738
  
--- 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_r132321803
  
--- 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_r132321846
  
--- 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 issue #702: GEODE-3416: Reduce synchronization blockages in SocketClos...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on the issue:

https://github.com/apache/geode/pull/702
  
@bschuchardt @dschneider-pivotal is on the review of 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 #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_r132303567
  
--- 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 --

The bug number reference is not meaningful in the context of the geode 
community. I suggest rewording this to remove the specific reference while 
still describing the the nature of the bug that this test verifies the fix for.


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

Unused field?


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

[GitHub] geode pull request #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_r132314473
  
--- 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 --

Unused fields?


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

[pdxrunner]

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

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

See my other comment regarding specific bug reference.


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

Unused field?


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

Re: [DRAFT] Geode Board Report August 2017

[William Markito Oliveira]

LGTM

On Wed, Aug 9, 2017 at 4:29 PM, Joey McAllister 
wrote:

> Looks good.  Thanks, Dan!
>
> On Wed, Aug 9, 2017 at 2:23 PM Anthony Baker  wrote:
>
> > LGTM
> >
> > > On Aug 9, 2017, at 2:01 PM, Mark Bretl  wrote:
> > >
> > > Looks good to me. Thanks Dan for taking the time to create the draft.
> > >
> > > If there are no issues, then I will post this EOD PST.
> > >
> > > --Mark
> > >
> > > On Mon, Aug 7, 2017 at 2:04 PM, Dan Smith  wrote:
> > >
> > >> Hi folks,
> > >>
> > >> Please review the below draft for our report to the board for this
> past
> > >> quarter and offer corrections. As Mark mentioned, we need to submit
> the
> > >> draft this Wednesday.
> > >>
> > >> Thanks!
> > >> -Dan
> > >>
> > >> -
> > >>
> > >> ## Description:
> > >>
> > >> - Apache Geode provides a database-like consistency model, reliable
> > >> transaction processing and a shared-nothing architecture to maintain
> > very
> > >> low
> > >> latency performance with high concurrency processing.
> > >>
> > >> ## Issues:
> > >>
> > >> - There are no issues requiring board attention at this time.
> > >>
> > >> ## Activity:
> > >>
> > >> - Geode 1.2 was released with over 300 tickets resolved, including
> > >>  - Integration with Apache Lucene is no longer experimental
> > >>  - A built in partition resolver
> > >>  - Examples bundled with the distribution
> > >> - ApacheCon presentation - Building an IoT platform with Apache Apollo
> > and
> > >> Apache Geode by Swapnil Bawaskar
> > >>
> > >> ## Health report:
> > >>
> > >> - We’re continuing to work on attracting new contributors and making
> it
> > >> easier
> > >> to participate in the community.
> > >> - Mailing list activity is healthy.
> > >> - Work has started towards the next Geode release, 1.3.0 including
> > >> development of a new client/server protocol.
> > >>
> > >> ## PMC changes:
> > >>
> > >> - Currently 36 PMC members (+1 since last report)
> > >> - Joey McAllister was added to the PMC on Mon Jul 24 2017
> > >>
> > >> ## Committer base changes:
> > >>
> > >> - Currently 81 committers (+2 since last report)
> > >> - Joey McAllister was added as a committer on Tue Jul 25 2017
> > >> - Deepak Dixit was added as a committer on Thu Jul 13 2017
> > >>
> > >> ## Releases:
> > >>
> > >> - 1.2.0 was released on Wed July 17th 2017
> > >>
> > >> ## Mailing list activity:
> > >>
> > >> - Mailing lists remain active and we’re seeing continued growth in
> > >> subscriber
> > >> counts.  Mailing list traffic on dev went down because we redirected
> > jira
> > >> traffic to iss...@geode.apache.org, but the combined activity
> > increased.
> > >>
> > >> - dev@geode.apache.org:
> > >> - 174 subscribers (up 10 in the last 3 months)
> > >> - 4264 emails sent in the past 3 months, 7166 in the previous cycle
> > >>
> > >> - iss...@geode.apache.org:
> > >> - 56 subscribers (up 1 in the last 3 months)
> > >> - 3183 emails sent in the past 3 months, 0 in the previous cycle
> > >>
> > >> - u...@geode.apache.org:
> > >> - 232 subscribers (up 12 in the last 3 months)
> > >> - 286 emails sent in the past 3 months, 202 in the previous cycle
> > >>
> > >> ## JIRA activity:
> > >>
> > >> - 513 JIRA tickets created in the last 3 months
> > >> - 351 JIRA tickets closed/resolved in the last 3 months
> > >>
> >
> >
>

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

[PurelyApplied]

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

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

Are we really ignoring the exception if we're doing something in response 
to it?
  Ditto lines 272, 438.


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

[PurelyApplied]

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

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

As long as we're touching this, we could consider using `isNotBlank` for 
better readability.


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

[PurelyApplied]

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

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

I'd remove this and the line 510 comments altogether.  If the suggestion 
has value, we should open a ticket for it.

Likewise `LocatorLauncher:382`, `ServerLauncher:523`


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

[PurelyApplied]

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

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

This appears to be unused?  The JavaDoc has it as "NOT USED EXCEPT IN 
TEST."  This is perhaps no longer true?


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

[PurelyApplied]

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

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

Classic Pat nitpick: don't forget to reorder / optimize your imports to go 
`statics`, `java`, `javax`, (other), `org.apache.geode`.

This applies to the following files:
```
geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java

geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java

geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java

geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java

geode-core/src/main/java/org/apache/geode/internal/process/LocalProcessLauncher.java

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java
```


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

Re: [DRAFT] Geode Board Report August 2017

[Joey McAllister]

Looks good.  Thanks, Dan!

On Wed, Aug 9, 2017 at 2:23 PM Anthony Baker  wrote:

> LGTM
>
> > On Aug 9, 2017, at 2:01 PM, Mark Bretl  wrote:
> >
> > Looks good to me. Thanks Dan for taking the time to create the draft.
> >
> > If there are no issues, then I will post this EOD PST.
> >
> > --Mark
> >
> > On Mon, Aug 7, 2017 at 2:04 PM, Dan Smith  wrote:
> >
> >> Hi folks,
> >>
> >> Please review the below draft for our report to the board for this past
> >> quarter and offer corrections. As Mark mentioned, we need to submit the
> >> draft this Wednesday.
> >>
> >> Thanks!
> >> -Dan
> >>
> >> -
> >>
> >> ## Description:
> >>
> >> - Apache Geode provides a database-like consistency model, reliable
> >> transaction processing and a shared-nothing architecture to maintain
> very
> >> low
> >> latency performance with high concurrency processing.
> >>
> >> ## Issues:
> >>
> >> - There are no issues requiring board attention at this time.
> >>
> >> ## Activity:
> >>
> >> - Geode 1.2 was released with over 300 tickets resolved, including
> >>  - Integration with Apache Lucene is no longer experimental
> >>  - A built in partition resolver
> >>  - Examples bundled with the distribution
> >> - ApacheCon presentation - Building an IoT platform with Apache Apollo
> and
> >> Apache Geode by Swapnil Bawaskar
> >>
> >> ## Health report:
> >>
> >> - We’re continuing to work on attracting new contributors and making it
> >> easier
> >> to participate in the community.
> >> - Mailing list activity is healthy.
> >> - Work has started towards the next Geode release, 1.3.0 including
> >> development of a new client/server protocol.
> >>
> >> ## PMC changes:
> >>
> >> - Currently 36 PMC members (+1 since last report)
> >> - Joey McAllister was added to the PMC on Mon Jul 24 2017
> >>
> >> ## Committer base changes:
> >>
> >> - Currently 81 committers (+2 since last report)
> >> - Joey McAllister was added as a committer on Tue Jul 25 2017
> >> - Deepak Dixit was added as a committer on Thu Jul 13 2017
> >>
> >> ## Releases:
> >>
> >> - 1.2.0 was released on Wed July 17th 2017
> >>
> >> ## Mailing list activity:
> >>
> >> - Mailing lists remain active and we’re seeing continued growth in
> >> subscriber
> >> counts.  Mailing list traffic on dev went down because we redirected
> jira
> >> traffic to iss...@geode.apache.org, but the combined activity
> increased.
> >>
> >> - dev@geode.apache.org:
> >> - 174 subscribers (up 10 in the last 3 months)
> >> - 4264 emails sent in the past 3 months, 7166 in the previous cycle
> >>
> >> - iss...@geode.apache.org:
> >> - 56 subscribers (up 1 in the last 3 months)
> >> - 3183 emails sent in the past 3 months, 0 in the previous cycle
> >>
> >> - u...@geode.apache.org:
> >> - 232 subscribers (up 12 in the last 3 months)
> >> - 286 emails sent in the past 3 months, 202 in the previous cycle
> >>
> >> ## JIRA activity:
> >>
> >> - 513 JIRA tickets created in the last 3 months
> >> - 351 JIRA tickets closed/resolved in the last 3 months
> >>
>
>

Re: [DRAFT] Geode Board Report August 2017

[Anthony Baker]

LGTM

> On Aug 9, 2017, at 2:01 PM, Mark Bretl  wrote:
> 
> Looks good to me. Thanks Dan for taking the time to create the draft.
> 
> If there are no issues, then I will post this EOD PST.
> 
> --Mark
> 
> On Mon, Aug 7, 2017 at 2:04 PM, Dan Smith  wrote:
> 
>> Hi folks,
>> 
>> Please review the below draft for our report to the board for this past
>> quarter and offer corrections. As Mark mentioned, we need to submit the
>> draft this Wednesday.
>> 
>> Thanks!
>> -Dan
>> 
>> -
>> 
>> ## Description:
>> 
>> - Apache Geode provides a database-like consistency model, reliable
>> transaction processing and a shared-nothing architecture to maintain very
>> low
>> latency performance with high concurrency processing.
>> 
>> ## Issues:
>> 
>> - There are no issues requiring board attention at this time.
>> 
>> ## Activity:
>> 
>> - Geode 1.2 was released with over 300 tickets resolved, including
>>  - Integration with Apache Lucene is no longer experimental
>>  - A built in partition resolver
>>  - Examples bundled with the distribution
>> - ApacheCon presentation - Building an IoT platform with Apache Apollo and
>> Apache Geode by Swapnil Bawaskar
>> 
>> ## Health report:
>> 
>> - We’re continuing to work on attracting new contributors and making it
>> easier
>> to participate in the community.
>> - Mailing list activity is healthy.
>> - Work has started towards the next Geode release, 1.3.0 including
>> development of a new client/server protocol.
>> 
>> ## PMC changes:
>> 
>> - Currently 36 PMC members (+1 since last report)
>> - Joey McAllister was added to the PMC on Mon Jul 24 2017
>> 
>> ## Committer base changes:
>> 
>> - Currently 81 committers (+2 since last report)
>> - Joey McAllister was added as a committer on Tue Jul 25 2017
>> - Deepak Dixit was added as a committer on Thu Jul 13 2017
>> 
>> ## Releases:
>> 
>> - 1.2.0 was released on Wed July 17th 2017
>> 
>> ## Mailing list activity:
>> 
>> - Mailing lists remain active and we’re seeing continued growth in
>> subscriber
>> counts.  Mailing list traffic on dev went down because we redirected jira
>> traffic to iss...@geode.apache.org, but the combined activity increased.
>> 
>> - dev@geode.apache.org:
>> - 174 subscribers (up 10 in the last 3 months)
>> - 4264 emails sent in the past 3 months, 7166 in the previous cycle
>> 
>> - iss...@geode.apache.org:
>> - 56 subscribers (up 1 in the last 3 months)
>> - 3183 emails sent in the past 3 months, 0 in the previous cycle
>> 
>> - u...@geode.apache.org:
>> - 232 subscribers (up 12 in the last 3 months)
>> - 286 emails sent in the past 3 months, 202 in the previous cycle
>> 
>> ## JIRA activity:
>> 
>> - 513 JIRA tickets created in the last 3 months
>> - 351 JIRA tickets closed/resolved in the last 3 months
>> 

[GitHub] geode issue #702: GEODE-3416: Reduce synchronization blockages in SocketClos...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on the issue:

https://github.com/apache/geode/pull/702
  
@bschuchardt @galen-pivotal @hiteshk25 @pivotal-amurmann 
@dschneider-pivotal @WireBaron @kirklund 


---
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 #702: GEODE-3416: Reduce synchronization blockages in Soc...

[kohlmu-pivotal]

GitHub user kohlmu-pivotal opened a pull request:

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

GEODE-3416: Reduce synchronization blockages in SocketCloser.

Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added

Thank you for submitting a contribution to Apache Geode.

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

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

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

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

- [ ] Does `gradlew build` run cleanly?

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

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

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


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

$ git pull https://github.com/apache/geode feature/GEODE-3416

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

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


commit 56d7707e12ecdecbc1881a9ac1575fac854e8fe9
Author: Udo Kohlmeyer 
Date:   2017-08-09T21:17:59Z

GEODE-3416: Reduce synchronization blockages in SocketCloser.
Remove synchronization blocks around HashMap. Replace that implementation
with simpler ThreadPool that is not unbounded and does not grow as the
number of remoteAddress (clients/peers) are added




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

[GitHub] geode 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_r132308711
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherIntegrationTest.java
 ---
@@ -47,26 +50,21 @@
 
   @Before
   public void setUp() throws Exception {
-this.gemfirePropertiesFile =
-this.temporaryFolder.newFile(DistributionConfig.GEMFIRE_PREFIX + 
"properties");
+propertiesFile = temporaryFolder.newFile(GEMFIRE_PREFIX + 
"properties");
 
-this.expectedGemfireProperties = new Properties();
-this.expectedGemfireProperties.setProperty(NAME, "memberOne");
-this.expectedGemfireProperties.setProperty(GROUPS, "groupOne, 
groupTwo");
-this.expectedGemfireProperties.store(new 
FileWriter(this.gemfirePropertiesFile, false),
-this.testName.getMethodName());
+expectedProperties = new Properties();
+expectedProperties.setProperty(NAME, "memberOne");
+expectedProperties.setProperty(GROUPS, "groupOne, groupTwo");
+expectedProperties.store(new FileWriter(propertiesFile, false), 
testName.getMethodName());
 
-assertThat(this.gemfirePropertiesFile).isNotNull();
-assertThat(this.gemfirePropertiesFile.exists()).isTrue();
-assertThat(this.gemfirePropertiesFile.isFile()).isTrue();
+assertThat(propertiesFile).exists().isFile();
   }
 
   @Test
-  public void testLoadGemFirePropertiesFromFile() throws Exception {
-final Properties actualGemFireProperties =
-
AbstractLauncher.loadGemFireProperties(this.gemfirePropertiesFile.toURI().toURL());
+  public void loadGemFirePropertiesFromFile() throws Exception {
+Properties loadedProperties = 
loadGemFireProperties(propertiesFile.toURI().toURL());
 
-assertThat(actualGemFireProperties).isNotNull();
-
assertThat(actualGemFireProperties).isEqualTo(this.expectedGemfireProperties);
+assertThat(loadedProperties).isEqualTo(expectedProperties);
   }
+
--- End diff --

Removed from all classes in change-set.


---
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_r132308040
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 
---
@@ -14,253 +14,370 @@
  */
 package org.apache.geode.distributed;
 
+import static java.util.concurrent.TimeUnit.DAYS;
+import static java.util.concurrent.TimeUnit.HOURS;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static 
org.apache.geode.distributed.AbstractLauncher.ServiceState.toDaysHoursMinutesSeconds;
 import static org.apache.geode.distributed.ConfigurationProperties.NAME;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import java.net.URL;
+import java.util.Properties;
+
 import org.apache.commons.lang.StringUtils;
-import org.apache.geode.test.junit.categories.UnitTest;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.Properties;
-import java.util.concurrent.TimeUnit;
+import org.apache.geode.test.junit.categories.UnitTest;
 
 /**
- * The AbstractLauncherTest class is a test suite of unit tests testing 
the contract and
- * functionality of the AbstractLauncher class.
- * 
- * 
- * @see org.apache.geode.distributed.AbstractLauncher
- * @see org.junit.Assert
- * @see org.junit.Test
+ * Unit tests for {@link AbstractLauncher}.
+ *
  * @since GemFire 7.0
  */
 @Category(UnitTest.class)
 public class AbstractLauncherTest {
 
-  private AbstractLauncher createAbstractLauncher(final String 
memberName,
-  final String memberId) {
-return new FakeServiceLauncher(memberName, memberId);
-  }
-
   @Test
-  public void shouldBeMockable() throws Exception {
+  public void canBeMocked() throws Exception {
 AbstractLauncher mockAbstractLauncher = mock(AbstractLauncher.class);
 mockAbstractLauncher.setDebug(true);
 verify(mockAbstractLauncher, times(1)).setDebug(true);
   }
 
   @Test
-  public void testIsSet() {
-final Properties properties = new Properties();
+  public void isSetReturnsFalseIfPropertyDoesNotExist() throws Exception {
+assertThat(AbstractLauncher.isSet(new Properties(), NAME)).isFalse();
+  }
 
-assertFalse(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+  @Test
+  public void isSetReturnsFalseIfPropertyHasEmptyValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "");
--- End diff --

Done! I made this change in all launcher and process main classes and test 
classes.


---
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_r132307026
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 
---
@@ -14,253 +14,370 @@
  */
 package org.apache.geode.distributed;
 
+import static java.util.concurrent.TimeUnit.DAYS;
+import static java.util.concurrent.TimeUnit.HOURS;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static 
org.apache.geode.distributed.AbstractLauncher.ServiceState.toDaysHoursMinutesSeconds;
 import static org.apache.geode.distributed.ConfigurationProperties.NAME;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import java.net.URL;
+import java.util.Properties;
+
 import org.apache.commons.lang.StringUtils;
-import org.apache.geode.test.junit.categories.UnitTest;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.Properties;
-import java.util.concurrent.TimeUnit;
+import org.apache.geode.test.junit.categories.UnitTest;
 
 /**
- * The AbstractLauncherTest class is a test suite of unit tests testing 
the contract and
- * functionality of the AbstractLauncher class.
- * 
- * 
- * @see org.apache.geode.distributed.AbstractLauncher
- * @see org.junit.Assert
- * @see org.junit.Test
+ * Unit tests for {@link AbstractLauncher}.
+ *
  * @since GemFire 7.0
  */
 @Category(UnitTest.class)
 public class AbstractLauncherTest {
 
-  private AbstractLauncher createAbstractLauncher(final String 
memberName,
-  final String memberId) {
-return new FakeServiceLauncher(memberName, memberId);
-  }
-
   @Test
-  public void shouldBeMockable() throws Exception {
+  public void canBeMocked() throws Exception {
 AbstractLauncher mockAbstractLauncher = mock(AbstractLauncher.class);
 mockAbstractLauncher.setDebug(true);
 verify(mockAbstractLauncher, times(1)).setDebug(true);
   }
 
   @Test
-  public void testIsSet() {
-final Properties properties = new Properties();
+  public void isSetReturnsFalseIfPropertyDoesNotExist() throws Exception {
+assertThat(AbstractLauncher.isSet(new Properties(), NAME)).isFalse();
+  }
 
-assertFalse(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+  @Test
+  public void isSetReturnsFalseIfPropertyHasEmptyValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "");
 
-assertTrue(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
+  }
+
+  @Test
+  public void isSetReturnsFalseIfPropertyHasBlankValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "  ");
 
-assertTrue(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
+  }
+
+  @Test
+  public void isSetReturnsFalseIfPropertyHasRealValue() throws Exception {
--- End diff --

Nice catch!


---
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 61472: GEODE-3097: fix an accidental bug introduced when working on ssl over http

[Emily Yeh]

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


Ship it!




Ship It!

- Emily Yeh


On Aug. 7, 2017, 5:18 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61472/
> ---
> 
> (Updated Aug. 7, 2017, 5:18 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3097: fix an accidental bug introduced when working on ssl over http
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
>  be556a447d862144e453f69809a2de67ee00b654 
> 
> 
> Diff: https://reviews.apache.org/r/61472/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61409: GEODE-3328: simplify GfshParserRule

[Emily Yeh]

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


Ship it!




Ship It!

- Emily Yeh


On Aug. 3, 2017, 5:12 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61409/
> ---
> 
> (Updated Aug. 3, 2017, 5:12 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3328: simplify GfshParserRule
> 
> another step towards refactor connect command, some simple rule change.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
>  0700742fac70730c160d28c62c93b824e8ee0a3c 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
>  059611dc1c5101643cbae18d067a2943a573d405 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsTest.java
>  c2810801257479ad9a31452f294daaaf2975dfad 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java
>  b1bc7aa73b7d9273ad9a89af4c119d91a67aae03 
> 
> 
> Diff: https://reviews.apache.org/r/61409/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running, tests
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: [DRAFT] Geode Board Report August 2017

[Mark Bretl]

Looks good to me. Thanks Dan for taking the time to create the draft.

If there are no issues, then I will post this EOD PST.

--Mark

On Mon, Aug 7, 2017 at 2:04 PM, Dan Smith  wrote:

> Hi folks,
>
> Please review the below draft for our report to the board for this past
> quarter and offer corrections. As Mark mentioned, we need to submit the
> draft this Wednesday.
>
> Thanks!
> -Dan
>
> -
>
> ## Description:
>
> - Apache Geode provides a database-like consistency model, reliable
> transaction processing and a shared-nothing architecture to maintain very
> low
> latency performance with high concurrency processing.
>
> ## Issues:
>
> - There are no issues requiring board attention at this time.
>
> ## Activity:
>
> - Geode 1.2 was released with over 300 tickets resolved, including
>   - Integration with Apache Lucene is no longer experimental
>   - A built in partition resolver
>   - Examples bundled with the distribution
> - ApacheCon presentation - Building an IoT platform with Apache Apollo and
> Apache Geode by Swapnil Bawaskar
>
> ## Health report:
>
> - We’re continuing to work on attracting new contributors and making it
> easier
> to participate in the community.
> - Mailing list activity is healthy.
> - Work has started towards the next Geode release, 1.3.0 including
> development of a new client/server protocol.
>
> ## PMC changes:
>
> - Currently 36 PMC members (+1 since last report)
> - Joey McAllister was added to the PMC on Mon Jul 24 2017
>
> ## Committer base changes:
>
> - Currently 81 committers (+2 since last report)
> - Joey McAllister was added as a committer on Tue Jul 25 2017
> - Deepak Dixit was added as a committer on Thu Jul 13 2017
>
> ## Releases:
>
> - 1.2.0 was released on Wed July 17th 2017
>
> ## Mailing list activity:
>
> - Mailing lists remain active and we’re seeing continued growth in
> subscriber
> counts.  Mailing list traffic on dev went down because we redirected jira
> traffic to iss...@geode.apache.org, but the combined activity increased.
>
> - dev@geode.apache.org:
>  - 174 subscribers (up 10 in the last 3 months)
>  - 4264 emails sent in the past 3 months, 7166 in the previous cycle
>
> - iss...@geode.apache.org:
>  - 56 subscribers (up 1 in the last 3 months)
>  - 3183 emails sent in the past 3 months, 0 in the previous cycle
>
> - u...@geode.apache.org:
>  - 232 subscribers (up 12 in the last 3 months)
>  - 286 emails sent in the past 3 months, 202 in the previous cycle
>
> ## JIRA activity:
>
> - 513 JIRA tickets created in the last 3 months
> - 351 JIRA tickets closed/resolved in the last 3 months
>

Re: Gitbox enables the full GitHub workflow

[Nabarun Nag]

+1

will this allow us to choose reviewers while creating PRs on github now?

Regards
Nabarun

On Wed, Aug 9, 2017 at 1:19 PM Udo Kohlmeyer  wrote:

> +1
>
>
> On 8/9/17 12:56, Anthony Baker wrote:
> > +1
> >
> >> On Aug 8, 2017, at 12:46 PM, Mark Bretl  wrote:
> >>
> >> +1 for Gitbox
> >>
> >> I have been watching threads on INFRA list and been waiting to see when
> a
> >> good time would be to introduce the idea to the community. The Gitbox
> >> project has been going since the end of 2016 and looks as though it
> might
> >> be ready now as INFRA has begun moving some of their repositories to
> >> GitHub. We still need to 'apply' by submitting a ticket to INFRA and
> wait
> >> for approval, INFRA could still deny the move.
> >>
> >> Advantages:
> >> - GitHub PRs are fully functional (something which stopped us earlier)
> >> - Provides ability to align committer and non-committer workflows
> >> - Removing mirror from Git at Apache to GitHub and thus removing delay
> in
> >> sync
> >>
> >> Disadvantages:
> >> - Permission mapping is configured on id.apache.org (need to set
> GitHub ID)
> >>
> >> I can't think of many disadvantages to moving to GitBox.
> >>
> >> --Mark
> >>
> >>
> >> On Tue, Aug 8, 2017 at 10:56 AM, Dan Smith  wrote:
> >>
> >>> I'm in favor of switching to gitbox. We're already getting pretty much
> all
> >>> contributions from non-committers coming in as pull requests so we
> might as
> >>> well make it easier for us to manage things in github.
> >>>
> >>> -Dan
> >>>
> >>> On Tue, Aug 8, 2017 at 8:57 AM, Kirk Lund  wrote:
> >>>
>  One thing that's given me trouble is that the mirroring of ASF git
> (for
>  Apache Geode) to github can have a lengthy delay. In other words,
> after I
>  commit to ASF git, it's not uncommon for my commit to not show up in
> the
>  GitHub mirror right away and I've seen this delay take an hour.
> 
>  On Mon, Aug 7, 2017 at 6:09 PM, Roman Shaposhnik <
> ro...@shaposhnik.org>
>  wrote:
> 
> > Hi!
> >
> > it has just come to my attention that Gitbox at ASF
> > has been enabling full GitHub workflow (with being
> > able to click Merge this PR button, etc.) for quite
> > some time.
> >
> > This basically allows a project to have GH as a R/W
> > repo as opposed to R/O mirror of what we all currnently
> > have: https://gitbox.apache.org/repos/asf
> >
> > Personally I'm not sure I like GH workflow all that much,
> > but if there's interest -- you can opt-in into Gitbox. Once
> > you do -- your source of truth moves to GH. You can't
> > have it both ways with git-wip-us.apache.org and Gitbox.
> >
> > Thanks,
> > Roman.
> >
>
>

[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_r132299177
  
--- 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 --

Do we want to use org.apache.geode.internal.config instead?


---
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 #701: GEODE-3337: Refactoring LauncherLifecycleCommandsDU...

[YehEmily]

GitHub user YehEmily opened a pull request:

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

GEODE-3337: Refactoring LauncherLifecycleCommandsDUnitTest

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

`LauncherLifecycleCommandsDUnitTest` was a huge test class that tested 
multiple commands, in addition to using the deprecated `CliCommandsTestBase`. 
These tests have been refactored to use more up-to-date test rules and also 
separated into test classes based on the command that they test.

Some tests in the original `LauncherLifecycleCommandsDUnitTest` seemed to 
be unnecessary or redundant (for example, `StatusLocatorRealGfshTest` already 
tested some of the tests in `LauncherLifecycleCommandsDUnitTest`) so these were 
deleted.

*Testing Status: Precheckin in progress*

- [ ] JIRA ticket

- [ ] Rebased

- [ ] Single commit

- [ ] `gradlew build` runs cleanly

- [ ] Tests updated

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

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

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

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


commit c4cdc531c41502d6a162715e324380b66d95794e
Author: YehEmily 
Date:   2017-08-09T17:45:43Z

GEODE-3337: Refactoring LauncherLifecycleCommandsDUnitTest




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

[Bruce Schuchardt]

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

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

[pdxrunner]

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

https://github.com/apache/geode/pull/699#discussion_r132301005
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
 ---
@@ -0,0 +1,74 @@
+/*
+ * 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.lang;
--- End diff --

Ideally there would be a test for 'findAvailablePid(existing processPid)' 
returns a random pid. 


---
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: Gitbox enables the full GitHub workflow

[Udo Kohlmeyer]

+1


On 8/9/17 12:56, Anthony Baker wrote:

+1


On Aug 8, 2017, at 12:46 PM, Mark Bretl  wrote:

+1 for Gitbox

I have been watching threads on INFRA list and been waiting to see when a
good time would be to introduce the idea to the community. The Gitbox
project has been going since the end of 2016 and looks as though it might
be ready now as INFRA has begun moving some of their repositories to
GitHub. We still need to 'apply' by submitting a ticket to INFRA and wait
for approval, INFRA could still deny the move.

Advantages:
- GitHub PRs are fully functional (something which stopped us earlier)
- Provides ability to align committer and non-committer workflows
- Removing mirror from Git at Apache to GitHub and thus removing delay in
sync

Disadvantages:
- Permission mapping is configured on id.apache.org (need to set GitHub ID)

I can't think of many disadvantages to moving to GitBox.

--Mark


On Tue, Aug 8, 2017 at 10:56 AM, Dan Smith  wrote:


I'm in favor of switching to gitbox. We're already getting pretty much all
contributions from non-committers coming in as pull requests so we might as
well make it easier for us to manage things in github.

-Dan

On Tue, Aug 8, 2017 at 8:57 AM, Kirk Lund  wrote:


One thing that's given me trouble is that the mirroring of ASF git (for
Apache Geode) to github can have a lengthy delay. In other words, after I
commit to ASF git, it's not uncommon for my commit to not show up in the
GitHub mirror right away and I've seen this delay take an hour.

On Mon, Aug 7, 2017 at 6:09 PM, Roman Shaposhnik 
wrote:


Hi!

it has just come to my attention that Gitbox at ASF
has been enabling full GitHub workflow (with being
able to click Merge this PR button, etc.) for quite
some time.

This basically allows a project to have GH as a R/W
repo as opposed to R/O mirror of what we all currnently
have: https://gitbox.apache.org/repos/asf

Personally I'm not sure I like GH workflow all that much,
but if there's interest -- you can opt-in into Gitbox. Once
you do -- your source of truth moves to GH. You can't
have it both ways with git-wip-us.apache.org and Gitbox.

Thanks,
Roman.

Re: Gitbox enables the full GitHub workflow

[Anthony Baker]

+1

> On Aug 8, 2017, at 12:46 PM, Mark Bretl  wrote:
> 
> +1 for Gitbox
> 
> I have been watching threads on INFRA list and been waiting to see when a
> good time would be to introduce the idea to the community. The Gitbox
> project has been going since the end of 2016 and looks as though it might
> be ready now as INFRA has begun moving some of their repositories to
> GitHub. We still need to 'apply' by submitting a ticket to INFRA and wait
> for approval, INFRA could still deny the move.
> 
> Advantages:
> - GitHub PRs are fully functional (something which stopped us earlier)
> - Provides ability to align committer and non-committer workflows
> - Removing mirror from Git at Apache to GitHub and thus removing delay in
> sync
> 
> Disadvantages:
> - Permission mapping is configured on id.apache.org (need to set GitHub ID)
> 
> I can't think of many disadvantages to moving to GitBox.
> 
> --Mark
> 
> 
> On Tue, Aug 8, 2017 at 10:56 AM, Dan Smith  wrote:
> 
>> I'm in favor of switching to gitbox. We're already getting pretty much all
>> contributions from non-committers coming in as pull requests so we might as
>> well make it easier for us to manage things in github.
>> 
>> -Dan
>> 
>> On Tue, Aug 8, 2017 at 8:57 AM, Kirk Lund  wrote:
>> 
>>> One thing that's given me trouble is that the mirroring of ASF git (for
>>> Apache Geode) to github can have a lengthy delay. In other words, after I
>>> commit to ASF git, it's not uncommon for my commit to not show up in the
>>> GitHub mirror right away and I've seen this delay take an hour.
>>> 
>>> On Mon, Aug 7, 2017 at 6:09 PM, Roman Shaposhnik 
>>> wrote:
>>> 
 Hi!
 
 it has just come to my attention that Gitbox at ASF
 has been enabling full GitHub workflow (with being
 able to click Merge this PR button, etc.) for quite
 some time.
 
 This basically allows a project to have GH as a R/W
 repo as opposed to R/O mirror of what we all currnently
 have: https://gitbox.apache.org/repos/asf
 
 Personally I'm not sure I like GH workflow all that much,
 but if there's interest -- you can opt-in into Gitbox. Once
 you do -- your source of truth moves to GH. You can't
 have it both ways with git-wip-us.apache.org and Gitbox.
 
 Thanks,
 Roman.
 
>>> 
>> 

Re: Ability to push to dockerhub

[Anthony Baker]

done

> On Aug 9, 2017, at 12:40 PM, Jens Deppe  wrote:
> 
> Hi,
> 
> I'd like the ability to push to the 'apachegeode' dockerhub repository in
> order to publish various docker images used for building Geode. My
> dockerhub account is 'jensdeppe'.
> 
> 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_r132285291
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 
---
@@ -14,253 +14,370 @@
  */
 package org.apache.geode.distributed;
 
+import static java.util.concurrent.TimeUnit.DAYS;
+import static java.util.concurrent.TimeUnit.HOURS;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static 
org.apache.geode.distributed.AbstractLauncher.ServiceState.toDaysHoursMinutesSeconds;
 import static org.apache.geode.distributed.ConfigurationProperties.NAME;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import java.net.URL;
+import java.util.Properties;
+
 import org.apache.commons.lang.StringUtils;
-import org.apache.geode.test.junit.categories.UnitTest;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.Properties;
-import java.util.concurrent.TimeUnit;
+import org.apache.geode.test.junit.categories.UnitTest;
 
 /**
- * The AbstractLauncherTest class is a test suite of unit tests testing 
the contract and
- * functionality of the AbstractLauncher class.
- * 
- * 
- * @see org.apache.geode.distributed.AbstractLauncher
- * @see org.junit.Assert
- * @see org.junit.Test
+ * Unit tests for {@link AbstractLauncher}.
+ *
  * @since GemFire 7.0
  */
 @Category(UnitTest.class)
 public class AbstractLauncherTest {
 
-  private AbstractLauncher createAbstractLauncher(final String 
memberName,
-  final String memberId) {
-return new FakeServiceLauncher(memberName, memberId);
-  }
-
   @Test
-  public void shouldBeMockable() throws Exception {
+  public void canBeMocked() throws Exception {
 AbstractLauncher mockAbstractLauncher = mock(AbstractLauncher.class);
 mockAbstractLauncher.setDebug(true);
 verify(mockAbstractLauncher, times(1)).setDebug(true);
   }
 
   @Test
-  public void testIsSet() {
-final Properties properties = new Properties();
+  public void isSetReturnsFalseIfPropertyDoesNotExist() throws Exception {
+assertThat(AbstractLauncher.isSet(new Properties(), NAME)).isFalse();
+  }
 
-assertFalse(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+  @Test
+  public void isSetReturnsFalseIfPropertyHasEmptyValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "");
 
-assertTrue(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
+  }
+
+  @Test
+  public void isSetReturnsFalseIfPropertyHasBlankValue() throws Exception {
+Properties properties = new Properties();
 
 properties.setProperty(NAME, "  ");
 
-assertTrue(properties.containsKey(NAME));
-assertFalse(AbstractLauncher.isSet(properties, NAME));
+assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
+  }
+
+  @Test
+  public void isSetReturnsFalseIfPropertyHasRealValue() throws Exception {
--- End diff --

Shouldn't this test name be 'isSetReturnsTrueIfPropertyHasRealValue'


---
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_r132281558
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherIntegrationTest.java
 ---
@@ -47,26 +50,21 @@
 
   @Before
   public void setUp() throws Exception {
-this.gemfirePropertiesFile =
-this.temporaryFolder.newFile(DistributionConfig.GEMFIRE_PREFIX + 
"properties");
+propertiesFile = temporaryFolder.newFile(GEMFIRE_PREFIX + 
"properties");
 
-this.expectedGemfireProperties = new Properties();
-this.expectedGemfireProperties.setProperty(NAME, "memberOne");
-this.expectedGemfireProperties.setProperty(GROUPS, "groupOne, 
groupTwo");
-this.expectedGemfireProperties.store(new 
FileWriter(this.gemfirePropertiesFile, false),
-this.testName.getMethodName());
+expectedProperties = new Properties();
+expectedProperties.setProperty(NAME, "memberOne");
+expectedProperties.setProperty(GROUPS, "groupOne, groupTwo");
+expectedProperties.store(new FileWriter(propertiesFile, false), 
testName.getMethodName());
 
-assertThat(this.gemfirePropertiesFile).isNotNull();
-assertThat(this.gemfirePropertiesFile.exists()).isTrue();
-assertThat(this.gemfirePropertiesFile.isFile()).isTrue();
+assertThat(propertiesFile).exists().isFile();
   }
 
   @Test
-  public void testLoadGemFirePropertiesFromFile() throws Exception {
-final Properties actualGemFireProperties =
-
AbstractLauncher.loadGemFireProperties(this.gemfirePropertiesFile.toURI().toURL());
+  public void loadGemFirePropertiesFromFile() throws Exception {
+Properties loadedProperties = 
loadGemFireProperties(propertiesFile.toURI().toURL());
 
-assertThat(actualGemFireProperties).isNotNull();
-
assertThat(actualGemFireProperties).isEqualTo(this.expectedGemfireProperties);
+assertThat(loadedProperties).isEqualTo(expectedProperties);
   }
+
--- End diff --

nit-pick: unnecessary blank line 


---
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. 9, 2017, 7:02 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/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/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/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/2/


Testing (updated)
---

precheckin all Green


Thanks,

Jinmei Liao

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

[pivotal-amurmann]

GitHub user pivotal-amurmann opened a pull request:

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

GEODE-3386 - Make KeyedErrorResponse & ErrorResponse siblings

Thank you for submitting a contribution to Apache Geode.

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

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

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

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

- [ ] 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)?
N/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/pivotal-amurmann/geode feature/GEODE-3386

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

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


commit 81d7a34ef695738c61bf3d288b9ff01f054c30f5
Author: Alexander Murmann 
Date:   2017-08-09T18:52:17Z

GEODE-3386 - Make KeyedErrorResponse & ErrorResponse siblings




---
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 opened a pull request:

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

GEODE-3413: overhaul launcher and process classes and tests

This is primarily an overall of all ServerLauncher and LocatorLauncher
tests and org.apache.geode.internal.process tests. The main classes in
org.apachage.geode.internal.process package are also cleaned up.

In addition, several bugs involving these classes and tests are fixed.

Here is the complete list of tickets that are resolved in this overhaul:

* GEODE-1229: LocatorLauncherRemoteJUnitTest.testStartOverwritesStalePidFile
* GEODE-2791: 
LocatorLauncherAssemblyIntegrationTest.testLocatorStopsWhenJmxPortIsNonZero 
fails intermittently with AssertionError
* GEODE-1308: CI failure: 
LocatorLauncherTest.testSetBindAddressToNonLocalHost
* GEODE-1309: CI failure: 
ServerLauncherTest.testSetServerBindAddressToNonLocalHost
* GEODE-3193: locator pid file is removed even if there was a problem while 
shutting down
* GEODE-3413: Overhaul launcher tests and process tests
* GEODE-3414: Cleanup org.apache.geode.internal.process package

Note I moved all useful tests from LocatorLauncherAssemblyIntegrationTest
into the other launcher tests in geode-core.


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

$ git pull https://github.com/kirklund/geode GEODE-3183-3413-3414

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

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


commit 69cdd5cbc7423577b0a5fcde5f8da486437fa387
Author: Kirk Lund 
Date:   2017-07-10T19:30:09Z

GEODE-3413: overhaul launcher and process classes and tests

This is primarily an overall of all ServerLauncher and LocatorLauncher
tests and org.apache.geode.internal.process tests. The main classes in
org.apachage.geode.internal.process package are also cleaned up.

In addition, several bugs involving these classes and tests are fixed.

Here is the complete list of tickets that are resolved in this overhaul:

* GEODE-1229: LocatorLauncherRemoteJUnitTest.testStartOverwritesStalePidFile
* GEODE-2791: 
LocatorLauncherAssemblyIntegrationTest.testLocatorStopsWhenJmxPortIsNonZero 
fails intermittently with AssertionError
* GEODE-1308: CI failure: 
LocatorLauncherTest.testSetBindAddressToNonLocalHost
* GEODE-1309: CI failure: 
ServerLauncherTest.testSetServerBindAddressToNonLocalHost
* GEODE-3193: locator pid file is removed even if there was a problem while 
shutting down
* GEODE-3413: Overhaul launcher tests and process tests
* GEODE-3414: Cleanup org.apache.geode.internal.process package

Note I moved all useful tests from LocatorLauncherAssemblyIntegrationTest
into the other launcher tests in geode-core.




---
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: Geode Nightly Build with Docker

[Dan Smith]

I don't think this is a port conflict issue. I think this is a matter of
what our code does when we are running on an ubuntu machine with the
default configuration. Ubuntu puts an entry in /etc/hosts mapping 127.0.1.1
to the local hostname.

We saw this yesterday and Bruce was going to create a couple of JIRAs work
on fixing our code. Bruce, did you get a chance to do that yet?

I suppose we could run inside a centos container and maybe avoid these
issues, but then users will hit this when they run on ubuntu.

-Dan

On Wed, Aug 9, 2017 at 11:08 AM, Kirk Lund  wrote:

> Would changing Geode Nightly Build to run inside Docker solve network bind
> failures like we hit last night?
>
> org.apache.geode.cache.wan.GatewayReceiverAutoConnectionSourceDUnitTest >
> testBridgeServerAndGatewayReceiverClientAndServerWithGroup FAILED
> java.lang.AssertionError: Suspicious strings were written to the log
> during this run.
> Fix the strings or use IgnoredException.addIgnoredException to ignore.
> 
> ---
> Found suspect string in log4j at line 200
>
> [error 2017/08/08 20:56:52.894 UTC 
> tid=0x13] failed setting interface to /127.0.1.1:
> java.net.SocketException:
> bad argument for IP_MULTICAST_IF: address not bound to any interface
> java.net.SocketException: bad argument for IP_MULTICAST_IF: address not
> bound to any interface
> at java.net.PlainDatagramSocketImpl.socketSetOption0(Native
> Method)
> at
> java.net.PlainDatagramSocketImpl.socketSetOption(
> PlainDatagramSocketImpl.java:74)
> at
> java.net.AbstractPlainDatagramSocketImpl.setOption(
> AbstractPlainDatagramSocketImpl.java:309)
> at java.net.MulticastSocket.setInterface(MulticastSocket.java:471)
> at org.jgroups.protocols.UDP.setInterface(UDP.java:443)
> at org.jgroups.protocols.UDP.createMulticastSocket(UDP.java:511)
> at
> org.jgroups.protocols.UDP.createMulticastSocketWithBindPort(UDP.java:494)
> at org.jgroups.protocols.UDP.createSockets(UDP.java:348)
> at org.jgroups.protocols.UDP.start(UDP.java:266)
> at
> org.jgroups.stack.ProtocolStack.startStack(ProtocolStack.java:966)
> at org.jgroups.JChannel.startStack(JChannel.java:889)
> at org.jgroups.JChannel._preConnect(JChannel.java:553)
> at org.jgroups.JChannel.connect(JChannel.java:288)
> at org.jgroups.JChannel.connect(JChannel.java:279)
> at
> org.apache.geode.distributed.internal.membership.gms.
> messenger.JGroupsMessenger.start(JGroupsMessenger.java:344)
>

[GitHub] geode pull request #664: Fix for GEODE-3292

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 61426: GEODE-3277: Fix error path constructors of inner State classes of the Launchers

[Jinmei Liao]

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




geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
Lines 41 (patched)


this rule is not used.


- Jinmei Liao


On Aug. 4, 2017, 2:55 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61426/
> ---
> 
> (Updated Aug. 4, 2017, 2:55 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, 
> and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Updated tests for changes in the error constructors for ServerState and
> LocatorState.
> 
> Minor refactorings and spelling corrections to improve code clarity.
> 
> Some error results from gfsh 'status locator' and 'server loccator' commands 
> returned a messge of "null", or in some cases woudl throw a 
> NumberFormatException. These results were due to constructors for 
> LocatorLauncher.LocatorState and ServerLauncher.ServerState filling relevant 
> fields with nulls. This change sets the fields in the ...State instances to 
> values available through the command being executed. The affect constructors 
> are only used for building error results for the status commands.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
>  3a98373938e3de21da6badcf460dae3648218ac6 
>   geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 
> c5a2de88086e92dfc9b35d764b88ff8c8e524853 
>   geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
> 158e7bf45a3cb72f6b96345810b935096b44ee7e 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/lifecycle/StatusLocatorCommand.java
>  06f835034c32e7c6cc7a11d9657d8b5d40d0f2d8 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/lifecycle/StatusServerCommand.java
>  43374ab161b67357d2f8b2987d7656156cbc12c1 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
>  47e512a2dc3a8780dd941af8309865c4f1dbf36f 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/lifecycle/GfshStatusCommandsIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
>  e7f17ef208a1708f385c7c4041affb70fd309a4c 
> 
> 
> Diff: https://reviews.apache.org/r/61426/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin ran green
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Re: Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

[Jinmei Liao]

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




geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
Line 293 (original), 293 (patched)


should we use GemfireCacheImpl.getExisting(), which is not synchronized?


- Jinmei Liao


On Aug. 8, 2017, 4:38 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61487/
> ---
> 
> (Updated Aug. 8, 2017, 4:38 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3407
> https://issues.apache.org/jira/browse/GEODE-3407
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Change InternalClientMembership to not synchronize on CacheFactory
> by accepting Cache parameter from CacheServerBridge.
> 
> New regression test confirms bug and this fix.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
>  504081d65515adb294dd43ecffee450477f08339 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
>  728402c8a290d88026db753657e26e5f7440a990 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java
>  003a8f3cca4ff8fab031bdd84e0e360a14334f6b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
>  6834998da13deec5e074a61e5373ec2f8dce2ad7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61487/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

[GitHub] geode pull request #697: GEODE-3407: fix deadlock between JMX and reconnect

[asfgit]

Github user asfgit closed the pull request at:

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


---
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: continuous query internal mechanism

[Roi Apelker]

Dhanyavad

-Original Message-
From: Anilkumar Gingade [mailto:aging...@pivotal.io] 
Sent: Tuesday, August 08, 2017 9:55 PM
To: dev@geode.apache.org
Subject: Re: continuous query internal mechanism

Registered events, i meant, are events generated for interest registration 
"region.registerInterest(*)". And CqEvents are for CQs registered.

-Anil.


On Tue, Aug 8, 2017 at 12:27 AM, Roi Apelker  wrote:

> Shukriya
>
> What is the difference between registered events and CQ events?
>
> -Original Message-
> From: Anilkumar Gingade [mailto:aging...@pivotal.io]
> Sent: Monday, August 07, 2017 10:12 PM
> To: dev@geode.apache.org
> Subject: Re: continuous query internal mechanism
>
> CQ Processing on server side is same for all clients (Java, C++)...
>
> The subscription events are sent to client as ClientUpdateMessage, 
> which holds information about registered events and CQ events. The 
> client process this and updates/invokes the client side 
> cache/listeners with respective event. Look into 
> ClientUpdateMessageImpl and CacheClientUpdater (for client side processing).
>
> -Anil.
>
>
>
>
> On Mon, Aug 7, 2017 at 11:01 AM, Roi Apelker 
> wrote:
>
> > Thanks,
> >
> > By the way, is there any difference in the behaviour of the server, 
> > if the client that registered the CQ is a native (C++) client?
> >
> > I have been going over the classes and code for some time and can't 
> > seem to find the actual location where a CQ update/notification is
> sent...
> >
> > It's like CqEventImpl class is never even generated in this scenario.
> >
> > If anyone can help here I would be most grateful :-)
> >
> > Thanks
> >
> > Roi
> >
> >
> >
> > -Original Message-
> > From: Anilkumar Gingade [mailto:aging...@pivotal.io]
> > Sent: Monday, August 07, 2017 8:23 PM
> > To: dev@geode.apache.org
> > Subject: Re: continuous query internal mechanism
> >
> > You can find those in CqServiceImpl.process*()...
> >
> > -Anil.
> >
> >
> > On Mon, Aug 7, 2017 at 9:14 AM, Roi Apelker 
> > wrote:
> >
> > > Hello,
> > >
> > > I am trying to look into the code of the continuous query 
> > > mechanism
> > > - where the GEODE server sends the notification back to the client.
> > >
> > > Can anyone point me to the central classes of continuous query, 
> > > especially to the one that is responsible for the calculation of 
> > > the new data and packing it as a message back to the client?
> > >
> > > Thanks,
> > >
> > > Roi
> > >
> > > This message and the information contained herein is proprietary 
> > > and confidential and subject to the Amdocs policy statement,
> > >
> > > you may review at https://www.amdocs.com/about/email-disclaimer < 
> > > https://www.amdocs.com/about/email-disclaimer>
> > >
> > This message and the information contained herein is proprietary and 
> > confidential and subject to the Amdocs policy statement,
> >
> > you may review at https://www.amdocs.com/about/email-disclaimer < 
> > https://www.amdocs.com/about/email-disclaimer>
> >
> This message and the information contained herein is proprietary and 
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer < 
> https://www.amdocs.com/about/email-disclaimer>
>
This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at https://www.amdocs.com/about/email-disclaimer 

[GitHub] geode issue #664: Fix for GEODE-3292

[jujoramos]

Github user jujoramos commented on the issue:

https://github.com/apache/geode/pull/664
  
@jinmeiliao: thanks for reviewing this :-).
One more quick question and I'll leave you alone: do I need to change the 
status of [GEODE-3292](https://issues.apache.org/jira/browse/GEODE-3292) to 
`RESOLVED`?. Or that's done when the pull request is merged into `develop`?. 
Couldn't find a clear answer from [Code 
Contributions](https://cwiki.apache.org/confluence/display/GEODE/Code+contributions)
 and [Developer 
Workflow](https://cwiki.apache.org/confluence/display/GEODE/Developer+Workflow).
Cheers.


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