[GitHub] geode issue #568: GEODE-290 : Removed deprecated method stopWithPort from Lo...

2017-06-08 Thread ameybarve15
Github user ameybarve15 commented on the issue:

https://github.com/apache/geode/pull/568
  
LGTM


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


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Galen M O'Sullivan
One feature I like about PRs on GitHub that I haven't figured out how to do
on Review Board is breaking up a large changeset (which I would like to
have reviewed together) into multiple commits. It can be a useful way to
tell a story about your changes, but keep the review for them in one place.

I'm +0.5 for using GitHub for all code reviews. It's not open source, but
it's free as in beer (well, sort of), and it works well. Having one system
instead of two would be nice and could help make our lives simpler.

On Thu, Jun 8, 2017 at 2:41 PM, Jacob Barrett  wrote:

> On Thu, Jun 8, 2017 at 2:24 PM Nabarun Nag  wrote:
>
> > Also, IMHO feature branches from which the PRs are created should be in
> our
> > personal fork rather than the main geode git repo.
> >
>
> +1 - I had planned to bring this up in a separate discussion but yes, I
> think all work should happen out of your personal fork as though you are a
> non-committer contributor for consistency. The JIRA issue didn't cross my
> mind because I have a filter to delete all JIRA notifications that aren't
> for new tickets or ticket I am watching.
>
> -Jake
>


Re: Review Request 59850: GEODE-3023: TcpServer thread can be blocked in processRequest

2017-06-08 Thread Galen O'Sullivan

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


Fix it, then Ship it!




I had a couple of minor comments about the tests, but otherwise it looks good.

Oh, and don't forget to run Spotless!


geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java
Lines 82 (patched)


I'm missing the time delay here. Can you show me where it gets delayed?

... ah, it looks like you copied this from `TcpServerJUnitTest`. Would you 
mind renaming this function to something that makes more sense in this context?



geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java
Lines 104 (patched)


If we're expecting the exception as a requirement (and I think we are), we 
can assert that it's true by setting a boolean to false and then setting it to 
true in the catch block.



geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java
Lines 133 (patched)


We can reasonably set a very small timeout here, even less than the timeout 
of the socket (since we're mocking it). That (and setting the timeout to some 
silly number like 31337) would also help us verify that we're mocking correctly.



geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java
Lines 143 (patched)


What's the GemStoneAddition about?


- Galen O'Sullivan


On June 8, 2017, 12:20 a.m., Udo Kohlmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59850/
> ---
> 
> (Updated June 8, 2017, 12:20 a.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Hitesh Khamesra, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Moved the socket.setSoTimeout setting to be before the SSL handshake. This is 
> to avoid the timeout to never be set in the case of a SSLException. Added a 
> test to test that the socket timeout is correctly set upon failure within the 
> SSL configuration and handshake.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  86fe53261 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java
>  7c7a2b376 
>   
> geode-core/src/test/java/org/apache/geode/internal/net/DelaySocketCreator.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59850/diff/2/
> 
> 
> Testing
> ---
> 
> Junit test - done
> precheckin - done
> 
> 
> Thanks,
> 
> Udo Kohlmeyer
> 
>



Re: release 1.2

2017-06-08 Thread Anthony Baker
Sounds good.  Please cherry-pick these changes onto release/1.2.0 when ready.

Anthony


> On Jun 8, 2017, at 3:15 PM, Jared Stewart  wrote:
> 
> I think we probably ought to include GEODE-3045 
>  as well.  
> 
>> On Jun 8, 2017, at 3:12 PM, Kenneth Howe  wrote:
>> 
>> I think we should also include the fix for GEODE-3054 in 1.2. This is a bug 
>> that was introduced in the recent changes to the Gfsh parser, and affects 
>> Windows pathnames specified in options to gfsh commands.
>> 
>> https://issues.apache.org/jira/browse/GEODE-3054 
>>  
>> 
>> The fix is currently in testing.
>> 
>> 
>>> On Jun 8, 2017, at 2:23 PM, Bruce Schuchardt  wrote:
>>> 
>>> I'd like to include the fix for GEODE-3052 in the 1.2 release. It's under 
>>> review now and has passed new tests and is in precheckin testing.
>>> 
>>> 
>>> https://issues.apache.org/jira/browse/GEODE-3052
>>> 
>>> "Restarting 2 locators within 1s of each other causes potential locator 
>>> split brain"
>>> 
>> 
> 



[GitHub] geode issue #571: GEODE-2601: Fixing banner being logged twice during locato...

2017-06-08 Thread YehEmily
Github user YehEmily commented on the issue:

https://github.com/apache/geode/pull/571
  
Precheckin is in progress! Some failures so far that also exist on the 
develop branch - will take a look at those soon.


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


[GitHub] geode pull request #571: GEODE-2601: Fixing banner being logged twice during...

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

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

GEODE-2601: Fixing banner being logged twice during locator startup

Now only logs once if both locator and distributed systems are started!
There appear to be some minor changes in various places, possibly due to 
spotless checking; the main changes are in **lines 616-617 of 
`InternalDistributedSystem.java`** and  **lines 483-484 of 
`InternalLocator.java`**. Patrick (@PurelyApplied) also noticed an unnecessary 
`if` statement in **lines 95 & 96 of `LogWriterFactory.java`**, which has been 
fixed in this PR.

Thank you for submitting a contribution to Apache Geode.

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

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

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

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

- [ ] Does `gradlew build` run cleanly?

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

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

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


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

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

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

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


commit d0c07e7bd0d938649ad9b49e2da1bc135145218f
Author: YehEmily 
Date:   2017-06-08T21:40:39Z

GEODE-2601: Fixing banner being logged twice during locator startup (now 
only logs once if both locator and distributed systems are started)




---
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 59925: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

2017-06-08 Thread Galen O'Sullivan


> On June 8, 2017, 11:45 p.m., Galen O'Sullivan wrote:
> > This makes sense to me: we remove the locators if we can't connect to them.
> > 
> > I wonder what happens if the two locators can't talk to each other (at 
> > first, anyways) but can talk to the rest of the cluster. I imagine this is 
> > handled by our view management and as long as the cluster is otherwise 
> > healthy, it will be fine.
> > 
> > As an aside, I'm curious about weight and failure -- do we expire servers 
> > from the weighting for split-brain detection after a while?

I seem to have missed the button: SHIP IT! Looks good!


- Galen


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


On June 8, 2017, 6:36 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59925/
> ---
> 
> (Updated June 8, 2017, 6:36 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When restarting from a locatorView.dat file we should ignore any locator 
> entries in the view.  Recovery tries to get this state from other locators 
> before resorting to using the persisted view so there we know all of the 
> locator entries in the view are invalid.  This allows the locators to quickly 
> move into the concurrent-startup algorithm and find each other.
> 
> I removed the Flaky categorization of the test that I modified to reproduce 
> the problem.  A subclass's use of the test was reported as a Flaky failure 
> but I found that the ticket was closed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  e3635f2d93aae212cbff2f2058b6dc728a04776a 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 8ff9b67e13dd50499d861ff62ddae3fb8668dd28 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  9d49d30abfb8acccd8a5547ba0ee3c7bcf9e7970 
> 
> 
> Diff: https://reviews.apache.org/r/59925/diff/1/
> 
> 
> Testing
> ---
> 
> The problem was easily reproduced using LocatorDUnitTest.testStartTwoLocators 
> by repeating the cycling of the locators.  It failed every time I ran it.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



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

2017-06-08 Thread Udo Kohlmeyer

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


Ship it!




Ship It!

- Udo Kohlmeyer


On June 8, 2017, 6:36 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59925/
> ---
> 
> (Updated June 8, 2017, 6:36 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When restarting from a locatorView.dat file we should ignore any locator 
> entries in the view.  Recovery tries to get this state from other locators 
> before resorting to using the persisted view so there we know all of the 
> locator entries in the view are invalid.  This allows the locators to quickly 
> move into the concurrent-startup algorithm and find each other.
> 
> I removed the Flaky categorization of the test that I modified to reproduce 
> the problem.  A subclass's use of the test was reported as a Flaky failure 
> but I found that the ticket was closed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  e3635f2d93aae212cbff2f2058b6dc728a04776a 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 8ff9b67e13dd50499d861ff62ddae3fb8668dd28 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  9d49d30abfb8acccd8a5547ba0ee3c7bcf9e7970 
> 
> 
> Diff: https://reviews.apache.org/r/59925/diff/1/
> 
> 
> Testing
> ---
> 
> The problem was easily reproduced using LocatorDUnitTest.testStartTwoLocators 
> by repeating the cycling of the locators.  It failed every time I ran it.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



[GitHub] geode issue #565: GEODE-3021: Any call after the first to setPdxStringFlag s...

2017-06-08 Thread jhuynh1
Github user jhuynh1 commented on the issue:

https://github.com/apache/geode/pull/565
  
@boglesby @upthewaterspout @gesterzhou 
Adding additional reviewers


---
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 > #579 was SUCCESSFUL (with 1868 tests)

2017-06-08 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #579 was successful (rerun once).
---
This build was rerun by John Blum.
1870 tests in total.

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





--
This message is automatically generated by Atlassian Bamboo

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #579 has FAILED (1 tests failed)

2017-06-08 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #579 failed.
---
Scheduled
1/1870 tests failed.

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

-
Currently Responsible
-

No one is responsible for fixing this build.



--
Failing Jobs
--
  - Default Job (Default Stage): 1 of 1870 tests failed.




--
Tests
--
New Test Failures (1)
   - GemFireDataSourceUsingNonSpringConfiguredGemFireServerIntegrationTest: 
Client proxy region beans exist

--
This message is automatically generated by Atlassian Bamboo

Re: Review Request 59926: waitUntilFlush should check if its brq's tempQueue is not empty

2017-06-08 Thread Barry Oglesby

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


Ship it!




Ship It!

- Barry Oglesby


On June 8, 2017, 6:43 p.m., xiaojian zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59926/
> ---
> 
> (Updated June 8, 2017, 6:43 p.m.)
> 
> 
> Review request for geode, Barry Oglesby and Dan Smith.
> 
> 
> Bugs: GEODE-3055
> https://issues.apache.org/jira/browse/GEODE-3055
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There's time window that data region bucket is ready, but shadow key's bucket 
> is not. So the event will be added into tempQueue at that windows.
> 
> If we run waitUntilFlush during that window, it did not check the tempQueue 
> since its brq is not exist yet. It will cause data mismatch (i.e. we found 
> the key in data region, but not in index)
> 
> We should pass in the data region's bucket list and let it wait until these 
> tempQueue are empty.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/AsyncEventQueueImpl.java
>  bf7e87445 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
>  c38d5475a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/WaitUntilParallelGatewaySenderFlushedCoordinator.java
>  42ce68cab 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/wan/parallel/WaitUntilParallelGatewaySenderFlushedCoordinatorJUnitTest.java
>  5e12ed5ab 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java
>  e11384c59 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunctionJUnitTest.java
>  f92a296f7 
> 
> 
> Diff: https://reviews.apache.org/r/59926/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>



Re: Review Request 59852: GEODE-2632: make SecurityService immutable to improve client/server performance

2017-06-08 Thread Jared Stewart

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




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


It looks to me like this `securityService` isn't used.



geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
Lines 182 (patched)


It looks like there is a lot of duplication in these methods (which 
basically call authorize with the correct String args) between 
`CustomSecurityService` and `IntegratedSecurityMethods`.  Might be worth 
circling back after this change to abstract these duplicated methods into one 
place.



geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitialization.java
Lines 36 (patched)


It might be nice to change this (in a future ticket) from a hardcoded 
String to  `GeodePermissionResolver.class.getCanonicalName()` so that it would 
be picked up by an IDE if someone refactors the name or package of 
`GeodePermissionResolver`.


- Jared Stewart


On June 8, 2017, 9:22 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59852/
> ---
> 
> (Updated June 8, 2017, 9:22 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-2632
> https://issues.apache.org/jira/browse/GEODE-2632
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> See https://github.com/apache/geode/pull/450 for JMH benchmark 
> ClientCachePutBench. I increased the runtime of ClientCachePutBench from 30 
> minutes to 2 hours to get more accurate measurement. I'll update the "Testing 
> Done" section with performance numbers when that finishes running. My 
> measurements include current develop branch and feature/GEODE-2632-21.
> 
> The changes in this change-set involve making the SecurityService 
> implementation(s) immutable to improve performance of client/server as 
> measured by the JMH benchmark ClientCachePutBench.
> 
> Sorry it's such a long diff again -- I tried to shrink this one down. I could 
> leave out the bulk of client Command classes and tests -- let me know if this 
> review is too big and unwieldy.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
>  45e7a6139b30c800e6096d61d1f23db36c017f99 
>   geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
> 38fdac692315153cda9b4956d0fbbf66fa8f6399 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/ClientCacheFactory.java
>  0d678ca4d08d100ac99266c5d550d9cee7a13ea3 
>   geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 
> e06949c94ab3dd861e9dc64089bec809b3568a6c 
>   geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
> 4de1a95f9acdcdbc843a3412fd773c60910dc43a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  029e6377f56d80dd81e4cec430f106ac743e5178 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  7caad3f33ee55f72dc61c4adc2ab3de5429a1607 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
>  f324e3355e77cc64a8e1cce878b3c03ad4180106 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
>  60249663aad0a59f1d292d0c5336cac33e503204 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
>  bc94ab5afd3e6c9bb5deb9e0beed5f1f84d924fa 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
>  1404b3b6f2a44aeaf3c76b99dad8a428b1b5d1f7 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  ab0ca6b4533a52528818ab3b8059576c0bd1518f 
>   geode-core/src/main/java/org/apache/geode/internal/ClassLoadUtil.java 
> 60d1d3968916697ae1bbac979f6ad5f4a6ad30bd 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java
>  988be0a72b97cf6d67dd7dd930e229f6eb867166 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/CacheServerImpl.java 
> 670c697c7f6c3d22a3c79d10be4e9c9929cd612b 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
> 

Re: Review Request 59852: GEODE-2632: make SecurityService immutable to improve client/server performance

2017-06-08 Thread Jared Stewart

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


Ship it!




Ship It!

- Jared Stewart


On June 8, 2017, 9:22 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59852/
> ---
> 
> (Updated June 8, 2017, 9:22 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-2632
> https://issues.apache.org/jira/browse/GEODE-2632
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> See https://github.com/apache/geode/pull/450 for JMH benchmark 
> ClientCachePutBench. I increased the runtime of ClientCachePutBench from 30 
> minutes to 2 hours to get more accurate measurement. I'll update the "Testing 
> Done" section with performance numbers when that finishes running. My 
> measurements include current develop branch and feature/GEODE-2632-21.
> 
> The changes in this change-set involve making the SecurityService 
> implementation(s) immutable to improve performance of client/server as 
> measured by the JMH benchmark ClientCachePutBench.
> 
> Sorry it's such a long diff again -- I tried to shrink this one down. I could 
> leave out the bulk of client Command classes and tests -- let me know if this 
> review is too big and unwieldy.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
>  45e7a6139b30c800e6096d61d1f23db36c017f99 
>   geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
> 38fdac692315153cda9b4956d0fbbf66fa8f6399 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/ClientCacheFactory.java
>  0d678ca4d08d100ac99266c5d550d9cee7a13ea3 
>   geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 
> e06949c94ab3dd861e9dc64089bec809b3568a6c 
>   geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
> 4de1a95f9acdcdbc843a3412fd773c60910dc43a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  029e6377f56d80dd81e4cec430f106ac743e5178 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  7caad3f33ee55f72dc61c4adc2ab3de5429a1607 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
>  f324e3355e77cc64a8e1cce878b3c03ad4180106 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
>  60249663aad0a59f1d292d0c5336cac33e503204 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
>  bc94ab5afd3e6c9bb5deb9e0beed5f1f84d924fa 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
>  1404b3b6f2a44aeaf3c76b99dad8a428b1b5d1f7 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  ab0ca6b4533a52528818ab3b8059576c0bd1518f 
>   geode-core/src/main/java/org/apache/geode/internal/ClassLoadUtil.java 
> 60d1d3968916697ae1bbac979f6ad5f4a6ad30bd 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java
>  988be0a72b97cf6d67dd7dd930e229f6eb867166 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/CacheServerImpl.java 
> 670c697c7f6c3d22a3c79d10be4e9c9929cd612b 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
> 67fcce85c0f812e062f602e97d604dd39c0edd9f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  5e352241ac114b891adb178e1820e8c74017fa64 
>   geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 
> d9a34e1cfa85cc08ff4f0af81b6adc2bdbd7e869 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
>  037bff6ebd0f49b7afe6b5aad4c0edd9ca3b139a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/FetchEntriesMessage.java
>  489ffbaddabfc2d2a46d87d3060d5c61b76c7f32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Command.java 
> d7f7c7b20372351963dbc6cb1e8bab506ffcc1d0 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  9658f98da808e50b87fe54854df249b28b97662d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
>  1fb8c8cac62c54b582d40688a423fddcae23b36e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
>  

Re: release 1.2

2017-06-08 Thread Jared Stewart
I think we probably ought to include GEODE-3045 
 as well.  

> On Jun 8, 2017, at 3:12 PM, Kenneth Howe  wrote:
> 
> I think we should also include the fix for GEODE-3054 in 1.2. This is a bug 
> that was introduced in the recent changes to the Gfsh parser, and affects 
> Windows pathnames specified in options to gfsh commands.
> 
> https://issues.apache.org/jira/browse/GEODE-3054 
>  
> 
> The fix is currently in testing.
> 
> 
>> On Jun 8, 2017, at 2:23 PM, Bruce Schuchardt  wrote:
>> 
>> I'd like to include the fix for GEODE-3052 in the 1.2 release. It's under 
>> review now and has passed new tests and is in precheckin testing.
>> 
>> 
>> https://issues.apache.org/jira/browse/GEODE-3052
>> 
>> "Restarting 2 locators within 1s of each other causes potential locator 
>> split brain"
>> 
> 



Re: release 1.2

2017-06-08 Thread Kenneth Howe
I think we should also include the fix for GEODE-3054 in 1.2. This is a bug 
that was introduced in the recent changes to the Gfsh parser, and affects 
Windows pathnames specified in options to gfsh commands.

https://issues.apache.org/jira/browse/GEODE-3054 
 

The fix is currently in testing.


> On Jun 8, 2017, at 2:23 PM, Bruce Schuchardt  wrote:
> 
> I'd like to include the fix for GEODE-3052 in the 1.2 release. It's under 
> review now and has passed new tests and is in precheckin testing.
> 
> 
> https://issues.apache.org/jira/browse/GEODE-3052
> 
> "Restarting 2 locators within 1s of each other causes potential locator split 
> brain"
> 



[GitHub] geode pull request #570: GEODE-3055: waitUntilFlush should use data region's...

2017-06-08 Thread gesterzhou
GitHub user gesterzhou opened a pull request:

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

GEODE-3055: waitUntilFlush should use data region's bucketid list. So…

@upthewaterspout @boglesby 

…me of the buckets maybe not initialized, then wait for tempQueue to be 
empty.

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/GEM-1483

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

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


commit e07b513f3c564d9a4f6c69ca47fbddf70bb76563
Author: zhouxh 
Date:   2017-06-08T21:52:56Z

GEODE-3055: waitUntilFlush should use data region's bucketid list. Some of 
the buckets maybe not initialized, then wait for tempQueue to be empty.




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


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Jacob Barrett
On Thu, Jun 8, 2017 at 2:24 PM Nabarun Nag  wrote:

> Also, IMHO feature branches from which the PRs are created should be in our
> personal fork rather than the main geode git repo.
>

+1 - I had planned to bring this up in a separate discussion but yes, I
think all work should happen out of your personal fork as though you are a
non-committer contributor for consistency. The JIRA issue didn't cross my
mind because I have a filter to delete all JIRA notifications that aren't
for new tickets or ticket I am watching.

-Jake


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Nabarun Nag
+1 on using PR.
We can use the @tags and the github notification page[Participating tag] to
check the PRs that need our attention.

Also, IMHO feature branches from which the PRs are created should be in our
personal fork rather than the main geode git repo. Because when we push a
branch called feature/GEODE- into the main apache geode repo, the JIRA
system links the ticket to that branch and every commit/merge operation on
that branch is logged in the comment section of ticket. In my opinion, the
JIRA tickets should have clean and concise history. It should not contain
history / commits of branch that will be deleted eventually , or commits
that will be eventually squashed to a single commit message before merging
to the main develop.

Regards
Naba




On Thu, Jun 8, 2017 at 12:39 PM Jacob Barrett  wrote:

> On Thu, Jun 8, 2017 at 12:35 PM Mark Bretl  wrote:
>
> > I like the functionality we get with GitHub, including the Travis CI
> > integration.
> >
> > Do we have a proposed workflow change for committers?
>
>
> The proposed workflow change to committers is to use the same workflow as
> contributors. The only difference is after sufficient review of the PR you
> could commit it yourself.
>
> -Jake
>


release 1.2

2017-06-08 Thread Bruce Schuchardt
I'd like to include the fix for GEODE-3052 in the 1.2 release. It's 
under review now and has passed new tests and is in precheckin testing.



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

"Restarting 2 locators within 1s of each other causes potential locator 
split brain"




Re: Review Request 59852: GEODE-2632: make SecurityService immutable to improve client/server performance

2017-06-08 Thread Kirk Lund

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

(Updated June 8, 2017, 9:22 p.m.)


Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, and 
Patrick Rhomberg.


Changes
---

Add SecurityTest category to new security tests.


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


Repository: geode


Description
---

See https://github.com/apache/geode/pull/450 for JMH benchmark 
ClientCachePutBench. I increased the runtime of ClientCachePutBench from 30 
minutes to 2 hours to get more accurate measurement. I'll update the "Testing 
Done" section with performance numbers when that finishes running. My 
measurements include current develop branch and feature/GEODE-2632-21.

The changes in this change-set involve making the SecurityService 
implementation(s) immutable to improve performance of client/server as measured 
by the JMH benchmark ClientCachePutBench.

Sorry it's such a long diff again -- I tried to shrink this one down. I could 
leave out the bulk of client Command classes and tests -- let me know if this 
review is too big and unwieldy.


Diffs (updated)
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
 45e7a6139b30c800e6096d61d1f23db36c017f99 
  geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
38fdac692315153cda9b4956d0fbbf66fa8f6399 
  
geode-core/src/main/java/org/apache/geode/cache/client/ClientCacheFactory.java 
0d678ca4d08d100ac99266c5d550d9cee7a13ea3 
  geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 
e06949c94ab3dd861e9dc64089bec809b3568a6c 
  geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
4de1a95f9acdcdbc843a3412fd773c60910dc43a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
 029e6377f56d80dd81e4cec430f106ac743e5178 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 7caad3f33ee55f72dc61c4adc2ab3de5429a1607 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
 f324e3355e77cc64a8e1cce878b3c03ad4180106 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
 60249663aad0a59f1d292d0c5336cac33e503204 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
 bc94ab5afd3e6c9bb5deb9e0beed5f1f84d924fa 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
 1404b3b6f2a44aeaf3c76b99dad8a428b1b5d1f7 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 ab0ca6b4533a52528818ab3b8059576c0bd1518f 
  geode-core/src/main/java/org/apache/geode/internal/ClassLoadUtil.java 
60d1d3968916697ae1bbac979f6ad5f4a6ad30bd 
  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java
 988be0a72b97cf6d67dd7dd930e229f6eb867166 
  geode-core/src/main/java/org/apache/geode/internal/cache/CacheServerImpl.java 
670c697c7f6c3d22a3c79d10be4e9c9929cd612b 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
67fcce85c0f812e062f602e97d604dd39c0edd9f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
5e352241ac114b891adb178e1820e8c74017fa64 
  geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 
d9a34e1cfa85cc08ff4f0af81b6adc2bdbd7e869 
  
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
 037bff6ebd0f49b7afe6b5aad4c0edd9ca3b139a 
  
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/FetchEntriesMessage.java
 489ffbaddabfc2d2a46d87d3060d5c61b76c7f32 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/Command.java 
d7f7c7b20372351963dbc6cb1e8bab506ffcc1d0 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 9658f98da808e50b87fe54854df249b28b97662d 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
 1fb8c8cac62c54b582d40688a423fddcae23b36e 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
 adf702a1d361ba28f4ebe46695325d23344db756 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
 fd5154f9c5bdd516dad2eb93cd78e7b73213255f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxy.java
 8450db905a3834374bd451b55d8720d313d21d2a 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
 388f838e3c906aaba1e6993051f2fcf2ae3c9e82 
  

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

2017-06-08 Thread Kirk Lund

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


Ship it!




Ship it after GEODE-2632 is merged to develop.

- Kirk Lund


On June 5, 2017, 6:32 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 5, 2017, 6:32 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de565a8301168f13e1917ea739a8879162c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  f9fade1cfe8c181b0a0874869a66643c00300f98 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391212095413c0d577cfc65de7247080b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda8437e06de818ead40731818f937c3aef5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e1425d71af9d2ea59046b17afd70ad30dd68 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  6514a33e52611994ddc16a58414146ebaad75c65 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
>   
> geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
>  2d6fbcaeb470c79f383626b8e15e3bd8650377dd 
>   geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
> 6080b5de8c4b11f013d0800baf2a1d9f18cb7f1d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
>  80ff719b015ae0ffb5a648fe026bb01bc6128df8 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/7/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 59754: GEODE-2928: get rid of the isGfshVM static variable

2017-06-08 Thread Kirk Lund

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


Fix it, then Ship it!




Fix it and ship it!


geode-core/src/test/java/org/apache/TestSuite.java
Lines 38 (patched)


You probably want to delete this. If you keep it then move it to a deeper 
package and rename it to something other than TestSuite.


- Kirk Lund


On June 5, 2017, 8:03 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59754/
> ---
> 
> (Updated June 5, 2017, 8:03 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * consolidate the availability indicators
> * remove the isGfshVM and isGfshVM() method
> * enhance the MultiStepCommand to include info on shellOnly commands to 
> enhance command validation
> * remove the SUPPORT_MULTIPLE_GFSH static flag and properly remove the gfsh 
> instance at the end of each test
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/GfeConsoleReaderFactory.java
>  120d6257b 
>   geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java 
> 2e6dc3973 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
>  038e0691e 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ClientCommands.java
>  e61934261 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CommandAvailabilityIndicator.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java
>  bc9c05b81 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  ad40518f8 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  cb9c4fe50 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java
>  30d840a0f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DurableClientCommands.java
>  6441f20cc 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
>  9d263d110 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java
>  2774584ff 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
>  d46024d38 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java
>  b3d96757b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b681 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MemberCommands.java
>  695718a82 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
>  9754d7d52 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/PDXCommands.java
>  9f1290d16 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java
>  d3c263509 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/RegionCommands.java
>  2009dcc05 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  efd10d27b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StatusCommands.java
>  fffb9646f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/WanCommands.java
>  28686ce4d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  e2164a375 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/CLIMultiStepHelper.java
>  d53261d04 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/MultiStepCommand.java
>  6708726cd 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/RemoteExecutionStrategy.java
>  fa0f3b259 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/AbstractResultData.java
>  f453ec67c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  c5ff6b6a5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
>  2b39bedd1 

Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

2017-06-08 Thread Kirk Lund

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


Ship it!




Ship It!

- Kirk Lund


On June 8, 2017, 9:08 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59811/
> ---
> 
> (Updated June 8, 2017, 9:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, 
> and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2420: add file-size-limit param to the ExportLogsController
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
>  0ff780cbf66937d8ececfb3a2d0789ee485b9b62 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
>  57355c0efae4c6da9470267f95e27e59aa4d8b2c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
>  a369c6e1ffb330715fbde2cd69d023ed36f133ad 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
>  16549e70bbebf4390bb73a481274e92ca6cad035 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
>  8609b3aaf0a0eb1ba903bd39c64103f9510a6a78 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
>  09ee08dd6af29b9a418ef7499defc4980da787ed 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
>  44a036298e0991c880fc552596d296e104b97ca1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
>  4e1dac013d239437829bc52dc70689c4ba15dc58 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
>  cc5e7d5256741ad0a48ff87c7f989a18b90f7f03 
> 
> 
> Diff: https://reviews.apache.org/r/59811/diff/4/
> 
> 
> Testing
> ---
> 
> 6/7/17: re-started precheckin
> precheckin is green with the exception of unrelated DUnit tests
> 
> * re-starting precheckin once more
> 
> 6/8/17: previous precheckin green except for 1 known flaky test
> 
> Precheckin started
> 
> 
> Thanks,
> 
> Ken Howe
> 
>



Re: Review Request 59905: GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and startServer

2017-06-08 Thread Kirk Lund

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


Fix it, then Ship it!




Fix the test category on LauncherLifecycleCommandsTest and ship it!


geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
Lines 2370 (patched)


At first I thought this was a HOSTNAME for the client itself. Maybe reword 
to:

"Hostname provided to clients from the locator for the location of a JMX 
Manager."



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


This should be an IntegrationTest since it uses TemporaryFolder.


- Kirk Lund


On June 8, 2017, 5:33 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59905/
> ---
> 
> (Updated June 8, 2017, 5:33 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and 
> startServer
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
>  df16e9b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e14 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  c5ff6b6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
>  a122de0 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java
>  1ff60d6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  ab6dc3d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59905/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passed (other than 1 known flaky)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>



Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

2017-06-08 Thread Ken Howe

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

(Updated June 8, 2017, 9:08 p.m.)


Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, and 
Patrick Rhomberg.


Changes
---

As suggested removed unused imports that crept in while iterating while trying 
out different things during the earlier revision to the warning semantics.


Repository: geode


Description
---

GEODE-2420: add file-size-limit param to the ExportLogsController


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
 0ff780cbf66937d8ececfb3a2d0789ee485b9b62 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
 57355c0efae4c6da9470267f95e27e59aa4d8b2c 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
 a369c6e1ffb330715fbde2cd69d023ed36f133ad 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
 16549e70bbebf4390bb73a481274e92ca6cad035 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
 8609b3aaf0a0eb1ba903bd39c64103f9510a6a78 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
 09ee08dd6af29b9a418ef7499defc4980da787ed 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
 44a036298e0991c880fc552596d296e104b97ca1 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
 4e1dac013d239437829bc52dc70689c4ba15dc58 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
 cc5e7d5256741ad0a48ff87c7f989a18b90f7f03 


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

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


Testing (updated)
---

6/7/17: re-started precheckin
precheckin is green with the exception of unrelated DUnit tests

* re-starting precheckin once more

6/8/17: previous precheckin green except for 1 known flaky test

Precheckin started


Thanks,

Ken Howe



Re: Review Request 59852: GEODE-2632: make SecurityService immutable to improve client/server performance

2017-06-08 Thread Kirk Lund

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

(Updated June 8, 2017, 9:04 p.m.)


Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, and 
Patrick Rhomberg.


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


Repository: geode


Description
---

See https://github.com/apache/geode/pull/450 for JMH benchmark 
ClientCachePutBench. I increased the runtime of ClientCachePutBench from 30 
minutes to 2 hours to get more accurate measurement. I'll update the "Testing 
Done" section with performance numbers when that finishes running. My 
measurements include current develop branch and feature/GEODE-2632-21.

The changes in this change-set involve making the SecurityService 
implementation(s) immutable to improve performance of client/server as measured 
by the JMH benchmark ClientCachePutBench.

Sorry it's such a long diff again -- I tried to shrink this one down. I could 
leave out the bulk of client Command classes and tests -- let me know if this 
review is too big and unwieldy.


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
 45e7a6139b30c800e6096d61d1f23db36c017f99 
  geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
38fdac692315153cda9b4956d0fbbf66fa8f6399 
  
geode-core/src/main/java/org/apache/geode/cache/client/ClientCacheFactory.java 
0d678ca4d08d100ac99266c5d550d9cee7a13ea3 
  geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 
e06949c94ab3dd861e9dc64089bec809b3568a6c 
  geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
4de1a95f9acdcdbc843a3412fd773c60910dc43a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
 029e6377f56d80dd81e4cec430f106ac743e5178 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 7caad3f33ee55f72dc61c4adc2ab3de5429a1607 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
 f324e3355e77cc64a8e1cce878b3c03ad4180106 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
 60249663aad0a59f1d292d0c5336cac33e503204 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
 bc94ab5afd3e6c9bb5deb9e0beed5f1f84d924fa 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
 1404b3b6f2a44aeaf3c76b99dad8a428b1b5d1f7 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 ab0ca6b4533a52528818ab3b8059576c0bd1518f 
  geode-core/src/main/java/org/apache/geode/internal/ClassLoadUtil.java 
60d1d3968916697ae1bbac979f6ad5f4a6ad30bd 
  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java
 988be0a72b97cf6d67dd7dd930e229f6eb867166 
  geode-core/src/main/java/org/apache/geode/internal/cache/CacheServerImpl.java 
670c697c7f6c3d22a3c79d10be4e9c9929cd612b 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
67fcce85c0f812e062f602e97d604dd39c0edd9f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
5e352241ac114b891adb178e1820e8c74017fa64 
  geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 
d9a34e1cfa85cc08ff4f0af81b6adc2bdbd7e869 
  
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
 037bff6ebd0f49b7afe6b5aad4c0edd9ca3b139a 
  
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/FetchEntriesMessage.java
 489ffbaddabfc2d2a46d87d3060d5c61b76c7f32 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/Command.java 
d7f7c7b20372351963dbc6cb1e8bab506ffcc1d0 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 9658f98da808e50b87fe54854df249b28b97662d 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
 1fb8c8cac62c54b582d40688a423fddcae23b36e 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
 adf702a1d361ba28f4ebe46695325d23344db756 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
 fd5154f9c5bdd516dad2eb93cd78e7b73213255f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxy.java
 8450db905a3834374bd451b55d8720d313d21d2a 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
 388f838e3c906aaba1e6993051f2fcf2ae3c9e82 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
 

Re: Review Request 59852: GEODE-2632: make SecurityService immutable to improve client/server performance

2017-06-08 Thread Kirk Lund

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

(Updated June 8, 2017, 9:02 p.m.)


Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, and 
Patrick Rhomberg.


Changes
---

Feedback fixup and extract utility methods from SecurityService to static 
utility classes. Note: I did not cleanup imports in any classes I didn't 
directly edit.


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


Repository: geode


Description
---

See https://github.com/apache/geode/pull/450 for JMH benchmark 
ClientCachePutBench. I increased the runtime of ClientCachePutBench from 30 
minutes to 2 hours to get more accurate measurement. I'll update the "Testing 
Done" section with performance numbers when that finishes running. My 
measurements include current develop branch and feature/GEODE-2632-21.

The changes in this change-set involve making the SecurityService 
implementation(s) immutable to improve performance of client/server as measured 
by the JMH benchmark ClientCachePutBench.

Sorry it's such a long diff again -- I tried to shrink this one down. I could 
leave out the bulk of client Command classes and tests -- let me know if this 
review is too big and unwieldy.


Diffs (updated)
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
 45e7a6139b30c800e6096d61d1f23db36c017f99 
  geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
38fdac692315153cda9b4956d0fbbf66fa8f6399 
  
geode-core/src/main/java/org/apache/geode/cache/client/ClientCacheFactory.java 
0d678ca4d08d100ac99266c5d550d9cee7a13ea3 
  geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java 
e06949c94ab3dd861e9dc64089bec809b3568a6c 
  geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
4de1a95f9acdcdbc843a3412fd773c60910dc43a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
 029e6377f56d80dd81e4cec430f106ac743e5178 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 7caad3f33ee55f72dc61c4adc2ab3de5429a1607 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
 f324e3355e77cc64a8e1cce878b3c03ad4180106 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
 60249663aad0a59f1d292d0c5336cac33e503204 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
 bc94ab5afd3e6c9bb5deb9e0beed5f1f84d924fa 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
 1404b3b6f2a44aeaf3c76b99dad8a428b1b5d1f7 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 ab0ca6b4533a52528818ab3b8059576c0bd1518f 
  geode-core/src/main/java/org/apache/geode/internal/ClassLoadUtil.java 
60d1d3968916697ae1bbac979f6ad5f4a6ad30bd 
  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java
 988be0a72b97cf6d67dd7dd930e229f6eb867166 
  geode-core/src/main/java/org/apache/geode/internal/cache/CacheServerImpl.java 
670c697c7f6c3d22a3c79d10be4e9c9929cd612b 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
67fcce85c0f812e062f602e97d604dd39c0edd9f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
5e352241ac114b891adb178e1820e8c74017fa64 
  geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 
d9a34e1cfa85cc08ff4f0af81b6adc2bdbd7e869 
  
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
 037bff6ebd0f49b7afe6b5aad4c0edd9ca3b139a 
  
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/FetchEntriesMessage.java
 489ffbaddabfc2d2a46d87d3060d5c61b76c7f32 
  geode-core/src/main/java/org/apache/geode/internal/cache/tier/Command.java 
d7f7c7b20372351963dbc6cb1e8bab506ffcc1d0 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 9658f98da808e50b87fe54854df249b28b97662d 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java
 1fb8c8cac62c54b582d40688a423fddcae23b36e 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
 adf702a1d361ba28f4ebe46695325d23344db756 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
 fd5154f9c5bdd516dad2eb93cd78e7b73213255f 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientProxy.java
 8450db905a3834374bd451b55d8720d313d21d2a 
  

Build failed in Jenkins: Geode-release #58

2017-06-08 Thread Apache Jenkins Server
See 


Changes:

[boglesby] GEODE-3040: Compare gateway profile order policy against 9.0 instead 
of

[dbarnes] GEODE-3044: User Manual: Update Swagger example and screen shots -

[dbarnes] GEODE-2555: Region Management docs page refers to the wrong field (id=

[abaker] GEODE-194: Remove spark connector

[dbarnes] GEODE-3044: User Manual: Update Swagger example and screen shots -

[dschneider] GEODE-3027: add a simple PartitionResolver

--
[...truncated 98.55 KB...]
:geode-benchmarks:check
:geode-benchmarks:build
:geode-benchmarks:distributedTest UP-TO-DATE
:geode-benchmarks:integrationTest UP-TO-DATE
:geode-common:assemble
:geode-common:compileTestJava
:geode-common:processTestResources UP-TO-DATE
:geode-common:testClasses
:geode-common:checkMissedTests
:geode-common:spotlessJavaCheck
:geode-common:spotlessCheck
:geode-common:test
:geode-common:check
:geode-common:build
:geode-common:distributedTest
:geode-common:integrationTest
:geode-core:assemble
:geode-core:checkMissedTests
:geode-core:spotlessJavaCheck
:geode-core:spotlessCheck
:geode-core:test
:geode-core:check
:geode-core:build
:geode-core:distributedTest
:geode-core:integrationTest

org.apache.geode.redis.RedisServerTest > 
initializeRedisCreatesRegionsUsingSystemProperty FAILED
java.lang.RuntimeException: Could not start Server
at 
org.apache.geode.redis.GeodeRedisServer.start(GeodeRedisServer.java:387)
at 
org.apache.geode.redis.RedisServerTest.initializeRedisCreatesRegionsUsingSystemProperty(RedisServerTest.java:78)

Caused by:
java.net.BindException: Address already in use

java.lang.NullPointerException
at 
org.apache.geode.redis.GeodeRedisServer.shutdown(GeodeRedisServer.java:629)
at 
org.apache.geode.redis.RedisServerTest.teardown(RedisServerTest.java:46)

org.apache.geode.redis.RedisServerTest > initializeRedisCreatesThreeRegions 
FAILED
java.lang.AssertionError
at 
org.apache.geode.redis.RedisServerTest.initializeRedisCreatesThreeRegions(RedisServerTest.java:54)

org.apache.geode.redis.RedisServerTest > 
initializeRedisCreatesPartitionedRegionByDefault FAILED
java.lang.AssertionError
at 
org.apache.geode.redis.RedisServerTest.initializeRedisCreatesPartitionedRegionByDefault(RedisServerTest.java:64)

3567 tests completed, 3 failed, 163 skipped
:geode-core:integrationTest FAILED
:geode-cq:assemble
:geode-cq:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-cq:processTestResources
:geode-cq:testClasses
:geode-cq:checkMissedTests
:geode-cq:spotlessJavaCheck
:geode-cq:spotlessCheck
:geode-cq:test
:geode-cq:check
:geode-cq:build
:geode-cq:distributedTest
:geode-cq:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources
:geode-json:testClasses
:geode-json:checkMissedTests UP-TO-DATE
:geode-json:spotlessJavaCheck
:geode-json:spotlessCheck
:geode-json:test UP-TO-DATE
:geode-json:check
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJavaNote: Some input files use or override a 
deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

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

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

:geode-old-client-support:processTestResources UP-TO-DATE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest

Re: Review Request 59896: GEODE-3043 surprise member added when the member is already in the cluster

2017-06-08 Thread Brian Rowe

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


Ship it!




Ship It!

- Brian Rowe


On June 7, 2017, 10:02 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59896/
> ---
> 
> (Updated June 7, 2017, 10:02 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3043
> https://issues.apache.org/jira/browse/GEODE-3043
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Modified InternalDistributedMember to disregard the "name" field in compareTo 
> if partial IDs are being used in the comparison.
> 
> Modified GMSMembershipManager to attempt to replace partial IDs with the full 
> IDs from the membership view.  Typically these IDs will cause no problems and 
> methods in GMSMembershipManager that send messages to a recipient will also 
> attempt to replace the ID with a full ID from the membership view, but it's 
> good to try to keep IDs canonical.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
>  6982d31f13ad5f5f004f0e7c9a0f4e1aa26a151a 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
>  c82d97e9facca19bfb08b20407abf21dd274684e 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
>  fe560d9eb71131cf1ca5e5ca124e877769e9d518 
>   
> geode-core/src/test/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java
>  0153ca6506af574798ad01b98d9a395fad7be5d5 
> 
> 
> Diff: https://reviews.apache.org/r/59896/diff/1/
> 
> 
> Testing
> ---
> 
> New tests, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Re: Review Request 59894: GEODE-3041 CI failure: DistributedMemberDUnitTest.testGroupsInAllVMs fails intermittently

2017-06-08 Thread Brian Rowe

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


Ship it!




Ship It!

- Brian Rowe


On June 7, 2017, 9:56 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59894/
> ---
> 
> (Updated June 7, 2017, 9:56 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3041
> https://issues.apache.org/jira/browse/GEODE-3041
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When shutting down the MembershipManager after it has joined we should not 
> use uncleanShutdown because the member will appear to have crashed.  Instead 
> we should do a normal shutdown.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionChannel.java
>  ef4056ca56c1b102e507f96bbfe41396d0139aa5 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  029e6377f56d80dd81e4cec430f106ac743e5178 
> 
> 
> Diff: https://reviews.apache.org/r/59894/diff/1/
> 
> 
> Testing
> ---
> 
> The test was failing every time I ran it.  It no longer fails.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



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

2017-06-08 Thread Brian Rowe

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


Ship it!




Ship It!

- Brian Rowe


On June 8, 2017, 6:36 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59925/
> ---
> 
> (Updated June 8, 2017, 6:36 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When restarting from a locatorView.dat file we should ignore any locator 
> entries in the view.  Recovery tries to get this state from other locators 
> before resorting to using the persisted view so there we know all of the 
> locator entries in the view are invalid.  This allows the locators to quickly 
> move into the concurrent-startup algorithm and find each other.
> 
> I removed the Flaky categorization of the test that I modified to reproduce 
> the problem.  A subclass's use of the test was reported as a Flaky failure 
> but I found that the ticket was closed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
>  e3635f2d93aae212cbff2f2058b6dc728a04776a 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> 8ff9b67e13dd50499d861ff62ddae3fb8668dd28 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java
>  9d49d30abfb8acccd8a5547ba0ee3c7bcf9e7970 
> 
> 
> Diff: https://reviews.apache.org/r/59925/diff/1/
> 
> 
> Testing
> ---
> 
> The problem was easily reproduced using LocatorDUnitTest.testStartTwoLocators 
> by repeating the cycling of the locators.  It failed every time I ran it.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Re: Review Request 59924: Reverted GEODE-2804 (Allow a locator host to be taken off line and replaced with a different machine having the same host name)

2017-06-08 Thread Bruce Schuchardt

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


Ship it!




Ship It!

- Bruce Schuchardt


On June 8, 2017, 5:54 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59924/
> ---
> 
> (Updated June 8, 2017, 5:54 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There was conflict in "LauncherLifecycleCommands", this code moved to 
> "ClusterConfigurationStatusRetriever".
> 
> We are reverting this issue as we cache InetAddress in geode. But caching 
> "hostname" is causing various issuess.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  070451c 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  84d42cf 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  e9476b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  eb71d38 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
>  9f35edd 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  9ceb461 
> 
> 
> Diff: https://reviews.apache.org/r/59924/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>



Re: Review Request 59923: GEODE-3029: Tomcat Install Documentation has incorrect required JARs

2017-06-08 Thread David Anuta


> On June 8, 2017, 8:05 p.m., David Anuta wrote:
> >

Looks good!


- David


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


On June 8, 2017, 5:33 p.m., Dave Barnes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59923/
> ---
> 
> (Updated June 8, 2017, 5:33 p.m.)
> 
> 
> Review request for geode.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3029: Tomcat Install Documentation has incorrect required JARs
> 
> 
> Diffs
> -
> 
>   
> geode-docs/tools_modules/http_session_mgmt/tomcat_installing_the_module.html.md.erb
>  b96a2548352e4dc6d945830f67c6c986d873c9f5 
> 
> 
> Diff: https://reviews.apache.org/r/59923/diff/1/
> 
> 
> Testing
> ---
> 
> Successfully built User Guide from source using bookbinder, new text 
> introduced no errors.
> 
> 
> Thanks,
> 
> Dave Barnes
> 
>



Re: Review Request 59923: GEODE-3029: Tomcat Install Documentation has incorrect required JARs

2017-06-08 Thread David Anuta

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



- David Anuta


On June 8, 2017, 5:33 p.m., Dave Barnes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59923/
> ---
> 
> (Updated June 8, 2017, 5:33 p.m.)
> 
> 
> Review request for geode.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3029: Tomcat Install Documentation has incorrect required JARs
> 
> 
> Diffs
> -
> 
>   
> geode-docs/tools_modules/http_session_mgmt/tomcat_installing_the_module.html.md.erb
>  b96a2548352e4dc6d945830f67c6c986d873c9f5 
> 
> 
> Diff: https://reviews.apache.org/r/59923/diff/1/
> 
> 
> Testing
> ---
> 
> Successfully built User Guide from source using bookbinder, new text 
> introduced no errors.
> 
> 
> Thanks,
> 
> Dave Barnes
> 
>



Re: Review Request 59905: GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and startServer

2017-06-08 Thread Jinmei Liao

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


Ship it!




Ship It!

- Jinmei Liao


On June 8, 2017, 5:33 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59905/
> ---
> 
> (Updated June 8, 2017, 5:33 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and 
> startServer
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
>  df16e9b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e14 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  c5ff6b6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
>  a122de0 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java
>  1ff60d6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  ab6dc3d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59905/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passed (other than 1 known flaky)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>



Re: Review Request 59905: GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and startServer

2017-06-08 Thread Emily Yeh

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


Ship it!




Ship It!

- Emily Yeh


On June 8, 2017, 5:33 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59905/
> ---
> 
> (Updated June 8, 2017, 5:33 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and 
> startServer
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
>  df16e9b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e14 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  c5ff6b6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
>  a122de0 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java
>  1ff60d6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  ab6dc3d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59905/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passed (other than 1 known flaky)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>



Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Jacob Barrett
On Thu, Jun 8, 2017 at 12:35 PM Mark Bretl  wrote:

> I like the functionality we get with GitHub, including the Travis CI
> integration.
>
> Do we have a proposed workflow change for committers?


The proposed workflow change to committers is to use the same workflow as
contributors. The only difference is after sufficient review of the PR you
could commit it yourself.

-Jake


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Mark Bretl
I like the functionality we get with GitHub, including the Travis CI
integration.

Do we have a proposed workflow change for committers?

--Mark

On Thu, Jun 8, 2017 at 11:20 AM, Jacob Barrett  wrote:

> Some more fuel on this fire... PR's get checked against Travis CI
> automatically and the results show up in the PR. This is a good safety
> check even for committers before merging their branch into the development
> branch.
>
> -Jake
>


Re: Review Request 59754: GEODE-2928: get rid of the isGfshVM static variable

2017-06-08 Thread Emily Yeh

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




geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java
Lines 41-42 (original), 41-42 (patched)


A possible nitpick: is this comment missing a period? ("Indicates that the 
command will only run in the gfsh shell. Gfsh ExecutionStrategy will use this 
flag to determine whether to invoke remote call or not.)


- Emily Yeh


On June 5, 2017, 8:03 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59754/
> ---
> 
> (Updated June 5, 2017, 8:03 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * consolidate the availability indicators
> * remove the isGfshVM and isGfshVM() method
> * enhance the MultiStepCommand to include info on shellOnly commands to 
> enhance command validation
> * remove the SUPPORT_MULTIPLE_GFSH static flag and properly remove the gfsh 
> instance at the end of each test
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/GfeConsoleReaderFactory.java
>  120d6257b 
>   geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java 
> 2e6dc3973 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
>  038e0691e 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ClientCommands.java
>  e61934261 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CommandAvailabilityIndicator.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java
>  bc9c05b81 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  ad40518f8 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  cb9c4fe50 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java
>  30d840a0f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DurableClientCommands.java
>  6441f20cc 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
>  9d263d110 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java
>  2774584ff 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
>  d46024d38 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java
>  b3d96757b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b681 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MemberCommands.java
>  695718a82 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
>  9754d7d52 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/PDXCommands.java
>  9f1290d16 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java
>  d3c263509 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/RegionCommands.java
>  2009dcc05 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  efd10d27b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StatusCommands.java
>  fffb9646f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/WanCommands.java
>  28686ce4d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  e2164a375 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/CLIMultiStepHelper.java
>  d53261d04 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/MultiStepCommand.java
>  6708726cd 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/RemoteExecutionStrategy.java
>  fa0f3b259 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/result/AbstractResultData.java
>  f453ec67c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  c5ff6b6a5 
>   
> 

[GitHub] geode pull request #569: Resolve Eclipse/IntelliJ import order disagreement

2017-06-08 Thread PurelyApplied
GitHub user PurelyApplied opened a pull request:

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

Resolve Eclipse/IntelliJ import order disagreement

The style guides contained in `etc/` are not consistent.
This commit modifies `etc/intellij-java-google-style.xml` to adhere to the 
import order specified in `etc/eclipseOrganizeImports.importorder`.
This seems a significant enough deviation from the original Google style 
guide to warrant a name change in the XML.

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

$ git pull https://github.com/PurelyApplied/geode geode-import-order

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

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


commit 41c8f7b1651914d61a428f6b8519a8277f2c414d
Author: Patrick Rhomberg 
Date:   2017-06-08T18:48:47Z

Resolve Eclipse/IntelliJ import order disagreement in style files.

The style files contained in `etc/` are not consistent between IDEs.
This commit modifies `etc/intellij-java-google-style.xml` to adhere to the 
import order specified in `etc/eclipseOrganizeImports.importorder`.
This seems a significant enough deviation from the original Google style 
guide to warrant a name change in the XML.




---
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 59926: waitUntilFlush should check if its brq's tempQueue is not empty

2017-06-08 Thread xiaojian zhou

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

Review request for geode, Barry Oglesby and Dan Smith.


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


Repository: geode


Description
---

There's time window that data region bucket is ready, but shadow key's bucket 
is not. So the event will be added into tempQueue at that windows.

If we run waitUntilFlush during that window, it did not check the tempQueue 
since its brq is not exist yet. It will cause data mismatch (i.e. we found the 
key in data region, but not in index)

We should pass in the data region's bucket list and let it wait until these 
tempQueue are empty.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/AsyncEventQueueImpl.java
 bf7e87445 
  
geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
 c38d5475a 
  
geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/WaitUntilParallelGatewaySenderFlushedCoordinator.java
 42ce68cab 
  
geode-core/src/test/java/org/apache/geode/internal/cache/wan/parallel/WaitUntilParallelGatewaySenderFlushedCoordinatorJUnitTest.java
 5e12ed5ab 
  
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java
 e11384c59 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunctionJUnitTest.java
 f92a296f7 


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


Testing
---


Thanks,

xiaojian zhou



Re: Review Request 59894: GEODE-3041 CI failure: DistributedMemberDUnitTest.testGroupsInAllVMs fails intermittently

2017-06-08 Thread Hitesh Khamesra

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


Ship it!




Ship It!

- Hitesh Khamesra


On June 7, 2017, 9:56 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59894/
> ---
> 
> (Updated June 7, 2017, 9:56 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3041
> https://issues.apache.org/jira/browse/GEODE-3041
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When shutting down the MembershipManager after it has joined we should not 
> use uncleanShutdown because the member will appear to have crashed.  Instead 
> we should do a normal shutdown.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionChannel.java
>  ef4056ca56c1b102e507f96bbfe41396d0139aa5 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  029e6377f56d80dd81e4cec430f106ac743e5178 
> 
> 
> Diff: https://reviews.apache.org/r/59894/diff/1/
> 
> 
> Testing
> ---
> 
> The test was failing every time I ran it.  It no longer fails.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Re: Review Request 59894: GEODE-3041 CI failure: DistributedMemberDUnitTest.testGroupsInAllVMs fails intermittently

2017-06-08 Thread Udo Kohlmeyer

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


Ship it!




Ship It!

- Udo Kohlmeyer


On June 7, 2017, 9:56 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59894/
> ---
> 
> (Updated June 7, 2017, 9:56 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Bugs: GEODE-3041
> https://issues.apache.org/jira/browse/GEODE-3041
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> When shutting down the MembershipManager after it has joined we should not 
> use uncleanShutdown because the member will appear to have crashed.  Instead 
> we should do a normal shutdown.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionChannel.java
>  ef4056ca56c1b102e507f96bbfe41396d0139aa5 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  029e6377f56d80dd81e4cec430f106ac743e5178 
> 
> 
> Diff: https://reviews.apache.org/r/59894/diff/1/
> 
> 
> Testing
> ---
> 
> The test was failing every time I ran it.  It no longer fails.
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>



Re: Review Request 59924: Reverted GEODE-2804 (Allow a locator host to be taken off line and replaced with a different machine having the same host name)

2017-06-08 Thread Udo Kohlmeyer

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


Ship it!




If precheckin passes... Ship it..

- Udo Kohlmeyer


On June 8, 2017, 5:54 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59924/
> ---
> 
> (Updated June 8, 2017, 5:54 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There was conflict in "LauncherLifecycleCommands", this code moved to 
> "ClusterConfigurationStatusRetriever".
> 
> We are reverting this issue as we cache InetAddress in geode. But caching 
> "hostname" is causing various issuess.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
>  070451c 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  84d42cf 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  e9476b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  eb71d38 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
>  9f35edd 
>   
> geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
>  9ceb461 
> 
> 
> Diff: https://reviews.apache.org/r/59924/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>



Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Jacob Barrett
Some more fuel on this fire... PR's get checked against Travis CI
automatically and the results show up in the PR. This is a good safety
check even for committers before merging their branch into the development
branch.

-Jake


Re: [DISCUSS] Discussions of API changes missing or lost in noise

2017-06-08 Thread Jacob Barrett
I am fine with the initial request coming to the dev list but not every
update. I would have been just as happy if we had changed JIRA to just
notify about new tickets and left followups with watchers only. I believe
the same behavior can be achieved with PR and Reviews (although we could
just stop using review board).

-Jake

On Thu, Jun 8, 2017 at 10:39 AM Dan Smith  wrote:

> The JIRA noise has gone away, which is awesome! Do we want to move github
> PRs and review requests off this list as well or keep sending them to the
> list?
>
> -Dan
>
> On Mon, Jun 5, 2017 at 1:02 PM, Anthony Baker  wrote:
>
> > Fixed!  Please check iss...@geode.apache.org for JIRA updates.
> >
> > Anthony
> >
> > > On Jun 1, 2017, at 3:41 PM, Anthony Baker  wrote:
> > >
> > > There is an iss...@geode.apache.org mailing list but it seems to have
> > been misconfigured last December.  All the JIRA traffic got shunted over
> to
> > dev@.
> > >
> > > I filed a ticket to fix this:
> > > https://issues.apache.org/jira/browse/INFRA-14266
> > >
> > >
> > > Anthony
> > >
> > >> On Jun 1, 2017, at 2:09 PM, Dan Smith  wrote:
> > >>
> > >> Hi devs,
> > >>
> > >> This is similar to the discussion John started about keeping track of
> > >> changes to geode. I'm seeing some changes happening to the public API
> > that
> > >> I feel like maybe should have a more visible discussion. For example
> > >> GEODE-2892 (Region.sizeOnServer) or GEODE-3005 (new API for
> > partitioning).
> > >>
> > >> I think we should have a clear policy to send an email with [DISCUSS]
> in
> > >> the header to mailing list for changes to the public API, behavior, or
> > >> dependencies. Or wiki
> > >>  > Criteria+for+Code+Submissions>
> > >> says that changes should be discussed, but it doesn't really specify
> > how.
> > >>
> > >> Part of the issue is that I find it impossible to keep up with the
> > amount
> > >> of JIRA noise on the dev list, so just creating a JIRA is not enough
> > for me
> > >> to notice a new API change. I propose that we segregate all of this
> > >> automated email onto a separate list, either geode-commits or some new
> > >> list. I'd like to segregate anything not directly sent by a human -
> > JIRAs,
> > >> PRs, and reviewboards.
> > >>
> > >> -Dan
> > >
> >
> >
>


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Jacob Barrett
On Thu, Jun 8, 2017 at 10:20 AM Dan Smith  wrote:

> One thing that can
> help is if you add your github id to your apache profile - the PR will then
> show that it is coming from an apache member.


To illustrate what Dan is saying, notice the "Member" tag on my comment in
the screen cap below.
[image: Screen Shot 2017-06-08 at 11.08.22 AM.png]


-Jake


Review Request 59924: Reverted GEODE-2804 (Allow a locator host to be taken off line and replaced with a different machine having the same host name)

2017-06-08 Thread Hitesh Khamesra

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

Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo Kohlmeyer.


Repository: geode


Description
---

There was conflict in "LauncherLifecycleCommands", this code moved to 
"ClusterConfigurationStatusRetriever".

We are reverting this issue as we cache InetAddress in geode. But caching 
"hostname" is causing various issuess.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java
 070451c 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 84d42cf 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 e9476b5 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 4c668b6 
  
geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ClusterConfigurationStatusRetriever.java
 9f35edd 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java
 9ceb461 


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


Testing
---


Thanks,

Hitesh Khamesra



Re: Review Request 59923: GEODE-3029: Tomcat Install Documentation has incorrect required JARs

2017-06-08 Thread Karen Miller

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


Ship it!




Ship It!

- Karen Miller


On June 8, 2017, 5:33 p.m., Dave Barnes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59923/
> ---
> 
> (Updated June 8, 2017, 5:33 p.m.)
> 
> 
> Review request for geode.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3029: Tomcat Install Documentation has incorrect required JARs
> 
> 
> Diffs
> -
> 
>   
> geode-docs/tools_modules/http_session_mgmt/tomcat_installing_the_module.html.md.erb
>  b96a2548352e4dc6d945830f67c6c986d873c9f5 
> 
> 
> Diff: https://reviews.apache.org/r/59923/diff/1/
> 
> 
> Testing
> ---
> 
> Successfully built User Guide from source using bookbinder, new text 
> introduced no errors.
> 
> 
> Thanks,
> 
> Dave Barnes
> 
>



Re: [DISCUSS] Discussions of API changes missing or lost in noise

2017-06-08 Thread Dan Smith
The JIRA noise has gone away, which is awesome! Do we want to move github
PRs and review requests off this list as well or keep sending them to the
list?

-Dan

On Mon, Jun 5, 2017 at 1:02 PM, Anthony Baker  wrote:

> Fixed!  Please check iss...@geode.apache.org for JIRA updates.
>
> Anthony
>
> > On Jun 1, 2017, at 3:41 PM, Anthony Baker  wrote:
> >
> > There is an iss...@geode.apache.org mailing list but it seems to have
> been misconfigured last December.  All the JIRA traffic got shunted over to
> dev@.
> >
> > I filed a ticket to fix this:
> > https://issues.apache.org/jira/browse/INFRA-14266
> >
> >
> > Anthony
> >
> >> On Jun 1, 2017, at 2:09 PM, Dan Smith  wrote:
> >>
> >> Hi devs,
> >>
> >> This is similar to the discussion John started about keeping track of
> >> changes to geode. I'm seeing some changes happening to the public API
> that
> >> I feel like maybe should have a more visible discussion. For example
> >> GEODE-2892 (Region.sizeOnServer) or GEODE-3005 (new API for
> partitioning).
> >>
> >> I think we should have a clear policy to send an email with [DISCUSS] in
> >> the header to mailing list for changes to the public API, behavior, or
> >> dependencies. Or wiki
> >>  Criteria+for+Code+Submissions>
> >> says that changes should be discussed, but it doesn't really specify
> how.
> >>
> >> Part of the issue is that I find it impossible to keep up with the
> amount
> >> of JIRA noise on the dev list, so just creating a JIRA is not enough
> for me
> >> to notice a new API change. I propose that we segregate all of this
> >> automated email onto a separate list, either geode-commits or some new
> >> list. I'd like to segregate anything not directly sent by a human -
> JIRAs,
> >> PRs, and reviewboards.
> >>
> >> -Dan
> >
>
>


Re: Review Request 59754: GEODE-2928: get rid of the isGfshVM static variable

2017-06-08 Thread Patrick Rhomberg

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




geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
Lines 37-38 (original), 37-38 (patched)


Remove unused imports from the following files:

ClientCommands.java
CliUtil.java
ConfigCommands.java
CreateAlterDestroyRegionCommands.java
DataCommands.java
DeployCommands.java
DiskStoreCommands.java
DurableClientCommands.java
FunctionCommands.java
IndexCommands.java
MemberCommands.java
MiscellaneousCommands.java
QueueCommands.java
RegionCommands.java
StatusCommands.java



geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/MultiStepCommand.java
Lines 30 (patched)


Remove redundant `public static`.


- Patrick Rhomberg


On June 5, 2017, 8:03 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59754/
> ---
> 
> (Updated June 5, 2017, 8:03 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * consolidate the availability indicators
> * remove the isGfshVM and isGfshVM() method
> * enhance the MultiStepCommand to include info on shellOnly commands to 
> enhance command validation
> * remove the SUPPORT_MULTIPLE_GFSH static flag and properly remove the gfsh 
> instance at the end of each test
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/GfeConsoleReaderFactory.java
>  120d6257b 
>   geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java 
> 2e6dc3973 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
>  038e0691e 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ClientCommands.java
>  e61934261 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CommandAvailabilityIndicator.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java
>  bc9c05b81 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  ad40518f8 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  cb9c4fe50 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java
>  30d840a0f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DurableClientCommands.java
>  6441f20cc 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
>  9d263d110 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java
>  2774584ff 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
>  d46024d38 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java
>  b3d96757b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b681 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MemberCommands.java
>  695718a82 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
>  9754d7d52 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/PDXCommands.java
>  9f1290d16 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java
>  d3c263509 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/RegionCommands.java
>  2009dcc05 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  efd10d27b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StatusCommands.java
>  fffb9646f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/WanCommands.java
>  28686ce4d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  e2164a375 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/CLIMultiStepHelper.java
>  d53261d04 
>   
> 

Review Request 59923: GEODE-3029: Tomcat Install Documentation has incorrect required JARs

2017-06-08 Thread Dave Barnes

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

Review request for geode.


Repository: geode


Description
---

GEODE-3029: Tomcat Install Documentation has incorrect required JARs


Diffs
-

  
geode-docs/tools_modules/http_session_mgmt/tomcat_installing_the_module.html.md.erb
 b96a2548352e4dc6d945830f67c6c986d873c9f5 


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


Testing
---

Successfully built User Guide from source using bookbinder, new text introduced 
no errors.


Thanks,

Dave Barnes



Re: Review Request 59905: GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and startServer

2017-06-08 Thread Jared Stewart

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

(Updated June 8, 2017, 5:33 p.m.)


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


Changes
---

Incorporate review feedback


Repository: geode


Description
---

GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and 
startServer


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
 df16e9b 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 4c668b6 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 9f68d3a 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
 c2c6e14 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 c5ff6b6 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
 a122de0 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java
 1ff60d6 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
 ab6dc3d 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java 
PRE-CREATION 


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

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


Testing
---

Precheckin passed (other than 1 known flaky)


Thanks,

Jared Stewart



Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Ernest Burghardt
+1 for just PRs

+1 for trying the functionality Jared has pointed out in Github

On Thu, Jun 8, 2017 at 10:19 AM, Jared Stewart  wrote:

>
> On Jun 8, 2017, at 8:32 AM, Bruce Schuchardt 
> wrote:
>
> I also don't see any way to push a PR to specific individuals for review.
> In Reviewboard there is a nice queue of pending reviews that I can go
> through.  On github they're all mixed together and it's difficult to tell
> whether any of them are relevant to me.
>
>
> Github has functionality for this (notice the “assignees” section on the
> right hand side of this PR).  I believe it may be a setting in our
> repository that is preventing us from actually making such assignments.
>
>
>
> I like the idea of a single source of history for reviews but I don't much
> like the idea of having to create PRs on a read-only system and then merge
> my changes to ASF's repo.  Being able to commit directly seems like a
> committer perk that your idea would take away from us.
>
> Can you expand a bit about what you mean by this?  It would seem to me
> that whether we use ReviewBoard or PRs the flow is essentially the same for
> committers:
> 1) Post review request (either from a diff on Reviewboard or from a branch
> on a PR)
> 2a) Receive feedback
> 2b) Update review request (with a new diff on Reviewboard or by pushing
> new commits to the branch of your PR)
> 3) Merge your changes into develop and push them to the ASF repo
>
>


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Dan Smith
+1 to just using PRs.

I think the benefits to new people to the project make it worth it to
switch. New people will see PRs from committers when they are creating
their PRs, which will help provide good examples. It's one less system to
sign up on as a contributor so it's easier for new people to find and
comment on committer PRs.

Personally I've switched from reviewboard to PRs and it works well for me.
You can just mention people in comments to ask for a review. You can get
your nice "queue" view, just go to https://github.com/pulls/mentioned to
see all of the open PRs where you are mentioned.

I agree it would help to have a convention to know if a PR is from a
committer or not. So far I think we've just relied on the fact that the
committers generally know who the other committers are. One thing that can
help is if you add your github id to your apache profile - the PR will then
show that it is coming from an apache member.

-Dan



On Thu, Jun 8, 2017 at 10:05 AM, Jacob Barrett  wrote:

> On Thu, Jun 8, 2017 at 8:32 AM Bruce Schuchardt 
> wrote:
>
> > One thing I find confusing in PRs is whether the person submitting the
> > request is a committer or not.  Non-committers need someone to merge the
> > PR while committers are just looking for a review and will merge the
> > changes to develop themselves.
> >
>
> I don't see how PR complicates the process for committers since all work
> should be on a topic branch and would be merged by the committer after
> review (PR or Review Board). The process of merging the changes does not
> change.
>
>
> > I also don't see any way to push a PR to specific individuals for
> > review.  In Reviewboard there is a nice queue of pending reviews that I
> > can go through.  On github they're all mixed together and it's difficult
> > to tell whether any of them are relevant to me.
> >
>
> While PR doesn't have a nice queue concept it does allow you call out
> individuals by mentioning them in comments.
>
>
> > I like the idea of a single source of history for reviews but I don't
> > much like the idea of having to create PRs on a read-only system and
> > then merge my changes to ASF's repo.  Being able to commit directly
> > seems like a committer perk that your idea would take away from us.
> >
>
> Yes, I wish the PR system was not read only with ASF but having used this
> proposed method extensively with geode-native I really find it not to have
> a consistent single place to do reviews of bother peer committers and
> contributors.
>
> Another argument for this proposal is that individual contributors don't
> have to learn two different review methods to review committer's works nor
> do they have to change review methods if they are voted in as committers.
>
> -Jake
>


Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

2017-06-08 Thread Patrick Rhomberg

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




geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
Line 41 (original), 43 (patched)


Remove unused imports present in:
- ExportLogsCommand.java
- ExportLogsCommandTest.java
- ExportLogsDUnitTest.java
- SizeExportLogsFunction.java


- Patrick Rhomberg


On June 7, 2017, 10:34 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59811/
> ---
> 
> (Updated June 7, 2017, 10:34 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, 
> and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2420: add file-size-limit param to the ExportLogsController
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
>  0ff780cbf66937d8ececfb3a2d0789ee485b9b62 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
>  57355c0efae4c6da9470267f95e27e59aa4d8b2c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
>  a369c6e1ffb330715fbde2cd69d023ed36f133ad 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
>  16549e70bbebf4390bb73a481274e92ca6cad035 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
>  8609b3aaf0a0eb1ba903bd39c64103f9510a6a78 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
>  09ee08dd6af29b9a418ef7499defc4980da787ed 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
>  44a036298e0991c880fc552596d296e104b97ca1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
>  4e1dac013d239437829bc52dc70689c4ba15dc58 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
>  cc5e7d5256741ad0a48ff87c7f989a18b90f7f03 
> 
> 
> Diff: https://reviews.apache.org/r/59811/diff/3/
> 
> 
> Testing
> ---
> 
> 6/7/17: re-started precheckin
> precheckin is green with the exception of unrelated DUnit tests
> 
> * re-starting precheckin once more
> 
> Precheckin started
> 
> 
> Thanks,
> 
> Ken Howe
> 
>



Re: Need information about FunctionStatistics

2017-06-08 Thread Barry Oglesby
Dinesh,

The FunctionStatistics and FunctionServiceStatistics look to be displaying
properly in vsd. Are you not seeing them?

Thanks,
Barry Oglesby


On Thu, Jun 8, 2017 at 9:51 AM, Kirk Lund  wrote:

> I think we would probably need to introduce a new FunctionServiceMXBean
> with these stats as attributes or add showFunctionMetrics() operation to
> MemberMXBean.
>
> On Wed, Jun 7, 2017 at 6:32 AM, Dinesh Akhand  wrote:
>
> > Hi Team,
> >
> > Currently I can see Function stats are getting generated .
> > functionExecutionsCompleted operations/sec: samples=1955 min=0 max=0
> > average=0 stddev=0 last=0
> >   functionExecutionsCompletedProcessingTime nanoseconds/sec:
> samples=1955
> > min=0 max=0 average=0 stddev=0 last=0
> >   functionExecutionsRunning operations: samples=1956 min=0 max=0
> average=0
> > stddev=0 last=0
> >   resultsSentToResultCollector operations/sec: samples=1955 min=0 max=2.6
> > average=0 stddev=0.1 last=0
> >   resultsReceived operations/sec: samples=1955 min=0 max=2.6 average=0
> > stddev=0.1 last=0
> >   functionExecutionCalls operations/sec: samples=1955 min=0 max=0
> > average=0 stddev=0 last=0
> >
> > but I am not able to see them on JMX .
> >
> > I found last end point in  MemberMBeanBridge for JMX.   Is there any
> > information and document can you provide.
> > I want to publish FunctionStatistics on JMX , any suggestion will be
> > welcome.
> >
> >
> > Thanks,
> > Dinesh Akhand
> > This message and the information contained herein is proprietary and
> > confidential and subject to the Amdocs policy statement,
> >
> > you may review at https://www.amdocs.com/about/email-disclaimer <
> > https://www.amdocs.com/about/email-disclaimer>
> >
>


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Jacob Barrett
On Thu, Jun 8, 2017 at 8:32 AM Bruce Schuchardt 
wrote:

> One thing I find confusing in PRs is whether the person submitting the
> request is a committer or not.  Non-committers need someone to merge the
> PR while committers are just looking for a review and will merge the
> changes to develop themselves.
>

I don't see how PR complicates the process for committers since all work
should be on a topic branch and would be merged by the committer after
review (PR or Review Board). The process of merging the changes does not
change.


> I also don't see any way to push a PR to specific individuals for
> review.  In Reviewboard there is a nice queue of pending reviews that I
> can go through.  On github they're all mixed together and it's difficult
> to tell whether any of them are relevant to me.
>

While PR doesn't have a nice queue concept it does allow you call out
individuals by mentioning them in comments.


> I like the idea of a single source of history for reviews but I don't
> much like the idea of having to create PRs on a read-only system and
> then merge my changes to ASF's repo.  Being able to commit directly
> seems like a committer perk that your idea would take away from us.
>

Yes, I wish the PR system was not read only with ASF but having used this
proposed method extensively with geode-native I really find it not to have
a consistent single place to do reviews of bother peer committers and
contributors.

Another argument for this proposal is that individual contributors don't
have to learn two different review methods to review committer's works nor
do they have to change review methods if they are voted in as committers.

-Jake


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Dave Barnes
I've used both PRs and Review Boards for doc changes.
The Review Board's targeted reviewer list, as Bruce points out, is a plus.
It would be great if PRs could do that.

On Thu, Jun 8, 2017 at 9:28 AM, John Blum  wrote:

> +1 to Bruce's comments.
>
> PRs are for contributors that do not have commit privileges.  ReviewBoard
> is a tool for "reviewing" changes.
>
> However, what is also common practice on open source projects, even for
> committers, is to create a topic branch containing the commit with the
> desired changes (labeled with an appropriate JIRA ticket + description).
> Then a reviewer can then pick up the topic branch, review the code changes,
> polish things up and even merge the topic branch back into the mainline
> (e.g. develop) and close the ticket.  IDEs, more than ReviewBoard/FishEye,
> etc, have better tooling for reviewing diffs, making change, running tests,
> etc.
>
> -John
>
>
>
> On Thu, Jun 8, 2017 at 8:32 AM, Bruce Schuchardt 
> wrote:
>
> > One thing I find confusing in PRs is whether the person submitting the
> > request is a committer or not.  Non-committers need someone to merge the
> PR
> > while committers are just looking for a review and will merge the changes
> > to develop themselves.
> >
> > I also don't see any way to push a PR to specific individuals for review.
> > In Reviewboard there is a nice queue of pending reviews that I can go
> > through.  On github they're all mixed together and it's difficult to tell
> > whether any of them are relevant to me.
> >
> > I like the idea of a single source of history for reviews but I don't
> much
> > like the idea of having to create PRs on a read-only system and then
> merge
> > my changes to ASF's repo.  Being able to commit directly seems like a
> > committer perk that your idea would take away from us.
> >
> >
> > On 6/7/17 6:42 PM, Jacob Barrett wrote:
> >
> >> All,
> >>
> >> I would like to discuss transitioning all code reviews to pull requests
> >> over using review board. For non-committer community members we ask them
> >> to
> >> make pull requests against the mirrored GitHub repo. Some committers use
> >> pull requests for their own work reviews while others use review board.
> I
> >> propose that we just use on and that the one we use be pull requests. It
> >> would give us a single source of history for reviews, a single model to
> >> understand for reviewers and committers and keep workflow consistent
> with
> >> all contributors, committers or not.
> >>
> >> -Jake
> >>
> >>
> >
>
>
> --
> -John
> john.blum10101 (skype)
>


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Jacob Barrett
On Thu, Jun 8, 2017 at 9:28 AM John Blum  wrote:

> PRs are for contributors that do not have commit privileges.  ReviewBoard
> is a tool for "reviewing" changes.
>

This isn't some law, it was our choice. What I am proposing is that we
re-evaluate this choice for consistency.


> However, what is also common practice on open source projects, even for
> committers, is to create a topic branch containing the commit with the
> desired changes (labeled with an appropriate JIRA ticket + description).
> Then a reviewer can then pick up the topic branch, review the code changes,
> polish things up and even merge the topic branch back into the mainline
> (e.g. develop) and close the ticket.  IDEs, more than ReviewBoard/FishEye,
> etc, have better tooling for reviewing diffs, making change, running tests,
> etc.
>

I feel like this argument justifies the PR model because it forces the
practice of topic branches. It creates consistency across all contributes,
committer or not.

-Jake


Re: Review Request 59905: GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and startServer

2017-06-08 Thread Patrick Rhomberg

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



All of my issues are nitpicks.  I'm sorry.  But only a little.


geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
Lines 304-306 (original)


I am amused by this function and am a little sad to see it go.  It sounds 
like something a cartoon hero would say.

"Complete Super-Advanced Combat Maneuver Alpha!"



geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
Lines 2369 (patched)


To pick a nit: it looks like this file only pads newlines to group w.r.t. 
top-level commands.  Maybe remove this empty line?



geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
Lines 2522 (patched)


Ditto above nitpick: drop this empty line?



geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
Line 278 (original), 260 (patched)


Trivial, but you could correct the typo in these test names while you're in 
here.



geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
Lines 296-298 (original), 278-280 (patched)


This is dead and can be removed?



geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java
Line 20 (original), 20 (patched)


Unused import.



geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java
Lines 18-34 (patched)


Some of these have gone stale.  `assertThat`, `RemoteExecutionStrategy`, 
and `CommandMarker` appear unused.


- Patrick Rhomberg


On June 8, 2017, 1:48 a.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59905/
> ---
> 
> (Updated June 8, 2017, 1:48 a.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and 
> startServer
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
>  df16e9b9e9f71f21ef7c8ac84267a9858ee4298c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4c668b6813de71aae9e3e46c82a5c231a41c1f6a 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e1425d71af9d2ea59046b17afd70ad30dd68 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  c5ff6b6a51a0bd866d62e15dfce13938c87ecf6b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
>  a122de048eb75ca448155541e2266c872c98904d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java
>  1ff60d67a7dbfe582e632c116ba83d471211de45 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
>  ab6dc3d10a6235af0ca6fc5164cac5d3c2419cdd 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59905/diff/1/
> 
> 
> Testing
> ---
> 
> Precheckin passed (other than 1 known flaky)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>



Re: Need information about FunctionStatistics

2017-06-08 Thread Kirk Lund
I think we would probably need to introduce a new FunctionServiceMXBean
with these stats as attributes or add showFunctionMetrics() operation to
MemberMXBean.

On Wed, Jun 7, 2017 at 6:32 AM, Dinesh Akhand  wrote:

> Hi Team,
>
> Currently I can see Function stats are getting generated .
> functionExecutionsCompleted operations/sec: samples=1955 min=0 max=0
> average=0 stddev=0 last=0
>   functionExecutionsCompletedProcessingTime nanoseconds/sec: samples=1955
> min=0 max=0 average=0 stddev=0 last=0
>   functionExecutionsRunning operations: samples=1956 min=0 max=0 average=0
> stddev=0 last=0
>   resultsSentToResultCollector operations/sec: samples=1955 min=0 max=2.6
> average=0 stddev=0.1 last=0
>   resultsReceived operations/sec: samples=1955 min=0 max=2.6 average=0
> stddev=0.1 last=0
>   functionExecutionCalls operations/sec: samples=1955 min=0 max=0
> average=0 stddev=0 last=0
>
> but I am not able to see them on JMX .
>
> I found last end point in  MemberMBeanBridge for JMX.   Is there any
> information and document can you provide.
> I want to publish FunctionStatistics on JMX , any suggestion will be
> welcome.
>
>
> Thanks,
> Dinesh Akhand
> This message and the information contained herein is proprietary and
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer <
> https://www.amdocs.com/about/email-disclaimer>
>


Re: Build failed in Jenkins: Geode-nightly #860

2017-06-08 Thread Kirk Lund
We would have to exclude that machine (for now) and file an INFRA ticket to
fix that machine.

On Thu, Jun 8, 2017 at 9:42 AM, Jared Stewart  wrote:

> It looks like the most recent nightly build failed due to an invalid
> JAVA_HOME directory.  Do we have any control over this, or should a ticket
> be filed with Apache Infra?
>
> Thanks,
> Jared
>
> > Begin forwarded message:
> >
> > From: Apache Jenkins Server 
> > Subject: Build failed in Jenkins: Geode-nightly #860
> > Date: June 7, 2017 at 8:43:00 PM PDT
> > To: dev@geode.apache.org, kmil...@pivotal.io, lgalli...@pivotal.io,
> aba...@pivotal.io, dschnei...@pivotal.io, jstew...@pivotal.io,
> dbar...@pivotal.io, kl...@pivotal.io, bogle...@pivotal.io, n...@pivotal.io,
> bschucha...@pivotal.io
> > Reply-To: dev@geode.apache.org
> >
> > See  redirect?page=changes>
> >
> > Changes:
> >
> > [bschuchardt] GEODE-3024 race condition between server locator preparing
> membership
> >
> > [boglesby] GEODE-3040: Compare gateway profile order policy against 9.0
> instead of
> >
> > [dbarnes] GEODE-3044: User Manual: Update Swagger example and screen
> shots -
> >
> > [dbarnes] GEODE-2555: Region Management docs page refers to the wrong
> field (id=
> >
> > [dschneider] GEODE-3027: add a simple PartitionResolver
> >
> > [abaker] GEODE-194: Remove spark connector
> >
> > [dbarnes] GEODE-3044: User Manual: Update Swagger example and screen
> shots -
> >
> > --
> > Started by timer
> > [EnvInject] - Loading node environment variables.
> > Building remotely on mesos2 (mesos s390x) in workspace <
> https://builds.apache.org/job/Geode-nightly/ws/>
> >> git rev-parse --is-inside-work-tree # timeout=10
> > Fetching changes from the remote Git repository
> >> git config remote.origin.url https://git-wip-us.apache.org/
> repos/asf/geode.git # timeout=10
> > Fetching upstream changes from https://git-wip-us.apache.org/
> repos/asf/geode.git
> >> git --version # timeout=10
> >> git fetch --tags --progress https://git-wip-us.apache.org/
> repos/asf/geode.git +refs/heads/*:refs/remotes/origin/*
> >> git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
> >> git rev-parse refs/remotes/origin/origin/develop^{commit} # timeout=10
> > Checking out Revision d73fa3ce289b2108ba352755a68aa8baf9e9b4ca
> (refs/remotes/origin/develop)
> >> git config core.sparsecheckout # timeout=10
> >> git checkout -f d73fa3ce289b2108ba352755a68aa8baf9e9b4ca
> >> git branch -a -v --no-abbrev # timeout=10
> >> git branch -D develop # timeout=10
> >> git checkout -b develop d73fa3ce289b2108ba352755a68aa8baf9e9b4ca
> >> git rev-list 7bad40f368e14182b3cbc88d56505858af3a0ddd # timeout=10
> > [Geode-nightly] $ /bin/bash -xe /tmp/hudson7488740573973842216.sh
> > + git clean -d -f gemfire-pulse/src/main/resources/
> > [Gradle] - Launching build.
> > [Geode-nightly] $  job/Geode-nightly/ws/gradlew> --no-daemon clean precheckin distTar
> distZip uploadArchives -xflakyTest
> >
> > ERROR: JAVA_HOME is set to an invalid directory:
> /home/jenkins/tools/java/latest1.8
> >
> > Please set the JAVA_HOME variable in your environment to match the
> > location of your Java installation.
> >
> > Build step 'Invoke Gradle script' changed build result to FAILURE
> > Build step 'Invoke Gradle script' marked build as failure
> > Archiving artifacts
> > Recording test results
> > ERROR: Step ?Publish JUnit test result report? failed: No test report
> files were found. Configuration error?
> > Not sending mail to unregistered user e...@pivotal.io
> > Not sending mail to unregistered user jil...@pivotal.io
>
>


Fwd: Build failed in Jenkins: Geode-nightly #860

2017-06-08 Thread Jared Stewart
It looks like the most recent nightly build failed due to an invalid JAVA_HOME 
directory.  Do we have any control over this, or should a ticket be filed with 
Apache Infra?  

Thanks,
Jared 

> Begin forwarded message:
> 
> From: Apache Jenkins Server 
> Subject: Build failed in Jenkins: Geode-nightly #860
> Date: June 7, 2017 at 8:43:00 PM PDT
> To: dev@geode.apache.org, kmil...@pivotal.io, lgalli...@pivotal.io, 
> aba...@pivotal.io, dschnei...@pivotal.io, jstew...@pivotal.io, 
> dbar...@pivotal.io, kl...@pivotal.io, bogle...@pivotal.io, n...@pivotal.io, 
> bschucha...@pivotal.io
> Reply-To: dev@geode.apache.org
> 
> See 
> 
> 
> Changes:
> 
> [bschuchardt] GEODE-3024 race condition between server locator preparing 
> membership
> 
> [boglesby] GEODE-3040: Compare gateway profile order policy against 9.0 
> instead of
> 
> [dbarnes] GEODE-3044: User Manual: Update Swagger example and screen shots -
> 
> [dbarnes] GEODE-2555: Region Management docs page refers to the wrong field 
> (id=
> 
> [dschneider] GEODE-3027: add a simple PartitionResolver
> 
> [abaker] GEODE-194: Remove spark connector
> 
> [dbarnes] GEODE-3044: User Manual: Update Swagger example and screen shots -
> 
> --
> Started by timer
> [EnvInject] - Loading node environment variables.
> Building remotely on mesos2 (mesos s390x) in workspace 
> 
>> git rev-parse --is-inside-work-tree # timeout=10
> Fetching changes from the remote Git repository
>> git config remote.origin.url 
>> https://git-wip-us.apache.org/repos/asf/geode.git # timeout=10
> Fetching upstream changes from 
> https://git-wip-us.apache.org/repos/asf/geode.git
>> git --version # timeout=10
>> git fetch --tags --progress 
>> https://git-wip-us.apache.org/repos/asf/geode.git 
>> +refs/heads/*:refs/remotes/origin/*
>> git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
>> git rev-parse refs/remotes/origin/origin/develop^{commit} # timeout=10
> Checking out Revision d73fa3ce289b2108ba352755a68aa8baf9e9b4ca 
> (refs/remotes/origin/develop)
>> git config core.sparsecheckout # timeout=10
>> git checkout -f d73fa3ce289b2108ba352755a68aa8baf9e9b4ca
>> git branch -a -v --no-abbrev # timeout=10
>> git branch -D develop # timeout=10
>> git checkout -b develop d73fa3ce289b2108ba352755a68aa8baf9e9b4ca
>> git rev-list 7bad40f368e14182b3cbc88d56505858af3a0ddd # timeout=10
> [Geode-nightly] $ /bin/bash -xe /tmp/hudson7488740573973842216.sh
> + git clean -d -f gemfire-pulse/src/main/resources/
> [Gradle] - Launching build.
> [Geode-nightly] $  
> --no-daemon clean precheckin distTar distZip uploadArchives -xflakyTest
> 
> ERROR: JAVA_HOME is set to an invalid directory: 
> /home/jenkins/tools/java/latest1.8
> 
> Please set the JAVA_HOME variable in your environment to match the
> location of your Java installation.
> 
> Build step 'Invoke Gradle script' changed build result to FAILURE
> Build step 'Invoke Gradle script' marked build as failure
> Archiving artifacts
> Recording test results
> ERROR: Step ?Publish JUnit test result report? failed: No test report files 
> were found. Configuration error?
> Not sending mail to unregistered user e...@pivotal.io
> Not sending mail to unregistered user jil...@pivotal.io



Re: Requesting assignment permissions for Geode tickets

2017-06-08 Thread Kirk Lund
You've been added to the Role that can assign Geode tickets!

On Thu, Jun 8, 2017 at 9:34 AM, Nick Reich  wrote:

> Hi,
>
> Could I get permissions to assign Geode tickets (especially to myself)?
> User name is nreich.
>
> Thanks,
> Nick
>


Requesting assignment permissions for Geode tickets

2017-06-08 Thread Nick Reich
Hi,

Could I get permissions to assign Geode tickets (especially to myself)?
User name is nreich.

Thanks,
Nick


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread John Blum
+1 to Bruce's comments.

PRs are for contributors that do not have commit privileges.  ReviewBoard
is a tool for "reviewing" changes.

However, what is also common practice on open source projects, even for
committers, is to create a topic branch containing the commit with the
desired changes (labeled with an appropriate JIRA ticket + description).
Then a reviewer can then pick up the topic branch, review the code changes,
polish things up and even merge the topic branch back into the mainline
(e.g. develop) and close the ticket.  IDEs, more than ReviewBoard/FishEye,
etc, have better tooling for reviewing diffs, making change, running tests,
etc.

-John



On Thu, Jun 8, 2017 at 8:32 AM, Bruce Schuchardt 
wrote:

> One thing I find confusing in PRs is whether the person submitting the
> request is a committer or not.  Non-committers need someone to merge the PR
> while committers are just looking for a review and will merge the changes
> to develop themselves.
>
> I also don't see any way to push a PR to specific individuals for review.
> In Reviewboard there is a nice queue of pending reviews that I can go
> through.  On github they're all mixed together and it's difficult to tell
> whether any of them are relevant to me.
>
> I like the idea of a single source of history for reviews but I don't much
> like the idea of having to create PRs on a read-only system and then merge
> my changes to ASF's repo.  Being able to commit directly seems like a
> committer perk that your idea would take away from us.
>
>
> On 6/7/17 6:42 PM, Jacob Barrett wrote:
>
>> All,
>>
>> I would like to discuss transitioning all code reviews to pull requests
>> over using review board. For non-committer community members we ask them
>> to
>> make pull requests against the mirrored GitHub repo. Some committers use
>> pull requests for their own work reviews while others use review board. I
>> propose that we just use on and that the one we use be pull requests. It
>> would give us a single source of history for reviews, a single model to
>> understand for reviewers and committers and keep workflow consistent with
>> all contributors, committers or not.
>>
>> -Jake
>>
>>
>


-- 
-John
john.blum10101 (skype)


Re: [DISCUSS] Using Pull Requests over Review Board

2017-06-08 Thread Bruce Schuchardt
One thing I find confusing in PRs is whether the person submitting the 
request is a committer or not.  Non-committers need someone to merge the 
PR while committers are just looking for a review and will merge the 
changes to develop themselves.


I also don't see any way to push a PR to specific individuals for 
review.  In Reviewboard there is a nice queue of pending reviews that I 
can go through.  On github they're all mixed together and it's difficult 
to tell whether any of them are relevant to me.


I like the idea of a single source of history for reviews but I don't 
much like the idea of having to create PRs on a read-only system and 
then merge my changes to ASF's repo.  Being able to commit directly 
seems like a committer perk that your idea would take away from us.



On 6/7/17 6:42 PM, Jacob Barrett wrote:

All,

I would like to discuss transitioning all code reviews to pull requests
over using review board. For non-committer community members we ask them to
make pull requests against the mirrored GitHub repo. Some committers use
pull requests for their own work reviews while others use review board. I
propose that we just use on and that the one we use be pull requests. It
would give us a single source of history for reviews, a single model to
understand for reviewers and committers and keep workflow consistent with
all contributors, committers or not.

-Jake





Re: Review Request 59904: GEODE-3050: prevent empty dat files

2017-06-08 Thread Jinmei Liao

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


Ship it!




Ship It!

- Jinmei Liao


On June 8, 2017, 12:03 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59904/
> ---
> 
> (Updated June 8, 2017, 12:03 a.m.)
> 
> 
> Review request for geode and Bruce Schuchardt.
> 
> 
> Bugs: GEODE-3050
> https://issues.apache.org/jira/browse/GEODE-3050
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3050: prevent empty dat files
> 
> Move creation of dat files to test methods to avoid overwriting
> full dat file from other test.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java
>  dc73f04309d36fec79558914a81f5eddcbbbe5a9 
> 
> 
> Diff: https://reviews.apache.org/r/59904/diff/1/
> 
> 
> Testing
> ---
> 
> 1) AnalyzeSerializablesJUnitTest, AnalyzeCQSerializablesJUnitTest, 
> AnalyzeWANSerializablesJUnitTest
> 2) precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 59904: GEODE-3050: prevent empty dat files

2017-06-08 Thread Bruce Schuchardt

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


Ship it!




Ship It!

- Bruce Schuchardt


On June 8, 2017, 12:03 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59904/
> ---
> 
> (Updated June 8, 2017, 12:03 a.m.)
> 
> 
> Review request for geode and Bruce Schuchardt.
> 
> 
> Bugs: GEODE-3050
> https://issues.apache.org/jira/browse/GEODE-3050
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3050: prevent empty dat files
> 
> Move creation of dat files to test methods to avoid overwriting
> full dat file from other test.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java
>  dc73f04309d36fec79558914a81f5eddcbbbe5a9 
> 
> 
> Diff: https://reviews.apache.org/r/59904/diff/1/
> 
> 
> Testing
> ---
> 
> 1) AnalyzeSerializablesJUnitTest, AnalyzeCQSerializablesJUnitTest, 
> AnalyzeWANSerializablesJUnitTest
> 2) precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 59863: Removing obsolete SSL handling in `AcceptorImpl.accept` catch block

2017-06-08 Thread Bruce Schuchardt

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


Ship it!




Ship It!

- Bruce Schuchardt


On June 8, 2017, 12:40 a.m., Brian Rowe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59863/
> ---
> 
> (Updated June 8, 2017, 12:40 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3051
> https://issues.apache.org/jira/browse/GEODE-3051
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> SSL handshake is now done in a separate thread and will never reach the 
> handler code which is being removed.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  9658f98da 
> 
> 
> Diff: https://reviews.apache.org/r/59863/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin started
> 
> 
> Thanks,
> 
> Brian Rowe
> 
>



Re: Review Request 59893: GEODE-3032: Fix CI failure of CommandOverHttpDUnitTest

2017-06-08 Thread Ken Howe

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


Ship it!




Ship It!

- Ken Howe


On June 7, 2017, 11:13 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59893/
> ---
> 
> (Updated June 7, 2017, 11:13 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3032: Fix CI failure of CommandOverHttpDUnitTest
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MemberCommandsDUnitTest.java
>  5bbfc5b 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpDUnitTest.java
>  2a6a03b 
> 
> 
> Diff: https://reviews.apache.org/r/59893/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin is green except for unrelated flaky DUnits.
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>



Build failed in Jenkins: Geode-release #57

2017-06-08 Thread Apache Jenkins Server
See 

--
[...truncated 205.67 KB...]
:geode-json:test UP-TO-DATE
:geode-json:check
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJava
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-test-framework/6.4.1/lucene-test-framework-6.4.1.pom
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-codecs/6.4.1/lucene-codecs-6.4.1.pom
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-analyzers-phonetic/6.4.1/lucene-analyzers-phonetic-6.4.1.pom
Download 
https://repo1.maven.org/maven2/com/carrotsearch/randomizedtesting/randomizedtesting-runner/2.4.0/randomizedtesting-runner-2.4.0.pom
Download 
https://repo1.maven.org/maven2/com/carrotsearch/randomizedtesting/randomizedtesting-parent/2.4.0/randomizedtesting-parent-2.4.0.pom
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-test-framework/6.4.1/lucene-test-framework-6.4.1.jar
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-codecs/6.4.1/lucene-codecs-6.4.1.jar
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-analyzers-phonetic/6.4.1/lucene-analyzers-phonetic-6.4.1.jar
Download 
https://repo1.maven.org/maven2/com/carrotsearch/randomizedtesting/randomizedtesting-runner/2.4.0/randomizedtesting-runner-2.4.0.jar
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:geode-lucene:processTestResources
:geode-lucene:testClasses
:geode-lucene:checkMissedTests
:geode-lucene:spotlessJavaCheck
:geode-lucene:spotlessCheck
:geode-lucene:test
:geode-lucene:check
:geode-lucene:build
:geode-lucene:distributedTest
:geode-lucene:integrationTest
:geode-old-client-support:assemble
:geode-old-client-support:compileTestJavaNote: 

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

:geode-old-client-support:processTestResources UP-TO-DATE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest
:geode-old-client-support:integrationTest
:geode-old-versions:javadoc UP-TO-DATE
:geode-old-versions:javadocJar
:geode-old-versions:sourcesJar
:geode-old-versions:signArchives SKIPPED
:geode-old-versions:assemble
:geode-old-versions:compileTestJava UP-TO-DATE
:geode-old-versions:processTestResources UP-TO-DATE
:geode-old-versions:testClasses UP-TO-DATE
:geode-old-versions:checkMissedTests UP-TO-DATE
:geode-old-versions:spotlessJavaCheck
:geode-old-versions:spotlessCheck
:geode-old-versions:test UP-TO-DATE
:geode-old-versions:check
:geode-old-versions:build
:geode-old-versions:distributedTest UP-TO-DATE
:geode-old-versions:integrationTest UP-TO-DATE
:geode-pulse:assemble
:geode-pulse:compileTestJava
Download 
https://repo1.maven.org/maven2/com/codeborne/phantomjsdriver/1.3.0/phantomjsdriver-1.3.0.pom
Download 
https://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-api/3.0.1/selenium-api-3.0.1.pom
Download 
https://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-remote-driver/3.0.1/selenium-remote-driver-3.0.1.pom
Download 
https://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-support/3.0.1/selenium-support-3.0.1.pom
Download 
https://repo1.maven.org/maven2/org/springframework/spring-test/4.3.6.RELEASE/spring-test-4.3.6.RELEASE.pom
Download 
https://repo1.maven.org/maven2/com/codeborne/phantomjsdriver/1.3.0/phantomjsdriver-1.3.0.jar
Download 
https://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-api/3.0.1/selenium-api-3.0.1.jar
Download 
https://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-remote-driver/3.0.1/selenium-remote-driver-3.0.1.jar
Download 
https://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-support/3.0.1/selenium-support-3.0.1.jar
Download 
https://repo1.maven.org/maven2/org/springframework/spring-test/4.3.6.RELEASE/spring-test-4.3.6.RELEASE.jar
Note: 

[GitHub] geode pull request #568: GEODE-290 : Removed deprecated method stopWithPort ...

2017-06-08 Thread deepakddixit
GitHub user deepakddixit opened a pull request:

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

GEODE-290 : Removed deprecated method stopWithPort from LocatorLauncher…

Removed deprecated properties.

Modified test cases to use alternate constants.

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?
 Yes. GEODE-290
- [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?

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

- [x] Have you written or updated unit tests to verify your changes?
Modified existing test cases.
- [x] 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/deepakddixit/incubator-geode GEODE-290

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

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


commit 920a8d13fb5ec221e1359e9ea6887d31f21d15d7
Author: Deepak Dixit 
Date:   2017-06-06T17:09:14Z

GEODE-290 : Removed deprecated method stopWithPort from LocatorLauncher. 
Removed deprecated properties.
Modified test cases to use alternate constants.




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