Build failed in Jenkins: Geode-nightly-flaky #109

[Apache Jenkins Server]

See 

--
Started by upstream project "Geode-nightly" build number 941
originally caused by:
 Started by timer
[EnvInject] - Loading node environment variables.
Building remotely on H26 (couchdbtest ubuntu xenial) 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/*
ERROR: Error fetching remote repo 'origin'
hudson.plugins.git.GitException: Failed to fetch from 
https://git-wip-us.apache.org/repos/asf/geode.git
at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:817)
at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1084)
at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1115)
at hudson.scm.SCM.checkout(SCM.java:495)
at hudson.model.AbstractProject.checkout(AbstractProject.java:1276)
at 
hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:560)
at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
at 
hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:485)
at hudson.model.Run.execute(Run.java:1735)
at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
at hudson.model.ResourceController.execute(ResourceController.java:97)
at hudson.model.Executor.run(Executor.java:405)
Caused by: hudson.plugins.git.GitException: Command "git fetch --tags 
--progress https://git-wip-us.apache.org/repos/asf/geode.git 
+refs/heads/*:refs/remotes/origin/*" returned status code 128:
stdout: 
stderr: fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/' 
not found

at 
org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1924)
at 
org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:1643)
at 
org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$300(CliGitAPIImpl.java:71)
at 
org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:352)
at 
org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:153)
at 
org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:146)
at hudson.remoting.UserRequest.perform(UserRequest.java:153)
at hudson.remoting.UserRequest.perform(UserRequest.java:50)
at hudson.remoting.Request$2.run(Request.java:336)
at 
hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:748)
at ..remote call to H26(Native Method)
at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1545)
at hudson.remoting.UserResponse.retrieve(UserRequest.java:253)
at hudson.remoting.Channel.call(Channel.java:830)
at 
org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.execute(RemoteGitImpl.java:146)
at sun.reflect.GeneratedMethodAccessor866.invoke(Unknown Source)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.invoke(RemoteGitImpl.java:132)
at com.sun.proxy.$Proxy106.execute(Unknown Source)
at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:815)
... 11 more
ERROR: Error fetching remote repo 'origin'
Retrying after 10 seconds
 > 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/*
ERROR: Error fetching remote repo 'origin'
hudson.plugins.git.GitException: Failed to fetch from 
https://git-wip-us.apache.org/repos/asf/geode.git
at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:817)
at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1084)
  

Build failed in Jenkins: Geode-nightly #941

[Apache Jenkins Server]

See 


Changes:

[jstewart] GEODE-3525: Dockerize AcceptanceTests

[jiliao] GEODE-3549: fix the constantly failing flaky tests in

--
[...truncated 90.07 KB...]

org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest > 
offlineStatusCommandShouldSucceedWhenConnected_locator_pid FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitLoudly(GfshScript.java:133)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitIfNecessary(GfshScript.java:112)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshRule.execute(GfshRule.java:64)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.execute(GfshScript.java:105)
at 
org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest$TestConfiguration.startNecessaryMembers(GfshExitCodeStatusCommandsTest.java:384)
at 
org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest.offlineStatusCommandShouldSucceedWhenConnected_locator_pid(GfshExitCodeStatusCommandsTest.java:209)

org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest > 
offlineStatusCommandShouldSucceedWhenNotConnected_locator_host_and_port FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitLoudly(GfshScript.java:133)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitIfNecessary(GfshScript.java:112)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshRule.execute(GfshRule.java:64)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.execute(GfshScript.java:105)
at 
org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest$TestConfiguration.startNecessaryMembers(GfshExitCodeStatusCommandsTest.java:384)
at 
org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest.offlineStatusCommandShouldSucceedWhenNotConnected_locator_host_and_port(GfshExitCodeStatusCommandsTest.java:140)

org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest > 
onlineStatusCommandShouldSucceedWhenConnected_locator_name FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitLoudly(GfshScript.java:133)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitIfNecessary(GfshScript.java:112)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshRule.execute(GfshRule.java:64)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.execute(GfshScript.java:105)
at 
org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest$TestConfiguration.startNecessaryMembers(GfshExitCodeStatusCommandsTest.java:384)
at 
org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest.onlineStatusCommandShouldSucceedWhenConnected_locator_name(GfshExitCodeStatusCommandsTest.java:151)

org.apache.geode.management.internal.cli.shell.GfshExitCodeStatusCommandsTest > 
onlineStatusCommandShouldSucceedWhenConnected_locator_port FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitLoudly(GfshScript.java:133)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.awaitIfNecessary(GfshScript.java:112)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshRule.execute(GfshRule.java:64)
at 
org.apache.geode.test.dunit.rules.gfsh.GfshScript.execute(GfshScript.java:105)
at 
org.apache.geode.management.internal.cli.shell

Re: git repo is gone

[Jacob Barrett]

Have you registered your github account with Apache’s directory? Log into the 
Apache ID portal and add your github. You’ll have access after the next sync. 

> On Sep 1, 2017, at 4:16 PM, Bruce Schuchardt  wrote:
> 
> I am unable to push to the github repo
> 
> > git push origin develop
> ERROR: Permission to apache/geode.git denied to bschuchardt.
> fatal: Could not read from remote repository.
> 
> Please make sure you have the correct access rights
> and the repository exists.
> 
> > git remote -v
> 
> originssh://g...@github.com/apache/geode.git (fetch)
> originssh://g...@github.com/apache/geode.git (push)
> 
> 
>> On 9/1/17 3:51 PM, Jacob Barrett wrote:
>> Gitbox is the mirror now, github is the authoritative now.
>> 
>>> On Sep 1, 2017, at 2:48 PM, Bruce Schuchardt  wrote:
>>> 
>>> So we go directly to github now?  Not gitbox.apache.org?
>>> 
>>> I'm able to clone this: https://gitbox.apache.org/repos/asf/geode.git
>>> 
>>> 
 On 9/1/17 2:37 PM, Jens Deppe wrote:
 TL;DR - Remove references to
 https://git-wip-us.apache.org/repos/asf/geode.git and replace/add ssh://
 g...@github.com/apache/geode.git
 
 In my environment this is what I had for remotes:
 
 $ git remote -v
 apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
 apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
 origin http://github.com/apache/geode.git (fetch)
 origin http://github.com/apache/geode.git (push)
 
 Then I did:
 
 $ git remote remove apache
 $ git remote set-url origin ssh://g...@github.com/apache/geode.git
 
 This adds github as a read/write remote.
 
 --Jens
 
> On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:
> 
> I believe it is. Just looking to see how/what we're supposed to use now.
> 
> --Jens
> 
> On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
> wrote:
> 
>> I'm not able to access the Apache repo anymore
>> 
>> git clone https://git-wip-us.apache.org/repos/asf/geode.git
>> Cloning into 'geode'...
>> fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
>> not found
>> 
>> Is this part of the gitbox migration?
>> 
> 

Re: git repo is gone

[Bruce Schuchardt]

I am unable to push to the github repo

> git push origin develop
ERROR: Permission to apache/geode.git denied to bschuchardt.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

> git remote -v

origin    ssh://g...@github.com/apache/geode.git (fetch)
origin    ssh://g...@github.com/apache/geode.git (push)


On 9/1/17 3:51 PM, Jacob Barrett wrote:

Gitbox is the mirror now, github is the authoritative now.


On Sep 1, 2017, at 2:48 PM, Bruce Schuchardt  wrote:

So we go directly to github now?  Not gitbox.apache.org?

I'm able to clone this: https://gitbox.apache.org/repos/asf/geode.git



On 9/1/17 2:37 PM, Jens Deppe wrote:
TL;DR - Remove references to
https://git-wip-us.apache.org/repos/asf/geode.git and replace/add ssh://
g...@github.com/apache/geode.git

In my environment this is what I had for remotes:

$ git remote -v
apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
origin http://github.com/apache/geode.git (fetch)
origin http://github.com/apache/geode.git (push)

Then I did:

$ git remote remove apache
$ git remote set-url origin ssh://g...@github.com/apache/geode.git

This adds github as a read/write remote.

--Jens


On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:

I believe it is. Just looking to see how/what we're supposed to use now.

--Jens

On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
wrote:


I'm not able to access the Apache repo anymore

git clone https://git-wip-us.apache.org/repos/asf/geode.git
Cloning into 'geode'...
fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
not found

Is this part of the gitbox migration?

Re: git repo is gone

[Jens Deppe]

I'm going to update our Jenkins nightly builds to point at the github repo.

--Jens

On Fri, Sep 1, 2017 at 3:51 PM, Jacob Barrett  wrote:

> Gitbox is the mirror now, github is the authoritative now.
>
> > On Sep 1, 2017, at 2:48 PM, Bruce Schuchardt 
> wrote:
> >
> > So we go directly to github now?  Not gitbox.apache.org?
> >
> > I'm able to clone this: https://gitbox.apache.org/repos/asf/geode.git
> >
> >
> >> On 9/1/17 2:37 PM, Jens Deppe wrote:
> >> TL;DR - Remove references to
> >> https://git-wip-us.apache.org/repos/asf/geode.git and replace/add
> ssh://
> >> g...@github.com/apache/geode.git
> >>
> >> In my environment this is what I had for remotes:
> >>
> >> $ git remote -v
> >> apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
> >> apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
> >> origin http://github.com/apache/geode.git (fetch)
> >> origin http://github.com/apache/geode.git (push)
> >>
> >> Then I did:
> >>
> >> $ git remote remove apache
> >> $ git remote set-url origin ssh://g...@github.com/apache/geode.git
> >>
> >> This adds github as a read/write remote.
> >>
> >> --Jens
> >>
> >>> On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:
> >>>
> >>> I believe it is. Just looking to see how/what we're supposed to use
> now.
> >>>
> >>> --Jens
> >>>
> >>> On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt <
> bschucha...@pivotal.io>
> >>> wrote:
> >>>
>  I'm not able to access the Apache repo anymore
> 
>  git clone https://git-wip-us.apache.org/repos/asf/geode.git
>  Cloning into 'geode'...
>  fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/
> '
>  not found
> 
>  Is this part of the gitbox migration?
> 
> >>>
> >
>

Re: git repo is gone

[Jacob Barrett]

Gitbox is the mirror now, github is the authoritative now.

> On Sep 1, 2017, at 2:48 PM, Bruce Schuchardt  wrote:
> 
> So we go directly to github now?  Not gitbox.apache.org?
> 
> I'm able to clone this: https://gitbox.apache.org/repos/asf/geode.git
> 
> 
>> On 9/1/17 2:37 PM, Jens Deppe wrote:
>> TL;DR - Remove references to
>> https://git-wip-us.apache.org/repos/asf/geode.git and replace/add ssh://
>> g...@github.com/apache/geode.git
>> 
>> In my environment this is what I had for remotes:
>> 
>> $ git remote -v
>> apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
>> apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
>> origin http://github.com/apache/geode.git (fetch)
>> origin http://github.com/apache/geode.git (push)
>> 
>> Then I did:
>> 
>> $ git remote remove apache
>> $ git remote set-url origin ssh://g...@github.com/apache/geode.git
>> 
>> This adds github as a read/write remote.
>> 
>> --Jens
>> 
>>> On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:
>>> 
>>> I believe it is. Just looking to see how/what we're supposed to use now.
>>> 
>>> --Jens
>>> 
>>> On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
>>> wrote:
>>> 
 I'm not able to access the Apache repo anymore
 
 git clone https://git-wip-us.apache.org/repos/asf/geode.git
 Cloning into 'geode'...
 fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
 not found
 
 Is this part of the gitbox migration?
 
>>> 
> 

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

[Spring CI]

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

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





--
This message is automatically generated by Atlassian Bamboo

Re: git repo is gone

[Bruce Schuchardt]

So we go directly to github now?  Not gitbox.apache.org?

I'm able to clone this: https://gitbox.apache.org/repos/asf/geode.git


On 9/1/17 2:37 PM, Jens Deppe wrote:

TL;DR - Remove references to
https://git-wip-us.apache.org/repos/asf/geode.git and replace/add ssh://
g...@github.com/apache/geode.git

In my environment this is what I had for remotes:

$ git remote -v
apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
origin http://github.com/apache/geode.git (fetch)
origin http://github.com/apache/geode.git (push)

Then I did:

$ git remote remove apache
$ git remote set-url origin ssh://g...@github.com/apache/geode.git

This adds github as a read/write remote.

--Jens

On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:


I believe it is. Just looking to see how/what we're supposed to use now.

--Jens

On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
wrote:


I'm not able to access the Apache repo anymore

git clone https://git-wip-us.apache.org/repos/asf/geode.git
Cloning into 'geode'...
fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
not found

Is this part of the gitbox migration?

Re: [Discuss] Use of -1 as Infinite/All for retry related functions...

[Michael Stolz]

We would certainly rather have the time-out set correctly, but one of the
things I've noticed is, sometimes there is just one query or one function
that takes a really long time, and because we keep retrying it with the
same timeout, it keeps timing out each retry. It would probably be much
smarter to use some sort of increasing timeout on the retries until we give
up.

--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: +1-631-835-4771

On Fri, Sep 1, 2017 at 2:07 PM, Mark Hanson  wrote:

> I actually don’t really have a strong opinion one way or the other. I get
> both approaches. If we want to tailor the code to use a timeout instead of
> retry attempts I guess that is fine. It seems kind of like we are
> perpetuating the same API problem, that the LCD approach alleviates, but ok.
>
> It is more complicated to code because now you need to push everything
> through poll or select. Such as the connect. Not that that is a bad thing,
> because it is not. It is just more complicated.
>
> Thanks,
> Mark
>
> http://developerweb.net/viewtopic.php?id=3196  viewtopic.php?id=3196>
> > On Aug 31, 2017, at 3:47 PM, Jacob Barrett  wrote:
> >
> > None of the time spent performing the request is deterministic that’s
> why there are timeouts. I don’t follow your rational for claiming it
> complicated to code.
> >
> >> On Aug 31, 2017, at 3:27 PM, Mark Hanson  wrote:
> >>
> >> The only problem with that is the time to connect to another server is
> non-deterministic. So,  the code one would have to write to enable this
> would involve a select and a bit of not fun code, but in general could be
> not very useful as an API.
> >>
> >> I would say the lowest common denominator approach or the server based
> approach is better.
> >>
> >> Just two cents.
> >>
> >> Thanks,
> >> Mark
> >>> On Aug 31, 2017, at 1:41 PM, Jacob Barrett 
> wrote:
> >>>
> >>> I believe what Bruce was saying is that the behavior should be covered
> by
> >>> timeouts not iteration attempts. If the client is able to successfully
> send
> >>> the command to a server but a failure occurs waiting for a reply we
> would
> >>> not retry. If the client is unable to send the request to a sever
> because
> >>> the connection closes then we would try the next server, and the next,
> up
> >>> to the timeout value.
> >>>
>  On Thu, Aug 31, 2017 at 1:31 PM Mark Hanson 
> wrote:
> 
>  I can also see why the user doing the retries themselves has value.
> As a
>  lowest common denominator approach, pulling the API is sound.
> 
> > On Thu, Aug 31, 2017 at 1:26 PM, Mark Hanson 
> wrote:
> >
> > I think the setRetryAttempts really harks back to the case that
> Bruce was
> > alluding to in which the server goes down. Which is the one valid
> case
>  for
> > this kind of API in theory. Are we say that in that case we don't
> retry?
> > Seems like we are making the API a little less nice for people.
> > As a developer using an API, I want to do as little as possible and
> get
> > the most robust solution possible. This seems to go the wrong
> direction
>  of
> > that kind of intent in a way. I want the client to automatically try
>  every
> > server. I don't ever want to configure the value. I could limit with
> this
> > API and force it to never retry or I could cause it to retry more
> times
> > than I care for it to.  If we are going to get rid of this API in
> > particular, I would favor having it automatically try some number of
> > servers or all, but not retrying at all would not be my choice.
> >
> > On Thu, Aug 31, 2017 at 1:08 PM, Jacob Barrett 
> > wrote:
> >
> >>> On Thu, Aug 31, 2017 at 1:00 PM Mark Hanson 
> wrote:
> >>>
> >>> I would have to go looking, but the key concept is that this is a
>  bigger
> >>> problem.
> >>>
> >>> interval such as the time between retries
> >>> wait as in how long to wait for a response...
> >>>
> >>
> >> All time intervals should be expressed in terms of
> std::chrono::duration
> >> values. A value of std::chrono::duration::zero means don't wait. I
> would
> >> suggest that a negative time not be allowed and that some very
> large,
> >> MAXINT, value could take the place of "forever". There is a ticket
>  already
> >> open and in progress to replace all time based values with
> std::chrono.
> >>
> >>
> >>> retry as how many times to retry after a failure
> >>> attempts as in how many times to do a thing before giving up
> >>> Set of objects as in the setRetryAttempts code which , will try a
> >> number of
> >>> servers before giving up. where n is the number, -1 equals all,
> and 0
> >> means
> >>> (1 server, no retries).
> >>>
> >>
> >> If there are other examples of "iteration" then we should consider
> them
> >> based on what they iterate. I think the consensus on
> 

Re: Geode Clubhouse meeting on the integration of Lucene text search indexes in Geode

[Jagdish Mirani]

Hello everybody:
This is a quick reminder that we're hosting a Geode Clubhouse meeting on
Tuesday, Sept 5th, at 10AM PST. We'll be covering the text search feature
in Geode 1.2.0.

Here's the meeting link: https://pivotal.zoom.us/j/341598449


Have a great weekend.

Regards
Jag



On Tue, Aug 22, 2017 at 6:19 PM, Jagdish Mirani  wrote:

> Hello Geode Enthusiasts:
>
> Pivotal is hosting a Geode Clubhouse meeting on Sept 5th to present and
> demo the integration of Lucene indexes in Geode 1.2.0.
>
> Date: Tuesday, Sept 5th
> Time: 10AM - 10:30AM PST (short meeting, the Lucene text search feature is
> our only agenda item).
> Meeting logistics: please see zoom meetings details below:
> Join from PC, Mac, Linux, iOS or Android: https://pivotal.zoom.us/j/
> 341598449
> 
> Or iPhone one-tap (US Toll): +16468769923 <(646)%20876-9923>,,341598449#
> or +14086380968 <(408)%20638-0968>,,341598449# Or Telephone: Dial: +1 646
> 876 9923 <(646)%20876-9923> (US Toll) or +1 408 638 0968
> <(408)%20638-0968> (US Toll) +62 21 2188 9017 <+62%2021%2021889017>
> (Indonesia Toll) +63 92 3099 0478 (Philippines Toll) +66 2018 2486
> <+66%202%20018%202486> (Thailand Toll) +66 60 003 5790
> <+66%2060%20003%205790> (Thailand Toll) +84 28 4458 2373
> <+84%2028%204458%202373> (Vietnam Toll) +886 277 417 473 (Taiwan Toll) +1
> 877 369 0926 (US Toll Free) +1 877 853 5247 (US Toll Free) Meeting ID: 341
> 598 449 International numbers available: https://pivotal.zoom.us/
> zoomconference?m=FOcMbPXObcTbzL7lnsIko0GM_QvcVUy3
> 
> Or an H.323/SIP room system: H.323: 162.255.37.11 (US West) 162.255.36.11
> (US East) 221.122.88.195 (China) 115.114.131.7 (India) 213.19.144.110
> (EMEA) 202.177.207.158 (Australia) 209.9.211.110 (Hong Kong) 64.211.144.160
> (Brazil) Meeting ID: 341 598 449 SIP: 341598...@zoomcrc.com
>
> Hope you can attend. If not, as with other clubhouse meetings, this
> meeting will be recorded, and I'll send out the link to the recording.
>
> Regards
> Jagdish Mirani
>
> On Fri, Aug 18, 2017 at 11:38 AM, Jagdish Mirani 
> wrote:
>
>> Hello Geode Contributors:
>> For those who may not know me, I'm in Product Marketing at Pivotal, and
>> among other things, I work with our PM and Engineering teams on Geode.
>>
>> Congratulations for the Geode 1.2.0 release which went out about a month
>> ago. The integration of Lucene indexes to enable full text search was a big
>> step forward as it extends the product in to cover a large set of text
>> search-based use cases. Here is a link to the release notes:
>>
>> https://cwiki.apache.org/confluence/display/GEODE/Release+
>> Notes#ReleaseNotes-1.2.0
>>
>> I would like to host a Geode Clubhouse meeting so that Diane Hardman, our
>> PM for this, can go over the specifics of this feature and how it works,
>> including a demo. I am targeting *Tuesday, Sept 5th, 10AM PST* for the
>> meeting.
>>
>> I'll setup the meeting and blast out the logistics to both the dev and
>> user lists. Once I have the logistics sorted out, can someone help with
>> getting this posted on the events calendar on geode.apache.org. Do we
>> normally publicize these project specific meetings on the broader Apache
>> community calendar? If so, who can help with this?
>>
>> Let me know if you have any comments or questions about this.
>>
>> --
>> Regards
>> Jag
>>
>
>
>
> --
> Regards
> Jag
>



-- 
Regards
Jag

Re: git repo is gone

[Michael William Dodge]

Can non-committers use the ssh protocol, too?

Sarge

> On 1 Sep, 2017, at 14:37, Jens Deppe  wrote:
> 
> TL;DR - Remove references to
> https://git-wip-us.apache.org/repos/asf/geode.git and replace/add ssh://
> g...@github.com/apache/geode.git
> 
> In my environment this is what I had for remotes:
> 
> $ git remote -v
> apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
> apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
> origin http://github.com/apache/geode.git (fetch)
> origin http://github.com/apache/geode.git (push)
> 
> Then I did:
> 
> $ git remote remove apache
> $ git remote set-url origin ssh://g...@github.com/apache/geode.git
> 
> This adds github as a read/write remote.
> 
> --Jens
> 
> On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:
> 
>> I believe it is. Just looking to see how/what we're supposed to use now.
>> 
>> --Jens
>> 
>> On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
>> wrote:
>> 
>>> I'm not able to access the Apache repo anymore
>>> 
>>> git clone https://git-wip-us.apache.org/repos/asf/geode.git
>>> Cloning into 'geode'...
>>> fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
>>> not found
>>> 
>>> Is this part of the gitbox migration?
>>> 
>> 
>> 

Re: git repo is gone

[Jens Deppe]

TL;DR - Remove references to
https://git-wip-us.apache.org/repos/asf/geode.git and replace/add ssh://
g...@github.com/apache/geode.git

In my environment this is what I had for remotes:

$ git remote -v
apache https://git-wip-us.apache.org/repos/asf/geode.git (fetch)
apache https://git-wip-us.apache.org/repos/asf/geode.git (push)
origin http://github.com/apache/geode.git (fetch)
origin http://github.com/apache/geode.git (push)

Then I did:

$ git remote remove apache
$ git remote set-url origin ssh://g...@github.com/apache/geode.git

This adds github as a read/write remote.

--Jens

On Fri, Sep 1, 2017 at 2:31 PM, Jens Deppe  wrote:

> I believe it is. Just looking to see how/what we're supposed to use now.
>
> --Jens
>
> On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
> wrote:
>
>> I'm not able to access the Apache repo anymore
>>
>> git clone https://git-wip-us.apache.org/repos/asf/geode.git
>> Cloning into 'geode'...
>> fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
>> not found
>>
>> Is this part of the gitbox migration?
>>
>
>

Re: git repo is gone

[Galen O'Sullivan]

It looks like it's moved. If you go to the URL in a browser it will give
you a hint. This command should work to update your remote:

git remote set-url 
https://gitbox.apache.org/repos/asf/geode.git


running `git remote -v` after should confirm the change.

On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
wrote:

> I'm not able to access the Apache repo anymore
>
> git clone https://git-wip-us.apache.org/repos/asf/geode.git
> Cloning into 'geode'...
> fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
> not found
>
> Is this part of the gitbox migration?
>

Re: git repo is gone

[Jens Deppe]

I believe it is. Just looking to see how/what we're supposed to use now.

--Jens

On Fri, Sep 1, 2017 at 2:30 PM, Bruce Schuchardt 
wrote:

> I'm not able to access the Apache repo anymore
>
> git clone https://git-wip-us.apache.org/repos/asf/geode.git
> Cloning into 'geode'...
> fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/'
> not found
>
> Is this part of the gitbox migration?
>

git repo is gone

[Bruce Schuchardt]

I'm not able to access the Apache repo anymore

git clone https://git-wip-us.apache.org/repos/asf/geode.git
Cloning into 'geode'...
fatal: repository 'https://git-wip-us.apache.org/repos/asf/geode.git/' 
not found


Is this part of the gitbox migration?

Re: Review Request 61978: GEODE-3059: LoadMonitor.connectionClosed incrementing statistics only for client-server connection

[Hitesh Khamesra]

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


Ship it!




Ship It!

- Hitesh Khamesra


On Aug. 30, 2017, 8:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61978/
> ---
> 
> (Updated Aug. 30, 2017, 8:48 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3059
> https://issues.apache.org/jira/browse/GEODE-3059
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stat should only be incremented/decremented for certain kinds of 
> connections.  I've modified it to include protobuf connections.  All of the 
> constant byte identifiers in Acceptor.java have been moved to an enum in 
> CommunicationMode.java.  In that class I've added some "isa" checks to 
> replace the many big "if" checks for different kinds of connection modes.
> 
> A new connection modes will henceforth need to be added to 
> CommunicationMode.java where the appropriate "isa" methods can be updated to 
> include the new mode.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  dea8644bf6604a48b38e0f8a9fcfa48deb4b56c8 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
>  078844f7adc907761c8b0b1b5525874c8338144e 
>   
> geode-core/src/main/java/org/apache/geode/cache/server/internal/LoadMonitor.java
>  1c571a924f5517f7ba1a04e216c3641f96f9ddc4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  83f87eebe3f7cbea628107078e1bafac478a0228 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> e12a409bc556ab74718830ff8036edd6216ef53b 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/ClientHandShake.java
>  f7a39f3c86f261d76133b2bebb864f53be027f1a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2e33af897a852a2f397cf3c1d7f3b20ae3b1d69f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
>  e2612fc45ccf59ee64654380b771835415780f9d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e877852a42009c0cadeaa0e69516bdbcd84e6bd4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListener.java
>  104d88abc99d3b329834f53d558b4684b6f9c226 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListenerAdapter.java
>  7476b4fba5def66c52b1fa26fb3f0f2e4c63fd17 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  690fd83971d9ac9f8ecedf9b89acb7ade887f38e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  6f56e85382ded1668bc51f6c0f2cc4990a493750 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  00e8b8880a5f336bb82c578f29acaf256187bb5c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerHandShakeProcessor.java
>  47e6f3d0bd9cab249e8f92398b116c6111b0e60e 
>   
> geode-core/src/test/java/org/apache/geode/cache/server/internal/LoadMonitorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnectionTest.java
>  ea0001867e5a22025f4dbca4ead172ff25f2af4d 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  cffa05fa49de58187eaab5bb10c7f2a6ed3bf0f5 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  2aa89954659594428ae3f6ed6d7146261d518203 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  fbd582a1fa3c0e7b0bce033924f38d45ba2c2a9e 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/AuthenticationIntegrationTest.java
>  122b8e3ba7ecab407fdedf6be35153a1def728ff 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripLocatorConnectionJUnitTest.java
>  7ee307b4068247245fc02ab1ae7266c7e2e385c3 
> 
> 
> Diff: https://reviews.apache.org/r/61978/diff/3/
> 
> 
> Testing
> ---
> 
> precheckin is running now
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

[GitHub] geode pull request #754: GEODE-3550: Improve snapshot filter testing

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 61978: GEODE-3059: LoadMonitor.connectionClosed incrementing statistics only for client-server connection

[Alexander Murmann]

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


Ship it!




Ship It!

- Alexander Murmann


On Aug. 30, 2017, 8:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61978/
> ---
> 
> (Updated Aug. 30, 2017, 8:48 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3059
> https://issues.apache.org/jira/browse/GEODE-3059
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stat should only be incremented/decremented for certain kinds of 
> connections.  I've modified it to include protobuf connections.  All of the 
> constant byte identifiers in Acceptor.java have been moved to an enum in 
> CommunicationMode.java.  In that class I've added some "isa" checks to 
> replace the many big "if" checks for different kinds of connection modes.
> 
> A new connection modes will henceforth need to be added to 
> CommunicationMode.java where the appropriate "isa" methods can be updated to 
> include the new mode.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  dea8644bf6604a48b38e0f8a9fcfa48deb4b56c8 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
>  078844f7adc907761c8b0b1b5525874c8338144e 
>   
> geode-core/src/main/java/org/apache/geode/cache/server/internal/LoadMonitor.java
>  1c571a924f5517f7ba1a04e216c3641f96f9ddc4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  83f87eebe3f7cbea628107078e1bafac478a0228 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> e12a409bc556ab74718830ff8036edd6216ef53b 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/ClientHandShake.java
>  f7a39f3c86f261d76133b2bebb864f53be027f1a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2e33af897a852a2f397cf3c1d7f3b20ae3b1d69f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
>  e2612fc45ccf59ee64654380b771835415780f9d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e877852a42009c0cadeaa0e69516bdbcd84e6bd4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListener.java
>  104d88abc99d3b329834f53d558b4684b6f9c226 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListenerAdapter.java
>  7476b4fba5def66c52b1fa26fb3f0f2e4c63fd17 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  690fd83971d9ac9f8ecedf9b89acb7ade887f38e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  6f56e85382ded1668bc51f6c0f2cc4990a493750 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  00e8b8880a5f336bb82c578f29acaf256187bb5c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerHandShakeProcessor.java
>  47e6f3d0bd9cab249e8f92398b116c6111b0e60e 
>   
> geode-core/src/test/java/org/apache/geode/cache/server/internal/LoadMonitorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnectionTest.java
>  ea0001867e5a22025f4dbca4ead172ff25f2af4d 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  cffa05fa49de58187eaab5bb10c7f2a6ed3bf0f5 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  2aa89954659594428ae3f6ed6d7146261d518203 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  fbd582a1fa3c0e7b0bce033924f38d45ba2c2a9e 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/AuthenticationIntegrationTest.java
>  122b8e3ba7ecab407fdedf6be35153a1def728ff 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripLocatorConnectionJUnitTest.java
>  7ee307b4068247245fc02ab1ae7266c7e2e385c3 
> 
> 
> Diff: https://reviews.apache.org/r/61978/diff/3/
> 
> 
> Testing
> ---
> 
> precheckin is running now
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

[GitHub] geode pull request #753: GEODE-3283: Expose parallel import and export in gf...

[agingade]

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

https://github.com/apache/geode/pull/753#discussion_r136644610
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java
 ---
@@ -40,9 +40,13 @@ public void execute(FunctionContext context) {
 final Object[] args = (Object[]) context.getArguments();
 final String regionName = (String) args[0];
 final String importFileName = (String) args[1];
-boolean invokeCallbacks = false;
+boolean parallel = false;
 if (args.length > 2) {
--- End diff --

agingade just now Contributor
Don't we need to check for the total args size? which is always 4. What if 
an old gfsh client connected to newer server executes this function (which is 
not supported...yet one could do it)...in that case, won't invoke callbacks 
value get set to parallel?


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[agingade]

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

https://github.com/apache/geode/pull/753#discussion_r136644394
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java
 ---
@@ -40,9 +40,13 @@ public void execute(FunctionContext context) {
 final Object[] args = (Object[]) context.getArguments();
 final String regionName = (String) args[0];
 final String importFileName = (String) args[1];
-boolean invokeCallbacks = false;
+boolean parallel = false;
 if (args.length > 2) {
--- End diff --

Don't we need to check for the total args size? which is always 4. What if 
an old gfsh client connected to newer server executes this function (which is 
not supported...yet one could do it)...in that case, won't invoke callbacks 
value get set to parallel?


---
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 61978: GEODE-3059: LoadMonitor.connectionClosed incrementing statistics only for client-server connection

[Galen O'Sullivan]

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


Fix it, then Ship it!




Looks good! Except for the one comparison that should be fixed. I like that 
you've made `CommunicationMode`s a little more consolidated. 
This is awesome!


geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
Line 504 (original), 504 (patched)


This comparison is changed with byte vs int rules. Probably better to use 
the int comparison.

Thismight be even better as a method implemented on CommunicationMode.


- Galen O'Sullivan


On Aug. 30, 2017, 8:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61978/
> ---
> 
> (Updated Aug. 30, 2017, 8:48 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3059
> https://issues.apache.org/jira/browse/GEODE-3059
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stat should only be incremented/decremented for certain kinds of 
> connections.  I've modified it to include protobuf connections.  All of the 
> constant byte identifiers in Acceptor.java have been moved to an enum in 
> CommunicationMode.java.  In that class I've added some "isa" checks to 
> replace the many big "if" checks for different kinds of connection modes.
> 
> A new connection modes will henceforth need to be added to 
> CommunicationMode.java where the appropriate "isa" methods can be updated to 
> include the new mode.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  dea8644bf6604a48b38e0f8a9fcfa48deb4b56c8 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
>  078844f7adc907761c8b0b1b5525874c8338144e 
>   
> geode-core/src/main/java/org/apache/geode/cache/server/internal/LoadMonitor.java
>  1c571a924f5517f7ba1a04e216c3641f96f9ddc4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  83f87eebe3f7cbea628107078e1bafac478a0228 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> e12a409bc556ab74718830ff8036edd6216ef53b 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/ClientHandShake.java
>  f7a39f3c86f261d76133b2bebb864f53be027f1a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2e33af897a852a2f397cf3c1d7f3b20ae3b1d69f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
>  e2612fc45ccf59ee64654380b771835415780f9d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e877852a42009c0cadeaa0e69516bdbcd84e6bd4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListener.java
>  104d88abc99d3b329834f53d558b4684b6f9c226 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListenerAdapter.java
>  7476b4fba5def66c52b1fa26fb3f0f2e4c63fd17 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  690fd83971d9ac9f8ecedf9b89acb7ade887f38e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  6f56e85382ded1668bc51f6c0f2cc4990a493750 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  00e8b8880a5f336bb82c578f29acaf256187bb5c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerHandShakeProcessor.java
>  47e6f3d0bd9cab249e8f92398b116c6111b0e60e 
>   
> geode-core/src/test/java/org/apache/geode/cache/server/internal/LoadMonitorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnectionTest.java
>  ea0001867e5a22025f4dbca4ead172ff25f2af4d 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  cffa05fa49de58187eaab5bb10c7f2a6ed3bf0f5 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  2aa89954659594428ae3f6ed6d7146261d518203 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  fbd582a1fa3c0e7b0bce033924f38d45ba2c2a9e 
>   
> geode-protobuf/src/test/java/org/apache/geode/protocol/AuthenticationIntegrationTest.java
>  122b8e3ba7ecab407fde

[GitHub] geode pull request #753: GEODE-3283: Expose parallel import and export in gf...

[nreich]

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

https://github.com/apache/geode/pull/753#discussion_r136643154
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/snapshot/RegionSnapshotServiceImpl.java
 ---
@@ -342,6 +342,7 @@ private void exportOnMember(File snapshot, 
SnapshotFormat format, SnapshotOption
 
 long count = 0;
 long start = CachePerfStats.getStatTime();
+snapshot.getParentFile().mkdirs();
--- End diff --

You are correct: if only a relative route to a specific file is provided 
(which would be an error condition in this case), getParentFile() will return 
null. I can handle that case more gracefully.


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[nreich]

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

https://github.com/apache/geode/pull/753#discussion_r136642718
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java
 ---
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ *
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import java.util.stream.IntStream;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class ImportDataIntegrationTest extends SnapshotDataIntegrationTest 
{
--- End diff --

I tried to include only pretty generic code to share between the two tests, 
though (as with any code) it is always possible that they could diverge in the 
future, making the abstract class less / no longer valuable. I am not opposed 
to duplicating the shared code in each location instead if there is a stronger 
preference towards ease of test extension / modification over DRYness.


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[nreich]

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

https://github.com/apache/geode/pull/753#discussion_r136641632
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java
 ---
@@ -87,4 +80,45 @@ public Result exportData(
 }
 return result;
   }
+
+  private Result getFunctionResult(ResultCollector rc) {
+Result result;
+List results = (List) rc.getResult();
+if (results != null) {
+  Object resultObj = results.get(0);
+  if (resultObj instanceof String) {
+result = ResultBuilder.createInfoResult((String) resultObj);
+  } else if (resultObj instanceof Exception) {
+result = ResultBuilder.createGemFireErrorResult(((Exception) 
resultObj).getMessage());
+  } else {
+result = ResultBuilder.createGemFireErrorResult(
+CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, 
CliStrings.EXPORT_DATA));
+  }
+} else {
+  result = ResultBuilder.createGemFireErrorResult(
+  CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, 
CliStrings.EXPORT_DATA));
+}
+return result;
+  }
+
+  private String defaultFileName(String dirPath, String regionName) {
+return new File(dirPath, regionName + 
RegionSnapshotService.SNAPSHOT_FILE_EXTENSION)
+.getAbsolutePath();
+  }
+
+  private Optional validatePath(String filePath, String dirPath, 
boolean parallel) {
--- End diff --

I thought about having them both extend the same abstract base class: that 
would remove the duplicate code. The method is not dependant on state of the 
object, so it could be extracted into a static helper method (suggestions on 
where would be best to put it if this route is desirable?).


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[agingade]

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

https://github.com/apache/geode/pull/753#discussion_r136640863
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/snapshot/RegionSnapshotServiceImpl.java
 ---
@@ -342,6 +342,7 @@ private void exportOnMember(File snapshot, 
SnapshotFormat format, SnapshotOption
 
 long count = 0;
 long start = CachePerfStats.getStatTime();
+snapshot.getParentFile().mkdirs();
--- End diff --

Can getParentFile() return null...How about handling failure during 
mkdirs()...


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[jinmeiliao]

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

https://github.com/apache/geode/pull/753#discussion_r136638437
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java
 ---
@@ -40,9 +40,13 @@ public void execute(FunctionContext context) {
 final Object[] args = (Object[]) context.getArguments();
 final String regionName = (String) args[0];
 final String importFileName = (String) args[1];
-boolean invokeCallbacks = false;
+boolean parallel = false;
 if (args.length > 2) {
--- End diff --

is the args length check necessary? would caller present different args at 
different times? 


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[jinmeiliao]

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

https://github.com/apache/geode/pull/753#discussion_r136639140
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java
 ---
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ *
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import java.util.stream.IntStream;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class ImportDataIntegrationTest extends SnapshotDataIntegrationTest 
{
--- End diff --

In general we would want to discourage usage of abstract test classes and 
in favor of rules. If there is minimum set up needed using the rules, we don't 
mind a few lines of duplicate code for set up to avoid entangling of the tests. 
What if in the future you want to change the setup of one test instead of 
another. Extending a common abstract class makes it harder to add on to the 
test.


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[jinmeiliao]

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

https://github.com/apache/geode/pull/753#discussion_r136638047
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java
 ---
@@ -41,44 +44,34 @@ public Result exportData(
   @CliOption(key = CliStrings.EXPORT_DATA__REGION, mandatory = true,
   optionContext = ConverterHint.REGION_PATH,
   help = CliStrings.EXPORT_DATA__REGION__HELP) String regionName,
-  @CliOption(key = CliStrings.EXPORT_DATA__FILE, mandatory = true,
+  @CliOption(key = CliStrings.EXPORT_DATA__FILE,
   help = CliStrings.EXPORT_DATA__FILE__HELP) String filePath,
+  @CliOption(key = CliStrings.EXPORT_DATA__DIR,
+  help = CliStrings.EXPORT_DATA__DIR__HELP) String dirPath,
   @CliOption(key = CliStrings.MEMBER, optionContext = 
ConverterHint.MEMBERIDNAME,
-  mandatory = true, help = CliStrings.EXPORT_DATA__MEMBER__HELP) 
String memberNameOrId) {
+  mandatory = true, help = CliStrings.EXPORT_DATA__MEMBER__HELP) 
String memberNameOrId,
+  @CliOption(key = CliStrings.EXPORT_DATA__PARALLEL, 
unspecifiedDefaultValue = "false",
--- End diff --

also, since there is a unspecified default value (false), is it really 
necessary to make it a mandatory option if "dir" is specified? can we just make 
it default to false(or true, whatever sensible) if user didn't add --parallel, 
but specified --dir?


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[jinmeiliao]

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

https://github.com/apache/geode/pull/753#discussion_r136638633
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
 ---
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ *
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.nio.file.Files;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class ExportDataIntegrationTest extends SnapshotDataIntegrationTest 
{
+  @Test
+  public void testExport() throws Exception {
+String exportCommand = buildBaseExportCommand()
+.addOption(CliStrings.EXPORT_DATA__FILE, 
getSnapshotFile().toString()).getCommandString();
+gfsh.executeCommand(exportCommand);
--- End diff --

recommend using executeCommandAndVerify


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[jinmeiliao]

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

https://github.com/apache/geode/pull/753#discussion_r136637196
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 ---
@@ -1351,6 +1351,12 @@
   "File to which the exported data will be written. The file must have 
an extension of \".gfd\".";
   public static final String EXPORT_DATA__MEMBER__HELP =
   "Name/Id of a member which hosts the region. The data will be 
exported to the specified file on the host where the member is running.";
+  public static final String EXPORT_DATA__DIR = "directory";
--- End diff --

I think most commands will use "dir" instead of "directory"


---
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 #753: GEODE-3283: Expose parallel import and export in gf...

[jinmeiliao]

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

https://github.com/apache/geode/pull/753#discussion_r136638194
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java
 ---
@@ -87,4 +80,45 @@ public Result exportData(
 }
 return result;
   }
+
+  private Result getFunctionResult(ResultCollector rc) {
+Result result;
+List results = (List) rc.getResult();
+if (results != null) {
+  Object resultObj = results.get(0);
+  if (resultObj instanceof String) {
+result = ResultBuilder.createInfoResult((String) resultObj);
+  } else if (resultObj instanceof Exception) {
+result = ResultBuilder.createGemFireErrorResult(((Exception) 
resultObj).getMessage());
+  } else {
+result = ResultBuilder.createGemFireErrorResult(
+CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, 
CliStrings.EXPORT_DATA));
+  }
+} else {
+  result = ResultBuilder.createGemFireErrorResult(
+  CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, 
CliStrings.EXPORT_DATA));
+}
+return result;
+  }
+
+  private String defaultFileName(String dirPath, String regionName) {
+return new File(dirPath, regionName + 
RegionSnapshotService.SNAPSHOT_FILE_EXTENSION)
+.getAbsolutePath();
+  }
+
+  private Optional validatePath(String filePath, String dirPath, 
boolean parallel) {
--- End diff --

this seems to be duplicated between import/export, any way we can use the 
same code base?


---
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: [VOTE] Use of -1 as Infinite/All for retry related functions...

[Bruce Schuchardt]

+1


On 9/1/17 11:19 AM, Jacob Barrett wrote:

Remove retry attempts option and use existing timeouts  as sole abort mechanism.

If you -1 please include constructive reasoning.

-Jake




On Sep 1, 2017, at 11:07 AM, Mark Hanson  wrote:

I actually don’t really have a strong opinion one way or the other. I get both 
approaches. If we want to tailor the code to use a timeout instead of retry 
attempts I guess that is fine. It seems kind of like we are perpetuating the 
same API problem, that the LCD approach alleviates, but ok.

It is more complicated to code because now you need to push everything through 
poll or select. Such as the connect. Not that that is a bad thing, because it 
is not. It is just more complicated.

Thanks,
Mark

http://developerweb.net/viewtopic.php?id=3196 


On Aug 31, 2017, at 3:47 PM, Jacob Barrett  wrote:

None of the time spent performing the request is deterministic that’s why there 
are timeouts. I don’t follow your rational for claiming it complicated to code.


On Aug 31, 2017, at 3:27 PM, Mark Hanson  wrote:

The only problem with that is the time to connect to another server is 
non-deterministic. So,  the code one would have to write to enable this would 
involve a select and a bit of not fun code, but in general could be not very 
useful as an API.

I would say the lowest common denominator approach or the server based approach 
is better.

Just two cents.

Thanks,
Mark

On Aug 31, 2017, at 1:41 PM, Jacob Barrett  wrote:

I believe what Bruce was saying is that the behavior should be covered by
timeouts not iteration attempts. If the client is able to successfully send
the command to a server but a failure occurs waiting for a reply we would
not retry. If the client is unable to send the request to a sever because
the connection closes then we would try the next server, and the next, up
to the timeout value.


On Thu, Aug 31, 2017 at 1:31 PM Mark Hanson  wrote:

I can also see why the user doing the retries themselves has value. As a
lowest common denominator approach, pulling the API is sound.


On Thu, Aug 31, 2017 at 1:26 PM, Mark Hanson  wrote:

I think the setRetryAttempts really harks back to the case that Bruce was
alluding to in which the server goes down. Which is the one valid case

for

this kind of API in theory. Are we say that in that case we don't retry?
Seems like we are making the API a little less nice for people.
As a developer using an API, I want to do as little as possible and get
the most robust solution possible. This seems to go the wrong direction

of

that kind of intent in a way. I want the client to automatically try

every

server. I don't ever want to configure the value. I could limit with this
API and force it to never retry or I could cause it to retry more times
than I care for it to.  If we are going to get rid of this API in
particular, I would favor having it automatically try some number of
servers or all, but not retrying at all would not be my choice.

On Thu, Aug 31, 2017 at 1:08 PM, Jacob Barrett 
wrote:


On Thu, Aug 31, 2017 at 1:00 PM Mark Hanson  wrote:

I would have to go looking, but the key concept is that this is a

bigger

problem.

interval such as the time between retries
wait as in how long to wait for a response...


All time intervals should be expressed in terms of std::chrono::duration
values. A value of std::chrono::duration::zero means don't wait. I would
suggest that a negative time not be allowed and that some very large,
MAXINT, value could take the place of "forever". There is a ticket

already

open and in progress to replace all time based values with std::chrono.



retry as how many times to retry after a failure
attempts as in how many times to do a thing before giving up
Set of objects as in the setRetryAttempts code which , will try a

number of

servers before giving up. where n is the number, -1 equals all, and 0

means

(1 server, no retries).


If there are other examples of "iteration" then we should consider them
based on what they iterate. I think the consensus on setRetryAttempts is
to
abolish it.

-Jake

[VOTE] Use of -1 as Infinite/All for retry related functions...

[Jacob Barrett]

Remove retry attempts option and use existing timeouts  as sole abort mechanism.

If you -1 please include constructive reasoning.

-Jake



> On Sep 1, 2017, at 11:07 AM, Mark Hanson  wrote:
> 
> I actually don’t really have a strong opinion one way or the other. I get 
> both approaches. If we want to tailor the code to use a timeout instead of 
> retry attempts I guess that is fine. It seems kind of like we are 
> perpetuating the same API problem, that the LCD approach alleviates, but ok.
> 
> It is more complicated to code because now you need to push everything 
> through poll or select. Such as the connect. Not that that is a bad thing, 
> because it is not. It is just more complicated.
> 
> Thanks,
> Mark
> 
> http://developerweb.net/viewtopic.php?id=3196 
> 
>> On Aug 31, 2017, at 3:47 PM, Jacob Barrett  wrote:
>> 
>> None of the time spent performing the request is deterministic that’s why 
>> there are timeouts. I don’t follow your rational for claiming it complicated 
>> to code.
>> 
>>> On Aug 31, 2017, at 3:27 PM, Mark Hanson  wrote:
>>> 
>>> The only problem with that is the time to connect to another server is 
>>> non-deterministic. So,  the code one would have to write to enable this 
>>> would involve a select and a bit of not fun code, but in general could be 
>>> not very useful as an API.
>>> 
>>> I would say the lowest common denominator approach or the server based 
>>> approach is better.
>>> 
>>> Just two cents.
>>> 
>>> Thanks,
>>> Mark
 On Aug 31, 2017, at 1:41 PM, Jacob Barrett  wrote:
 
 I believe what Bruce was saying is that the behavior should be covered by
 timeouts not iteration attempts. If the client is able to successfully send
 the command to a server but a failure occurs waiting for a reply we would
 not retry. If the client is unable to send the request to a sever because
 the connection closes then we would try the next server, and the next, up
 to the timeout value.
 
> On Thu, Aug 31, 2017 at 1:31 PM Mark Hanson  wrote:
> 
> I can also see why the user doing the retries themselves has value. As a
> lowest common denominator approach, pulling the API is sound.
> 
>> On Thu, Aug 31, 2017 at 1:26 PM, Mark Hanson  wrote:
>> 
>> I think the setRetryAttempts really harks back to the case that Bruce was
>> alluding to in which the server goes down. Which is the one valid case
> for
>> this kind of API in theory. Are we say that in that case we don't retry?
>> Seems like we are making the API a little less nice for people.
>> As a developer using an API, I want to do as little as possible and get
>> the most robust solution possible. This seems to go the wrong direction
> of
>> that kind of intent in a way. I want the client to automatically try
> every
>> server. I don't ever want to configure the value. I could limit with this
>> API and force it to never retry or I could cause it to retry more times
>> than I care for it to.  If we are going to get rid of this API in
>> particular, I would favor having it automatically try some number of
>> servers or all, but not retrying at all would not be my choice.
>> 
>> On Thu, Aug 31, 2017 at 1:08 PM, Jacob Barrett 
>> wrote:
>> 
 On Thu, Aug 31, 2017 at 1:00 PM Mark Hanson  wrote:
 
 I would have to go looking, but the key concept is that this is a
> bigger
 problem.
 
 interval such as the time between retries
 wait as in how long to wait for a response...
 
>>> 
>>> All time intervals should be expressed in terms of std::chrono::duration
>>> values. A value of std::chrono::duration::zero means don't wait. I would
>>> suggest that a negative time not be allowed and that some very large,
>>> MAXINT, value could take the place of "forever". There is a ticket
> already
>>> open and in progress to replace all time based values with std::chrono.
>>> 
>>> 
 retry as how many times to retry after a failure
 attempts as in how many times to do a thing before giving up
 Set of objects as in the setRetryAttempts code which , will try a
>>> number of
 servers before giving up. where n is the number, -1 equals all, and 0
>>> means
 (1 server, no retries).
 
>>> 
>>> If there are other examples of "iteration" then we should consider them
>>> based on what they iterate. I think the consensus on setRetryAttempts is
>>> to
>>> abolish it.
>>> 
>>> -Jake
>>> 
>> 
>> 
> 
>>> 
> 

Re: [Discuss] Use of -1 as Infinite/All for retry related functions...

[Mark Hanson]

I actually don’t really have a strong opinion one way or the other. I get both 
approaches. If we want to tailor the code to use a timeout instead of retry 
attempts I guess that is fine. It seems kind of like we are perpetuating the 
same API problem, that the LCD approach alleviates, but ok.

It is more complicated to code because now you need to push everything through 
poll or select. Such as the connect. Not that that is a bad thing, because it 
is not. It is just more complicated.

Thanks,
Mark

http://developerweb.net/viewtopic.php?id=3196 
   
> On Aug 31, 2017, at 3:47 PM, Jacob Barrett  wrote:
> 
> None of the time spent performing the request is deterministic that’s why 
> there are timeouts. I don’t follow your rational for claiming it complicated 
> to code.
> 
>> On Aug 31, 2017, at 3:27 PM, Mark Hanson  wrote:
>> 
>> The only problem with that is the time to connect to another server is 
>> non-deterministic. So,  the code one would have to write to enable this 
>> would involve a select and a bit of not fun code, but in general could be 
>> not very useful as an API.
>> 
>> I would say the lowest common denominator approach or the server based 
>> approach is better.
>> 
>> Just two cents.
>> 
>> Thanks,
>> Mark
>>> On Aug 31, 2017, at 1:41 PM, Jacob Barrett  wrote:
>>> 
>>> I believe what Bruce was saying is that the behavior should be covered by
>>> timeouts not iteration attempts. If the client is able to successfully send
>>> the command to a server but a failure occurs waiting for a reply we would
>>> not retry. If the client is unable to send the request to a sever because
>>> the connection closes then we would try the next server, and the next, up
>>> to the timeout value.
>>> 
 On Thu, Aug 31, 2017 at 1:31 PM Mark Hanson  wrote:
 
 I can also see why the user doing the retries themselves has value. As a
 lowest common denominator approach, pulling the API is sound.
 
> On Thu, Aug 31, 2017 at 1:26 PM, Mark Hanson  wrote:
> 
> I think the setRetryAttempts really harks back to the case that Bruce was
> alluding to in which the server goes down. Which is the one valid case
 for
> this kind of API in theory. Are we say that in that case we don't retry?
> Seems like we are making the API a little less nice for people.
> As a developer using an API, I want to do as little as possible and get
> the most robust solution possible. This seems to go the wrong direction
 of
> that kind of intent in a way. I want the client to automatically try
 every
> server. I don't ever want to configure the value. I could limit with this
> API and force it to never retry or I could cause it to retry more times
> than I care for it to.  If we are going to get rid of this API in
> particular, I would favor having it automatically try some number of
> servers or all, but not retrying at all would not be my choice.
> 
> On Thu, Aug 31, 2017 at 1:08 PM, Jacob Barrett 
> wrote:
> 
>>> On Thu, Aug 31, 2017 at 1:00 PM Mark Hanson  wrote:
>>> 
>>> I would have to go looking, but the key concept is that this is a
 bigger
>>> problem.
>>> 
>>> interval such as the time between retries
>>> wait as in how long to wait for a response...
>>> 
>> 
>> All time intervals should be expressed in terms of std::chrono::duration
>> values. A value of std::chrono::duration::zero means don't wait. I would
>> suggest that a negative time not be allowed and that some very large,
>> MAXINT, value could take the place of "forever". There is a ticket
 already
>> open and in progress to replace all time based values with std::chrono.
>> 
>> 
>>> retry as how many times to retry after a failure
>>> attempts as in how many times to do a thing before giving up
>>> Set of objects as in the setRetryAttempts code which , will try a
>> number of
>>> servers before giving up. where n is the number, -1 equals all, and 0
>> means
>>> (1 server, no retries).
>>> 
>> 
>> If there are other examples of "iteration" then we should consider them
>> based on what they iterate. I think the consensus on setRetryAttempts is
>> to
>> abolish it.
>> 
>> -Jake
>> 
> 
> 
 
>> 

Re: DISCUSS : Monitor the neighbour JVM using neihbour's member-timeout (GEODE-3411)

[Udo Kohlmeyer]

Hi there Aravind,

I have a singular problem with this approach.

If a some members are designated to do more work, and don't have time to 
respond to the cluster that they are alive using the current member 
timeout, then they are not available to accept data. Which means they 
are not effective members of the cluster and we cannot count on them to 
host data or replicates.


This setting is there to safe guard the cluster against non-responsive 
members that cause the whole cluster to be unhealthy if left unchecked 
for too long. This can lead to potential data loss


If you feel that the member timeout is too short for some members, why 
don't you increase the current member timeout?


My opinion is a -1 for changing the current behavior.

--Udo

On 9/1/17 03:46, Aravind Musigumpula wrote:

Hi Brian,

This will help if the user has some member doing a heavy duty when compared to 
others, in this case we need to give such member some extra time to that member.

Thanks,
Aravind Musigumpula


-Original Message-
From: Brian Baynes [mailto:bbay...@pivotal.io]
Sent: Friday, September 01, 2017 4:39 AM
To: dev@geode.apache.org
Subject: Re: DISCUSS : Monitor the neighbour JVM using neihbour's 
member-timeout (GEODE-3411)

Hi, Aravind.

Can you help me understand why this might be a useful feature for Geode?  I see 
that your needs require it, but why would users in general want to allow longer 
timeouts for some members?  This is a significant change with 
backward-compatibility implications, so would be good for the community to 
understand the potential benefit.

Thanks!
Brian

On Mon, Aug 28, 2017 at 12:20 AM, Aravind Musigumpula < 
aravind.musigump...@amdocs.com> wrote:


Hi Team,

We have a requirement to configure  different member timeout for
different members as we need some members to survive in the view for
longer time than the other the members before being kicked out of the
view in case they aren't responding.


1.   Now with the current monitoring system it is not possible to
determine when the member will be kicked out of the view if we
configure different member-timeout's for some required members.

2.   Because if a member is not responding to any heartbeat requests,
the member who is monitoring the non-responding member will initiate
check member request.

3.   In this check member request monitoring member pings the
non-responding member and waits for member-timeout of monitoring
member for a response.

4.   If still there is no response, it will initiate a final suspect
request to coordinator where the coordinator does the final check
waiting for coordinators member-timeout.

5.   If coordinator did not get any response, it will remove the
non-responding member from the view and publishes it.

6.   So, Here the time period for removing a member depends on its
monitoring member's and coordinator's timeout. But the monitoring
member depends on the view but it may change from time to time.

So, now when a monitoring-member doing the check on a member, if we
wait for the non-responding member's timeout instead of the monitoring
member-timeout, then the time when the non-responding member will be
removed from the view depends on its own member-timeout and the
coordinators member-timeout.
Hence we can configure different member-timeout for the required members.

I created a pull request based on the above scenario:
https://github.com/apache/geode/pull/717

Is the above approach correct? Do we have any concerns around this area?
Please give your insights on this issue.

Thanks,
Aravind Musigumpula

This message and the information contained herein is proprietary and
confidential and subject to the Amdocs policy statement,

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


This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

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

Re: Review Request 61978: GEODE-3059: LoadMonitor.connectionClosed incrementing statistics only for client-server connection

[Bruce Schuchardt]

> On Aug. 29, 2017, 3:48 p.m., Alexander Murmann wrote:
> > geode-protobuf/src/test/java/org/apache/geode/protocol/AuthenticationIntegrationTest.java
> > Lines 105 (patched)
> > 
> >
> > We are doing this for the RoundTripCacheConnectionJUnitTest already. So 
> > it's not surprising we need this here as well. However, in the Roundtrip 
> > test we are also calling `SocketCreatorFactory.close()`. I am unsure why we 
> > are doing that since that other test doesn't interact with the 
> > `SocketCreatorFactory`. Is that something we should be doing here as well?

The other test you mentioned creates a SocketCreator with an SSL configuration 
so it needs to clean up SocketCreatorFactory.  This test doesn't do that


- Bruce


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


On Aug. 30, 2017, 1:48 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61978/
> ---
> 
> (Updated Aug. 30, 2017, 1:48 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Hitesh 
> Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-3059
> https://issues.apache.org/jira/browse/GEODE-3059
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The stat should only be incremented/decremented for certain kinds of 
> connections.  I've modified it to include protobuf connections.  All of the 
> constant byte identifiers in Acceptor.java have been moved to an enum in 
> CommunicationMode.java.  In that class I've added some "isa" checks to 
> replace the many big "if" checks for different kinds of connection modes.
> 
> A new connection modes will henceforth need to be added to 
> CommunicationMode.java where the appropriate "isa" methods can be updated to 
> include the new mode.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
>  dea8644bf6604a48b38e0f8a9fcfa48deb4b56c8 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
>  078844f7adc907761c8b0b1b5525874c8338144e 
>   
> geode-core/src/main/java/org/apache/geode/cache/server/internal/LoadMonitor.java
>  1c571a924f5517f7ba1a04e216c3641f96f9ddc4 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
>  83f87eebe3f7cbea628107078e1bafac478a0228 
>   geode-core/src/main/java/org/apache/geode/internal/cache/tier/Acceptor.java 
> e12a409bc556ab74718830ff8036edd6216ef53b 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/ClientHandShake.java
>  f7a39f3c86f261d76133b2bebb864f53be027f1a 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
>  2e33af897a852a2f397cf3c1d7f3b20ae3b1d69f 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CacheClientNotifier.java
>  e2612fc45ccf59ee64654380b771835415780f9d 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientHealthMonitor.java
>  e877852a42009c0cadeaa0e69516bdbcd84e6bd4 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListener.java
>  104d88abc99d3b329834f53d558b4684b6f9c226 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ConnectionListenerAdapter.java
>  7476b4fba5def66c52b1fa26fb3f0f2e4c63fd17 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
>  690fd83971d9ac9f8ecedf9b89acb7ade887f38e 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
>  6f56e85382ded1668bc51f6c0f2cc4990a493750 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
>  00e8b8880a5f336bb82c578f29acaf256187bb5c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerHandShakeProcessor.java
>  47e6f3d0bd9cab249e8f92398b116c6111b0e60e 
>   
> geode-core/src/test/java/org/apache/geode/cache/server/internal/LoadMonitorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnectionTest.java
>  ea0001867e5a22025f4dbca4ead172ff25f2af4d 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactoryTest.java
>  cffa05fa49de58187eaab5bb10c7f2a6ed3bf0f5 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionTest.java
>  2aa89954659594428ae3f6ed6d7146261d518203 
>

Re: Gitbox enables the full GitHub workflow

[Anthony Baker]

See https://issues.apache.org/jira/browse/INFRA-15001 
.

Anthony

> On Aug 22, 2017, at 10:43 AM, Anthony Baker  wrote:
> 
> 
>> 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.
> 
> Now that we’ve got some experience with gitbox on geode-native, are we ready 
> to convert the other repos?
> 
> - geode
> - geode-site
> - geode-examples
> 
> I think we should.
> 
> Anthony
> 

[VOTE] Apache Geode release - v1.2.1 RC2

[Anthony Baker]

This is the second release candidate for Apache Geode, version 1.2.1.
Thanks to all the community members for their contributions to this
release!

*** Please download, test and vote by Thursday, September 7, 0800 hrs
US Pacific. ***

It fixes the following issues:
  
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12318420&version=12341124

Note that we are voting upon the source tags:  rel/v1.2.1.RC2
  
https://git-wip-us.apache.org/repos/asf?p=geode.git;a=commit;h=afcd9817bc28069453861c935f428e89b8c7ff5d
  
https://git-wip-us.apache.org/repos/asf?p=geode-examples.git;a=commit;h=5d034de588a43cdefb8fbb3a6259579785768340

Commit ID:
  afcd9817bc28069453861c935f428e89b8c7ff5d (geode)
  5d034de588a43cdefb8fbb3a6259579785768340 (geode-examples)

Source and binary files:
  https://dist.apache.org/repos/dist/dev/geode/1.2.1.RC2

Maven staging repo:
  https://repository.apache.org/content/repositories/orgapachegeode-1022

Geode's KEYS file containing PGP keys we use to sign the release:
  
https://git-wip-us.apache.org/repos/asf?p=geode.git;a=blob_plain;f=KEYS;hb=HEAD

pub  4096R/C72CFB64 2015-10-01
  Fingerprint=948E 8234 14BE 693A 7F74  ABBE 19DB CAEE C72C FB64


Anthony

RE: DISCUSS : Monitor the neighbour JVM using neihbour's member-timeout (GEODE-3411)

[Aravind Musigumpula]

Hi Brian,

This will help if the user has some member doing a heavy duty when compared to 
others, in this case we need to give such member some extra time to that member.

Thanks,
Aravind Musigumpula 


-Original Message-
From: Brian Baynes [mailto:bbay...@pivotal.io] 
Sent: Friday, September 01, 2017 4:39 AM
To: dev@geode.apache.org
Subject: Re: DISCUSS : Monitor the neighbour JVM using neihbour's 
member-timeout (GEODE-3411)

Hi, Aravind.

Can you help me understand why this might be a useful feature for Geode?  I see 
that your needs require it, but why would users in general want to allow longer 
timeouts for some members?  This is a significant change with 
backward-compatibility implications, so would be good for the community to 
understand the potential benefit.

Thanks!
Brian

On Mon, Aug 28, 2017 at 12:20 AM, Aravind Musigumpula < 
aravind.musigump...@amdocs.com> wrote:

> Hi Team,
>
> We have a requirement to configure  different member timeout for 
> different members as we need some members to survive in the view for 
> longer time than the other the members before being kicked out of the 
> view in case they aren't responding.
>
>
> 1.   Now with the current monitoring system it is not possible to
> determine when the member will be kicked out of the view if we 
> configure different member-timeout's for some required members.
>
> 2.   Because if a member is not responding to any heartbeat requests,
> the member who is monitoring the non-responding member will initiate 
> check member request.
>
> 3.   In this check member request monitoring member pings the
> non-responding member and waits for member-timeout of monitoring 
> member for a response.
>
> 4.   If still there is no response, it will initiate a final suspect
> request to coordinator where the coordinator does the final check 
> waiting for coordinators member-timeout.
>
> 5.   If coordinator did not get any response, it will remove the
> non-responding member from the view and publishes it.
>
> 6.   So, Here the time period for removing a member depends on its
> monitoring member's and coordinator's timeout. But the monitoring 
> member depends on the view but it may change from time to time.
>
> So, now when a monitoring-member doing the check on a member, if we 
> wait for the non-responding member's timeout instead of the monitoring 
> member-timeout, then the time when the non-responding member will be 
> removed from the view depends on its own member-timeout and the 
> coordinators member-timeout.
> Hence we can configure different member-timeout for the required members.
>
> I created a pull request based on the above scenario:
> https://github.com/apache/geode/pull/717
>
> Is the above approach correct? Do we have any concerns around this area?
> Please give your insights on this issue.
>
> Thanks,
> Aravind Musigumpula
>
> This message and the information contained herein is proprietary and 
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer < 
> https://www.amdocs.com/about/email-disclaimer>
>
This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

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