[GitHub] geode pull request #702: GEODE-3416: Reduce synchronization blockages in Soc...

2017-08-14 Thread kohlmu-pivotal
Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/702#discussion_r133113689
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java ---
@@ -96,46 +99,55 @@ public int getMaxThreads() {
 return this.asyncClosePoolMaxThreads;
   }
 
-  private ThreadPoolExecutor getAsyncThreadExecutor(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool == null) {
-final ThreadGroup tg = 
LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
-ThreadFactory tf = new ThreadFactory() {
-  public Thread newThread(final Runnable command) {
-Thread thread = new Thread(tg, command);
-thread.setDaemon(true);
-return thread;
-  }
-};
-BlockingQueue workQueue = new 
LinkedBlockingQueue();
-pool = new ThreadPoolExecutor(this.asyncClosePoolMaxThreads, 
this.asyncClosePoolMaxThreads,
-this.asyncClosePoolKeepAliveSeconds, TimeUnit.SECONDS, 
workQueue, tf);
-pool.allowCoreThreadTimeOut(true);
-asyncCloseExecutors.put(address, pool);
+  private ExecutorService getAsyncThreadExecutor(String address) {
+ExecutorService executorService = asyncCloseExecutors.get(address);
+if (executorService == null) {
+  // To be used for pre-1.8 jdk releases.
+  // createThreadPool();
+
+  executorService = 
Executors.newWorkStealingPool(asyncClosePoolMaxThreads);
+
+  ExecutorService previousThreadPoolExecutor =
+  asyncCloseExecutors.putIfAbsent(address, executorService);
+
+  if (previousThreadPoolExecutor != null) {
+executorService.shutdownNow();
+return previousThreadPoolExecutor;
   }
-  return pool;
 }
+return executorService;
+  }
+
+  /**
+   * @deprecated this method is to be used for pre 1.8 jdk.
+   */
+  @Deprecated
+  private void createThreadPool() {
+ExecutorService executorService;
+final ThreadGroup threadGroup =
+LoggingThreadGroup.createThreadGroup("Socket asyncClose", logger);
+ThreadFactory threadFactory = new ThreadFactory() {
+  public Thread newThread(final Runnable command) {
+Thread thread = new Thread(threadGroup, command);
+thread.setDaemon(true);
+return thread;
+  }
+};
+
+executorService = new ThreadPoolExecutor(asyncClosePoolMaxThreads, 
asyncClosePoolMaxThreads,
+asyncCloseWaitTime, asyncCloseWaitUnits, new 
LinkedBlockingQueue<>(), threadFactory);
   }
 
   /**
* Call this method if you know all the resources in the closer for the 
given address are no
* longer needed. Currently a thread pool is kept for each address and 
if you know that an address
* no longer needs its pool then you should call this method.
*/
-  public void releaseResourcesForAddress(String address) {
-synchronized (asyncCloseExecutors) {
-  ThreadPoolExecutor pool = asyncCloseExecutors.get(address);
-  if (pool != null) {
-pool.shutdown();
-asyncCloseExecutors.remove(address);
-  }
-}
-  }
 
-  private boolean isClosed() {
-synchronized (asyncCloseExecutors) {
-  return this.closed;
+  public void releaseResourcesForAddress(String address) {
+ExecutorService executorService = asyncCloseExecutors.remove(address);
+if (executorService != null) {
+  executorService.shutdown();
--- End diff --

given that remove is on the concurrent hashmap, one should only ever get 
into this once. This should not suffer reentrancy problems.


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


Build failed in Jenkins: Geode-nightly #923

2017-08-14 Thread Apache Jenkins Server
See 


Changes:

[jiliao] GEODE-3328: fix a test failure on windows.

[klund] Revert "GEODE-3328: fix a test failure on windows."

[klund] GEODE-3436: revert recent refactoring of GFSH commands

--
[...truncated 129.88 KB...]
org.apache.geode.test.dunit.RMIException: While invoking 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest$$Lambda$56/1529418967.call
 in VM 1 running on Host asf905.gq1.ygridcore.net with 4 VMs
at org.apache.geode.test.dunit.VM.invoke(VM.java:387)
at org.apache.geode.test.dunit.VM.invoke(VM.java:357)
at org.apache.geode.test.dunit.VM.invoke(VM.java:325)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.setupServer(Tomcat8SessionsClientServerDUnitTest.java:60)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.postSetUp(Tomcat8SessionsClientServerDUnitTest.java:44)

Caused by:
java.net.BindException: Failed to create server socket on  null[40,404]
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:783)
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:745)
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:712)
at 
org.apache.geode.internal.cache.tier.sockets.AcceptorImpl.(AcceptorImpl.java:469)
at 
org.apache.geode.internal.cache.CacheServerImpl.start(CacheServerImpl.java:344)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.lambda$setupServer$f0fd67c5$1(Tomcat8SessionsClientServerDUnitTest.java:65)

Caused by:
java.net.BindException: Address already in use (Bind failed)
at java.net.PlainSocketImpl.socketBind(Native Method)
at 
java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:387)
at java.net.ServerSocket.bind(ServerSocket.java:375)
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:779)
... 5 more

org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest > 
testSanity FAILED
org.apache.geode.test.dunit.RMIException: While invoking 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest$$Lambda$56/1529418967.call
 in VM 1 running on Host asf905.gq1.ygridcore.net with 4 VMs
at org.apache.geode.test.dunit.VM.invoke(VM.java:387)
at org.apache.geode.test.dunit.VM.invoke(VM.java:357)
at org.apache.geode.test.dunit.VM.invoke(VM.java:325)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.setupServer(Tomcat8SessionsClientServerDUnitTest.java:60)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.postSetUp(Tomcat8SessionsClientServerDUnitTest.java:44)

Caused by:
java.net.BindException: Failed to create server socket on  null[40,404]
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:783)
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:745)
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:712)
at 
org.apache.geode.internal.cache.tier.sockets.AcceptorImpl.(AcceptorImpl.java:469)
at 
org.apache.geode.internal.cache.CacheServerImpl.start(CacheServerImpl.java:344)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.lambda$setupServer$f0fd67c5$1(Tomcat8SessionsClientServerDUnitTest.java:65)

Caused by:
java.net.BindException: Address already in use (Bind failed)
at java.net.PlainSocketImpl.socketBind(Native Method)
at 
java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:387)
at java.net.ServerSocket.bind(ServerSocket.java:375)
at 
org.apache.geode.internal.net.SocketCreator.createServerSocket(SocketCreator.java:779)
... 5 more

org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest > 
testInvalidate FAILED
org.apache.geode.test.dunit.RMIException: While invoking 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest$$Lambda$56/1529418967.call
 in VM 1 running on Host asf905.gq1.ygridcore.net with 4 VMs
at org.apache.geode.test.dunit.VM.invoke(VM.java:387)
at org.apache.geode.test.dunit.VM.invoke(VM.java:357)
at org.apache.geode.test.dunit.VM.invoke(VM.java:325)
at 
org.apache.geode.modules.session.Tomcat8SessionsClientServerDUnitTest.setupServer(Tomcat8SessionsClientServerDUnitTest.java:60)
at 

Re: Gitbox enables the full GitHub workflow

2017-08-14 Thread Jacob Barrett
Awesome! Thanks!

On Mon, Aug 14, 2017 at 4:27 PM Mark Bretl  wrote:

> Ticket created, https://issues.apache.org/jira/browse/INFRA-14876
>
> Lets see where it goes...
>
> --Mark
>
> On Mon, Aug 14, 2017 at 2:13 PM, Jacob Barrett 
> wrote:
>
> > Mark, if you want the experience for the next repo you should do it.
> > Thanks!
> >
> > -Jake
> >
> >
> > Sent from my iPhone
> >
> > > On Aug 14, 2017, at 1:17 PM, Ernest Burghardt 
> > wrote:
> > >
> > > Mark, Go ahead and file it and geode-native will be the canaries...
> > >
> > > Thanks,
> > > Ernie
> > >
> > >> On Mon, Aug 14, 2017 at 2:16 PM, Mark Bretl 
> wrote:
> > >>
> > >> Since we have people wanting this and need a small experiment group to
> > >> blaze the trail for us, I think the geode-native would be a good
> choice
> > >> since it has active PRs and commits. Looks like the process to migrate
> > from
> > >> Git@ASF to GitHub is to create an INFRA ticket with type 'Gitbox
> > Request'.
> > >>
> > >> Jake, do you want to file the ticket or I can do it?
> > >>
> > >> --Mark
> > >>
> > >> On Mon, Aug 14, 2017 at 12:42 PM, Jacob Barrett 
> > >> wrote:
> > >>
> > >>> Well, I am not seeing any -1 votes for this switch. Let's make the
> > >> change.
> > >>>
> > >>> Any takers for following up with Inra to make this go?
> > >>>
> > >>> On Mon, Aug 7, 2017 at 6:49 PM Jacob Barrett 
> > >> wrote:
> > >>>
> >  +1 for Gitbox
> > 
> >  This will greatly simplify the workflow committers go through with
> > pull
> >  requests from the community. It will also allow us to close out
> pulls
> > >>> that
> >  go dark. On our local repos we would really have no need to ever
> > >> include
> >  the apache repo so it reduces the number of remotes we need to
> track.
> > 
> >  -Jake
> > 
> >  Sent from my iPhone
> > 
> > > On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik  >
> >  wrote:
> > >
> > > Hi!
> > >
> > > it has just come to my attention that Gitbox at ASF
> > > has been enabling full GitHub workflow (with being
> > > able to click Merge this PR button, etc.) for quite
> > > some time.
> > >
> > > This basically allows a project to have GH as a R/W
> > > repo as opposed to R/O mirror of what we all currnently
> > > have: https://gitbox.apache.org/repos/asf
> > >
> > > Personally I'm not sure I like GH workflow all that much,
> > > but if there's interest -- you can opt-in into Gitbox. Once
> > > you do -- your source of truth moves to GH. You can't
> > > have it both ways with git-wip-us.apache.org and Gitbox.
> > >
> > > Thanks,
> > > Roman.
> > 
> > >>>
> > >>
> >
>


[GitHub] geode pull request #698: Mark ProtoBuf interface as experimental

2017-08-14 Thread asfgit
Github user asfgit closed the pull request at:

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


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


Re: Gitbox enables the full GitHub workflow

2017-08-14 Thread Mark Bretl
Ticket created, https://issues.apache.org/jira/browse/INFRA-14876

Lets see where it goes...

--Mark

On Mon, Aug 14, 2017 at 2:13 PM, Jacob Barrett  wrote:

> Mark, if you want the experience for the next repo you should do it.
> Thanks!
>
> -Jake
>
>
> Sent from my iPhone
>
> > On Aug 14, 2017, at 1:17 PM, Ernest Burghardt 
> wrote:
> >
> > Mark, Go ahead and file it and geode-native will be the canaries...
> >
> > Thanks,
> > Ernie
> >
> >> On Mon, Aug 14, 2017 at 2:16 PM, Mark Bretl  wrote:
> >>
> >> Since we have people wanting this and need a small experiment group to
> >> blaze the trail for us, I think the geode-native would be a good choice
> >> since it has active PRs and commits. Looks like the process to migrate
> from
> >> Git@ASF to GitHub is to create an INFRA ticket with type 'Gitbox
> Request'.
> >>
> >> Jake, do you want to file the ticket or I can do it?
> >>
> >> --Mark
> >>
> >> On Mon, Aug 14, 2017 at 12:42 PM, Jacob Barrett 
> >> wrote:
> >>
> >>> Well, I am not seeing any -1 votes for this switch. Let's make the
> >> change.
> >>>
> >>> Any takers for following up with Inra to make this go?
> >>>
> >>> On Mon, Aug 7, 2017 at 6:49 PM Jacob Barrett 
> >> wrote:
> >>>
>  +1 for Gitbox
> 
>  This will greatly simplify the workflow committers go through with
> pull
>  requests from the community. It will also allow us to close out pulls
> >>> that
>  go dark. On our local repos we would really have no need to ever
> >> include
>  the apache repo so it reduces the number of remotes we need to track.
> 
>  -Jake
> 
>  Sent from my iPhone
> 
> > On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik 
>  wrote:
> >
> > Hi!
> >
> > it has just come to my attention that Gitbox at ASF
> > has been enabling full GitHub workflow (with being
> > able to click Merge this PR button, etc.) for quite
> > some time.
> >
> > This basically allows a project to have GH as a R/W
> > repo as opposed to R/O mirror of what we all currnently
> > have: https://gitbox.apache.org/repos/asf
> >
> > Personally I'm not sure I like GH workflow all that much,
> > but if there's interest -- you can opt-in into Gitbox. Once
> > you do -- your source of truth moves to GH. You can't
> > have it both ways with git-wip-us.apache.org and Gitbox.
> >
> > Thanks,
> > Roman.
> 
> >>>
> >>
>


[GitHub] geode-native pull request #116: GEODE-3439: Split ThinClientRegionInterestTe...

2017-08-14 Thread asfgit
Github user asfgit closed the pull request at:

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


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


[GitHub] geode-native pull request #116: GEODE-3439: Split ThinClientRegionInterestTe...

2017-08-14 Thread pivotal-jbarrett
GitHub user pivotal-jbarrett opened a pull request:

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

GEODE-3439: Split ThinClientRegionInterestTestsN into mulitple tests …

…to avoid random failures.

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

$ git pull https://github.com/pivotal-jbarrett/geode-native 
feature/GEODE-3439

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

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


commit 70d2456b011f52c3c7b77bd362a567d6a0dbfb69
Author: Jacob Barrett 
Date:   2017-08-14T23:04:43Z

GEODE-3439: Split ThinClientRegionInterestTestsN into mulitple tests to 
avoid random failures.




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

2017-08-14 Thread Spring CI

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

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





--
This message is automatically generated by Atlassian Bamboo

Review Request 61627: GEODE-3437: Fix list and describe region tests

2017-08-14 Thread Jared Stewart

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

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


Repository: geode


Description
---

GEODE-3437: Fix list and describe region tests


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListAndDescribeRegionDUnitTest.java
 ab8c69b7cc99c88dd4e928efeb441d7d1a1d9b1b 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 fc7966f5eb2a9ca4c30369a20ce664d3929ecc22 


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


Testing
---

Precheckin running


Thanks,

Jared Stewart



[GitHub] geode pull request #682: GEODE-3393: One-way SSL authentication fails with F...

2017-08-14 Thread asfgit
Github user asfgit closed the pull request at:

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


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


Re: Gitbox enables the full GitHub workflow

2017-08-14 Thread Jacob Barrett
Mark, if you want the experience for the next repo you should do it. Thanks!

-Jake


Sent from my iPhone

> On Aug 14, 2017, at 1:17 PM, Ernest Burghardt  wrote:
> 
> Mark, Go ahead and file it and geode-native will be the canaries...
> 
> Thanks,
> Ernie
> 
>> On Mon, Aug 14, 2017 at 2:16 PM, Mark Bretl  wrote:
>> 
>> Since we have people wanting this and need a small experiment group to
>> blaze the trail for us, I think the geode-native would be a good choice
>> since it has active PRs and commits. Looks like the process to migrate from
>> Git@ASF to GitHub is to create an INFRA ticket with type 'Gitbox Request'.
>> 
>> Jake, do you want to file the ticket or I can do it?
>> 
>> --Mark
>> 
>> On Mon, Aug 14, 2017 at 12:42 PM, Jacob Barrett 
>> wrote:
>> 
>>> Well, I am not seeing any -1 votes for this switch. Let's make the
>> change.
>>> 
>>> Any takers for following up with Inra to make this go?
>>> 
>>> On Mon, Aug 7, 2017 at 6:49 PM Jacob Barrett 
>> wrote:
>>> 
 +1 for Gitbox
 
 This will greatly simplify the workflow committers go through with pull
 requests from the community. It will also allow us to close out pulls
>>> that
 go dark. On our local repos we would really have no need to ever
>> include
 the apache repo so it reduces the number of remotes we need to track.
 
 -Jake
 
 Sent from my iPhone
 
> On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik 
 wrote:
> 
> Hi!
> 
> it has just come to my attention that Gitbox at ASF
> has been enabling full GitHub workflow (with being
> able to click Merge this PR button, etc.) for quite
> some time.
> 
> This basically allows a project to have GH as a R/W
> repo as opposed to R/O mirror of what we all currnently
> have: https://gitbox.apache.org/repos/asf
> 
> Personally I'm not sure I like GH workflow all that much,
> but if there's interest -- you can opt-in into Gitbox. Once
> you do -- your source of truth moves to GH. You can't
> have it both ways with git-wip-us.apache.org and Gitbox.
> 
> Thanks,
> Roman.
 
>>> 
>> 


Re: Gitbox enables the full GitHub workflow

2017-08-14 Thread Ernest Burghardt
Mark, Go ahead and file it and geode-native will be the canaries...

Thanks,
Ernie

On Mon, Aug 14, 2017 at 2:16 PM, Mark Bretl  wrote:

> Since we have people wanting this and need a small experiment group to
> blaze the trail for us, I think the geode-native would be a good choice
> since it has active PRs and commits. Looks like the process to migrate from
> Git@ASF to GitHub is to create an INFRA ticket with type 'Gitbox Request'.
>
> Jake, do you want to file the ticket or I can do it?
>
> --Mark
>
> On Mon, Aug 14, 2017 at 12:42 PM, Jacob Barrett 
> wrote:
>
> > Well, I am not seeing any -1 votes for this switch. Let's make the
> change.
> >
> > Any takers for following up with Inra to make this go?
> >
> > On Mon, Aug 7, 2017 at 6:49 PM Jacob Barrett 
> wrote:
> >
> > > +1 for Gitbox
> > >
> > > This will greatly simplify the workflow committers go through with pull
> > > requests from the community. It will also allow us to close out pulls
> > that
> > > go dark. On our local repos we would really have no need to ever
> include
> > > the apache repo so it reduces the number of remotes we need to track.
> > >
> > > -Jake
> > >
> > > Sent from my iPhone
> > >
> > > > On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik 
> > > wrote:
> > > >
> > > > Hi!
> > > >
> > > > it has just come to my attention that Gitbox at ASF
> > > > has been enabling full GitHub workflow (with being
> > > > able to click Merge this PR button, etc.) for quite
> > > > some time.
> > > >
> > > > This basically allows a project to have GH as a R/W
> > > > repo as opposed to R/O mirror of what we all currnently
> > > > have: https://gitbox.apache.org/repos/asf
> > > >
> > > > Personally I'm not sure I like GH workflow all that much,
> > > > but if there's interest -- you can opt-in into Gitbox. Once
> > > > you do -- your source of truth moves to GH. You can't
> > > > have it both ways with git-wip-us.apache.org and Gitbox.
> > > >
> > > > Thanks,
> > > > Roman.
> > >
> >
>


Re: Gitbox enables the full GitHub workflow

2017-08-14 Thread Mark Bretl
Since we have people wanting this and need a small experiment group to
blaze the trail for us, I think the geode-native would be a good choice
since it has active PRs and commits. Looks like the process to migrate from
Git@ASF to GitHub is to create an INFRA ticket with type 'Gitbox Request'.

Jake, do you want to file the ticket or I can do it?

--Mark

On Mon, Aug 14, 2017 at 12:42 PM, Jacob Barrett  wrote:

> Well, I am not seeing any -1 votes for this switch. Let's make the change.
>
> Any takers for following up with Inra to make this go?
>
> On Mon, Aug 7, 2017 at 6:49 PM Jacob Barrett  wrote:
>
> > +1 for Gitbox
> >
> > This will greatly simplify the workflow committers go through with pull
> > requests from the community. It will also allow us to close out pulls
> that
> > go dark. On our local repos we would really have no need to ever include
> > the apache repo so it reduces the number of remotes we need to track.
> >
> > -Jake
> >
> > Sent from my iPhone
> >
> > > On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik 
> > wrote:
> > >
> > > Hi!
> > >
> > > it has just come to my attention that Gitbox at ASF
> > > has been enabling full GitHub workflow (with being
> > > able to click Merge this PR button, etc.) for quite
> > > some time.
> > >
> > > This basically allows a project to have GH as a R/W
> > > repo as opposed to R/O mirror of what we all currnently
> > > have: https://gitbox.apache.org/repos/asf
> > >
> > > Personally I'm not sure I like GH workflow all that much,
> > > but if there's interest -- you can opt-in into Gitbox. Once
> > > you do -- your source of truth moves to GH. You can't
> > > have it both ways with git-wip-us.apache.org and Gitbox.
> > >
> > > Thanks,
> > > Roman.
> >
>


Re: Gitbox enables the full GitHub workflow

2017-08-14 Thread Jacob Barrett
Well, I am not seeing any -1 votes for this switch. Let's make the change.

Any takers for following up with Inra to make this go?

On Mon, Aug 7, 2017 at 6:49 PM Jacob Barrett  wrote:

> +1 for Gitbox
>
> This will greatly simplify the workflow committers go through with pull
> requests from the community. It will also allow us to close out pulls that
> go dark. On our local repos we would really have no need to ever include
> the apache repo so it reduces the number of remotes we need to track.
>
> -Jake
>
> Sent from my iPhone
>
> > On Aug 7, 2017, at 6:09 PM, Roman Shaposhnik 
> wrote:
> >
> > Hi!
> >
> > it has just come to my attention that Gitbox at ASF
> > has been enabling full GitHub workflow (with being
> > able to click Merge this PR button, etc.) for quite
> > some time.
> >
> > This basically allows a project to have GH as a R/W
> > repo as opposed to R/O mirror of what we all currnently
> > have: https://gitbox.apache.org/repos/asf
> >
> > Personally I'm not sure I like GH workflow all that much,
> > but if there's interest -- you can opt-in into Gitbox. Once
> > you do -- your source of truth moves to GH. You can't
> > have it both ways with git-wip-us.apache.org and Gitbox.
> >
> > Thanks,
> > Roman.
>


[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread DivineEnder
Github user DivineEnder commented on a diff in the pull request:

https://github.com/apache/geode/pull/712#discussion_r133013984
  
--- Diff: geode-old-versions/build.gradle ---
@@ -65,6 +100,71 @@ task createGeodeClasspathsFile  {
 new FileOutputStream(classpathsFile).withStream { fos ->
   versions.store(fos, '')
 }
+
+installsFile.getParentFile().mkdirs();
+
+new FileOutputStream(installsFile).withStream { fos ->
+  project.ext.installs.store(fos, '')
+}
   }
+
+  // Add sourceSets for backwards compatibility, rolling upgrade, and
+// pdx testing.
+  addOldVersion('test100', '1.0.0-incubating')
+  addOldVersion('test110', '1.1.0')
+  addOldVersion('test111', '1.1.1')
+  addOldVersion('test120', '1.2.0')
+
+
+
+//
+//  def downloadUrl = (geodeReleaseUrl != "") ? geodeReleaseUrl :
--- End diff --

Clean up commented code


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


[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread DivineEnder
Github user DivineEnder commented on a diff in the pull request:

https://github.com/apache/geode/pull/712#discussion_r133013329
  
--- Diff: 
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java
 ---
@@ -34,6 +34,7 @@
 import org.apache.geode.cache.query.internal.DefaultQuery;
 import org.apache.geode.internal.cache.BucketNotFoundException;
 import org.apache.geode.internal.cache.PrimaryBucketException;
+import org.apache.geode.internal.cache.tier.sockets.command.Default;
--- End diff --

Organize imports


---
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 #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread DivineEnder
Github user DivineEnder commented on a diff in the pull request:

https://github.com/apache/geode/pull/712#discussion_r133012849
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java
 ---
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.session.tests;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.net.URISyntaxException;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import 
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+/**
+ * This test iterates through the versions of Geode and executes session 
client compatibility with
+ * the current version of Geode.
+ */
+@Category({DistributedTest.class, BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)

+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class TomcatSessionBackwardsCompatibilityTest {
+
+  @Parameterized.Parameters
+  public static Collection data() {
+List result = 
VersionManager.getInstance().getVersionsWithoutCurrent();
+result.removeIf(s -> Integer.parseInt(s) < 120);
+if (result.size() < 1) {
+  throw new RuntimeException("No older versions of Geode were found to 
test against");
+}
+return result;
+  }
+
+  @Rule
+  public transient GfshShellConnectionRule gfsh = new 
GfshShellConnectionRule();
+
+  @Rule
+  public transient LocatorServerStartupRule locatorStartup = new 
LocatorServerStartupRule();
+
+  @Rule
+  public transient TestName testName = new TestName();
+
+  public transient Client client;
+  public transient ContainerManager manager;
+
+  File oldBuild;
+  File oldModules;
+
+  TomcatInstall tomcat7079AndOldModules;
+  TomcatInstall tomcat7079AndCurrentModules;
+  TomcatInstall tomcat8AndOldModules;
+  TomcatInstall tomcat8AndCurrentModules;
+
+  int locatorPort;
+  String classPathTomcat7079;
+  String classPathTomcat8;
+
+  public TomcatSessionBackwardsCompatibilityTest(String version) {
+VersionManager versionManager = VersionManager.getInstance();
+String installLocation = versionManager.getInstall(version);
+oldBuild = new File(installLocation);
+oldModules = new File(installLocation + "/tools/Modules/");
+  }
+
+  protected void startServer(String name, String classPath, int 
locatorPort) throws Exception {
+CommandStringBuilder command = new 
CommandStringBuilder(CliStrings.START_SERVER);
+command.addOption(CliStrings.START_SERVER__NAME, name);
+command.addOption(CliStrings.START_SERVER__SERVER_PORT, "0");
+command.addOption(CliStrings.START_SERVER__CLASSPATH, classPath);
+command.addOption(CliStrings.START_SERVER__LOCATORS, "localhost[" + 
locatorPort + "]");
+gfsh.executeAndVerifyCommand(command.toString());
+  }
+
+  protected void 

[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread DivineEnder
Github user DivineEnder commented on a diff in the pull request:

https://github.com/apache/geode/pull/712#discussion_r133012057
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java 
---
@@ -150,6 +166,66 @@ public TomcatInstall(TomcatVersion version, 
ConnectionType connType, String inst
   }
 
   /**
+   * Copies jars specified by {@link #tomcatRequiredJars} from the {@link 
#getModulePath()} and the
+   * specified other directory passed to the function
+   *
+   * @throws IOException if the {@link #getModulePath()}, installation lib 
directory, or extra
+   * directory passed in contain no files.
+   */
+  private void copyTomcatGeodeReqFiles(String moduleJarDir, String 
extraJarsPath)
--- End diff --

Copy stuff in function into other function


---
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 #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread DivineEnder
Github user DivineEnder commented on a diff in the pull request:

https://github.com/apache/geode/pull/712#discussion_r133011336
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java 
---
@@ -131,15 +142,20 @@ public TomcatInstall(TomcatVersion version, 
ConnectionType connType) throws Exce
* within the installation's 'conf' folder, and {@link 
#updateProperties()} to set the jar
* skipping properties needed to speedup container startup.
*/
-  public TomcatInstall(TomcatVersion version, ConnectionType connType, 
String installDir)
-  throws Exception {
+  public TomcatInstall(TomcatVersion version, ConnectionType connType, 
String installDir,
+  String modulesJarLocation, String extraJarsPath) throws Exception {
 // Does download and install from URL
-super(installDir, version.getDownloadURL(), connType, "tomcat");
+super(installDir, version.getDownloadURL(), connType, "tomcat",
--- End diff --

Refactor both "tomcat" and DEFAULT_MODULE_LOCATION pass down/up into other 
constructors


---
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 #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread DivineEnder
Github user DivineEnder commented on a diff in the pull request:

https://github.com/apache/geode/pull/712#discussion_r133010884
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java
 ---
@@ -134,6 +134,7 @@ public ServerContainer(ContainerInstall install, File 
containerConfigHome,
 
 // Set cacheXML file
 File installXMLFile = install.getCacheXMLFile();
+String path = logDir.getAbsolutePath() + "/" + 
installXMLFile.getName();
--- End diff --

Probably not needed


---
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 #712: GEODE-3434: Allow the modules to be interoperable w...

2017-08-14 Thread jhuynh1
GitHub user jhuynh1 opened a pull request:

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

GEODE-3434: Allow the modules to be interoperable with current and ol…

…der versions of tomcat 7

Modified DeltaSessions to use reflection to handle attributes fields incase 
an earlier tomcat 7 is used
Modified DeltaSession7 and DeltaSession8 to extend from DeltaSession
Added session backward compatibility tests
Modified aseembly build to download old product installs

Thank you for submitting a contribution to Apache Geode.

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

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

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

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

- [ ] Does `gradlew build` run cleanly?

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

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

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


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

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

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

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


commit e9ed8a12f6f8cb9dfeedcec5922069bce2e7b7e3
Author: Jason Huynh 
Date:   2017-08-14T16:02:11Z

GEODE-3434: Allow the modules to be interoperable with current and older 
versions of tomcat 7
Modified DeltaSessions to use reflection to handle attributes fields incase 
an earlier tomcat 7 is used
Modified DeltaSession7 and DeltaSession8 to extend from DeltaSession
Added session backward compatibility tests
Modified aseembly build to download old product installs




---
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 #711: GEODE-3436: revert all commands refactorings

2017-08-14 Thread asfgit
Github user asfgit closed the pull request at:

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


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


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

2017-08-14 Thread pivotal-amurmann
Github user pivotal-amurmann commented on a diff in the pull request:

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

yeah it did 😔
Tried to reformat it, but it insists on making this bad.




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


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

2017-08-14 Thread pivotal-amurmann
Github user pivotal-amurmann commented on a diff in the pull request:

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

good catch! The variables were supposed to be temporary to enable 
IntelliJ's refactoring.


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


[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

2017-08-14 Thread galen-pivotal
Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132878512
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -72,9 +99,15 @@ public static ServerConnection 
makeServerConnection(Socket s, InternalCache c,
 throw new IOException("Acceptor received unknown communication 
mode: " + communicationMode);
   } else {
 protobufProtocolHandler = findClientProtocolMessageHandler();
-return new GenericProtocolServerConnection(s, c, helper, stats, 
hsTimeout, socketBufferSize,
-communicationModeStr, communicationMode, acceptor, 
protobufProtocolHandler,
-securityService);
+authenticatorClass = findStreamAuthenticator(
+
c.getInternalDistributedSystem().getConfig().getProtobufProtocolAuthenticationMode());
+try {
+  return new GenericProtocolServerConnection(s, c, helper, stats, 
hsTimeout,
+  socketBufferSize, communicationModeStr, communicationMode, 
acceptor,
+  protobufProtocolHandler, securityService, 
authenticatorClass.newInstance());
--- End diff --

We find the class exactly once, and create a new instance per protocol 
instance. We then create an instance per `GenericProtocolServerConnection`. 
This is because the authenticator (unlike the protocol handler) is stateful, so 
we can't share it between connections.


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


[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

2017-08-14 Thread galen-pivotal
Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132878378
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -63,6 +65,31 @@ private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
 }
   }
 
+  private static Class 
findStreamAuthenticator(
+  String implementationID) {
+if (authenticatorClass != null) {
+  return authenticatorClass;
+}
+
+synchronized (streamAuthenticatorLoadLock) {
--- End diff --

Once `authenticatorClass` is initialized, there will no longer be any 
synchronization necessary in this method. This means that each thread will need 
to synchronize at most once, which means no locking / cache misses.


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