Re: [discuss] RFC for Geode Authentication Expiation and Re-Authentication

[Aaron Lindsey]

Hi Jinmei,

I read through the proposal a few days ago and it sounded good to me. I didn't 
have any questions or concerns.

Aaron


From: Jinmei Liao 
Sent: Tuesday, July 27, 2021 2:26 PM
To: dev@geode.apache.org
Subject: Re: [discuss] RFC for Geode Authentication Expiation and 
Re-Authentication

Calling more feedback on this RFC. I will move this to “Under Development” if 
no objection to its general direction end of this Thursday.

Thanks!

From: Jinmei Liao 
Date: Thursday, July 22, 2021 at 5:37 PM
To: dev@geode.apache.org 
Subject: [discus] RFC for Geode Authentication Expiation and Re-Authentication
Hi, Fellow devs,

Here the feature proposal for the said topic. Please review and provide your 
feedback. Thanks!

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FOn%2BDemand%2BGeode%2BAuthentication%2BExpiration%2Band%2BRe-authenticationdata=04%7C01%7Calindsey%40vmware.com%7C68d6e250da6f4bfab01008d95145418c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637630180175087074%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=14uziP5Ks8hFVwCaVOESa17jp0v30Uf%2FJRYsrJ7xe%2FQ%3Dreserved=0

Jinmei

Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

[Aaron Lindsey]

+1 to keep only "Squash and merge" and "Rebase and merge".

Aaron Lindsey


From: Robert Houghton 
Sent: Monday, June 28, 2021 2:31 PM
To: dev@geode.apache.org
Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

I for one do not like revisionist history (rebase) but understand that linear 
history is easier for bisect.


From: Blake Bender 
Date: Monday, June 28, 2021 at 2:24 PM
To: dev@geode.apache.org 
Subject: RE: Fw: [DISCUSS] Rebase and Squash Options on Github develop
+1, only because I only get one vote.  Merge commits in develop are a scourge, 
IMO, and should be avoided in almost all instances.

Blake


-Original Message-
From: Donal Evans 
Sent: Monday, June 28, 2021 2:22 PM
To: dev@geode.apache.org
Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

I definitely approve of this proposal. I accidentally merged (rather than 
squashed and merged) a PR a while back because my GitHub session had expired 
and refreshing the page after trying to squash and merge resulted in the 
default merge option being used, much to my frustration. I've yet to see any 
explanation of why we would need to merge PRs rather than squash and 
merge/rebase and merge, so removing it as an option just seems like a good idea.

From: Nabarun Nag 
Sent: Monday, June 28, 2021 1:06 PM
To: Owen Nichols ; dev@geode.apache.org 

Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

Just a clarification.

The options I am proposing to be kept in the PRs are:

  *   Squash and merge
  *   Rebase and merge

Regards,
Nabarun

From: Owen Nichols 
Sent: Monday, June 28, 2021 1:03 PM
To: Nabarun Nag ; dev@geode.apache.org 
Subject: Re: Fw: [DISCUSS] Rebase and Squash Options on Github develop

Sounds like a good idea. I can't find any example of an intentional merge 
commit on develop...but plenty of accidental ones.

Upon releases we do a command line merge commit to the main trunk, which should 
still be fine as this proposal only applies to PRs.

I agree with still allowing rebase commits. I have seen several PRs use them to 
good effect.

You can disable merge commits through .asf.yaml, so changing the setting is as 
easy as opening a PR (and, if we don't like it, reverting is just as easy)


---
Sent from Workspace ONE 
Boxer<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwhatisworkspaceone.com%2Fboxerdata=04%7C01%7Calindsey%40vmware.com%7C4e593a623f154464739208d93a7c1a06%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637605126957444163%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=8hqSLfd%2BkE3YWB%2FVzmC1zUFIloQVOKun%2BRjxGpU%2BSx0%3Dreserved=0>

On June 28, 2021 at 12:38:50 PM PDT, Nabarun Nag  wrote:
Hi all,

Last year, we had a discussion on rebase and squash merge PRs to the develop 
branch,

  *   to have linear git history
  *   help with bisecting
  *   easier root cause analysis of failures
  *   easier to backport changes

However, many developers have reached out mentioning that sometimes they have 
clicked the merge button in hurry when they meant to rebase/squash. Once 
clicked there is no going back. All of them suggested that to have the GitHub 
setting on develop branch to rebase and squash merge option and hence there is 
no room for mistakes to occur.

I would like to propose to that we set the GitHub setting on develop PR to 
rebase and squash buttons.

Please do note that this is reversible and can be changed back, hence there is 
no harm in trying it out.


Regards
Nabarun Nag



From: Owen Nichols (Pivotal) 
Sent: Thursday, April 9, 2020 11:45 AM
To: dev@geode.apache.org 
Subject: Re: [REQUEST] Squash merge please

I've noticed quite a few PRs in the last week that were merged with "Merge" 
rather than "Squash and Merge".

While the community consensus was to continue to allow all merge options, 
please try to default to "Squash and Merge" whenever you can to keep history as 
linear as possible. GitHub will save the last method you used in a cookie, 
which helps, but then it's easy to miss when it resets itself back to the 
default of "Merge" some months later because you cleared cookies, changed 
browsers, etc.

> On Oct 22, 2019, at 5:12 PM, Nabarun Nag  wrote:
>
> Hi Geode Committers,
>
> A kind request for using squash commit instead of using merge.
> This will really help us in our bisect operations when a regression/flakiness 
> in the product is introduced. We can automate and go through fewer commits 
> faster, avoiding commits like "spotless fix" and "re-trigger precheck-in" or 
> other minor commits in the merged branch.
>
> Also, please use the commit format : (helps us to know who worked

Re: [DISCUSS] - RFC make key and trust stores reload automatically upon change

[Aaron Lindsey]

It doesn't look like this RFC has received any comments after being open for 
more than 2 weeks. As a reminder, if anyone has feedback on our approach please 
reach out as we are planning to start implementing the solution described.

Thanks,
Aaron


From: Joris Melchior 
Sent: Thursday, February 4, 2021 7:06 AM
To: dev@geode.apache.org
Subject: [DISCUSS] - RFC make key and trust stores reload automatically upon 
change

Hi Geode developers,

I have published an RFC for making the reloading of certs and trust automatic 
in Geode. The RFC can be found on our Wiki: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FMake%2Bkey%2Band%2Btrust%2Bstores%2Breload%2Bautomatically%2Bupon%2Bchangedata=04%7C01%7Calindsey%40vmware.com%7Cbb901557451f4e89437408d8c91e8595%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637480480214986686%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=zm%2F03QQoNH0Ee%2FlxvP3zNEBiYMHMlZY2jkjWbOwLqzY%3Dreserved=0

Please provide feedback by Thursday February 18, 2021.

Thanks,

Joris Melchior

Re: Different binding addresses for traffic & membership

[Aaron Lindsey]

> Is there any way to configure a bind address to be used only for membership?

To your first question, I asked around but I’m not aware of anything like what 
you are looking for. What you are describing does seem like it could become a 
common setup on Kubernetes, but I personally haven’t tried using Geode with 
Istio and Envoy. Please share what you learn!

> I thought that it will be interesting to take a look at how the membership 
> works (how the distributed system is created), to check if at some point I 
> could decouple how the value of "bind-address" parameter is used to configure 
> binding and to indicate other members that they can reach the new member at 
> that hostname. Any comment about what I should check first is welcome.

Maybe someone with more experience in the membership code could comment on this?

Aaron

> On Jan 20, 2021, at 9:07 AM, Alberto Bustamante Reyes 
>  wrote:
> 
> It seems this is not a trendic topic...  Let me share my approach by the 
> moment, maybe this will receive more comments:
> 
> I thought that it will be interesting to take a look at how the membership 
> works (how the distributed system is created), to check if at some point I 
> could decouple how the value of "bind-address" parameter is used to configure 
> binding and to indicate other members that they can reach the new member at 
> that hostname. Any comment about what I should check first is welcome.
> 
> Thanks!
> 
> BR/
> 
> Alberto Bustamante
> 
> 
> 
> 
> 
> 
> De: Alberto Bustamante Reyes 
> Enviado: martes, 19 de enero de 2021 1:45
> Para: dev@geode.apache.org 
> Asunto: Different binding addresses for traffic & membership
> 
> Hi geode-devs,
> 
> I have a question related with Geode & Kubernetes:
> We would like to use Istio with Geode. For that, a sidecar container (Envoy) 
> has to be added in each Geode pod. That sidecar container intercepts and 
> handles all incoming and outgoing traffic for that pod. One of the 
> requirements set by Istio towards applications trying to integrate with it is 
> that the application listening ports need to be bound to either localhost or 
> 0.0.0.0 address (which listens on all interfaces).
> 
> Geode binds the locator and server traffic port by default to 0.0.0.0, but 
> the membership ports are bound to the pod IP.
> And with Envoy listening on the pod IP for incoming traffic and proxying 
> everything towards localhost, applications binding to pod IPs won't receive 
> any traffic.
> 
> We have tried using the "bind-address" parameter, but that doesn't work for 
> our case. Geode binds the listening ports to the configured address, but it 
> also shares that same address to other members in the system as the address 
> to be used to reach it. If we configure that address to localhost, it just 
> won't work.
> 
> Is there any way to configure a bind address to be used only for membership? 
> I have not seen any configuration parameter or property that could be useful 
> to solve this problem, maybe I missed it.
> 
> Thanks in advance,
> 
> BR/
> 
> Alberto Bustamante

Re: [PROPOSAL] Backport fix for GEODE-8620 to 1.13.1

[Aaron Lindsey]

+1

Aaron

> On Oct 21, 2020, at 9:00 AM, Donal Evans  wrote:
> 
> Hi Geode dev,
> 
> I would like to backport the fix for 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FGEODE-8620data=04%7C01%7Calindsey%40vmware.com%7Cc8e451c321f742e42bc508d875da747b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637388928406271442%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=onPxPjhe00BkyZ56doKwRG%2Bqe2s3iB0KEzV6ciIJdvY%3Dreserved=0
>  to the 1.13.1 branch. This bug causes incorrect redundancy levels to be 
> reported for regions in which not all buckets have been created when using 
> the Restore/StatusRedundancy features introduced in 1.13.0. The fix is 
> minimal and has passed all tests run against the develop pipeline.
> 
> Thanks,
> Donal
> 

Re: Member that is shutting down initiate removal of other members from the cluster

[Aaron Lindsey]

Hi Jakov,

Do you see this issue if you drain the worker node before shutting it down? For 
reference: 
https://kubernetes.io/docs/tasks/administer-cluster/safely-drain-node/. I know 
this doesn’t directly answer your questions, but it seems like draining the 
node first would help the members receive the shutdown signal before the TCP 
connections are terminated. For what it’s worth, we also run Geode on 
Kubernetes but haven’t encountered this issue. Knowing whether it happens when 
you drain the node first would help us to be able to reproduce it.

Thanks,
Aaron

On Oct 20, 2020, at 7:49 AM, Jakov Varenina 
mailto:jakov.varen...@est.tech>> wrote:

Hi Ernie,

Thank you really much for trying to help.

Unfortunately we haven't been able to reproduce this issue without k8s.

I would like to share some additional info about this issue. We know that this 
is not the "correct" way to shut down member gracefully, since it's complete 
TCP communication towards the other members is forcefully terminated even 
before it receives shutdown indication. This issue with removals from previous 
mail can be easily avoided by enforcing policy that the TCP connections towards 
other members aren't terminated until member terminates them by itself during 
graceful shutdown.

Another thing is that the removals occur only when when we shut down complete 
k8s node as described in previous mail. When we reproduce the same situation 
without shutting down k8s node then the removals aren't initiated:

"In parallel terminate member TCP communication and initiate gracefully shut 
down the member. What happens then is that availability check over UDP using 
Heartbeat messages towards other members pass, and removals aren't initiated 
towards other members"

Given the above additional test result when using k8s with 1.12 release, and 
due to fact that we were unable to reproduce the issue on geode only, we are 
not sure that the problem is actually in geode. Please feel free to contact me 
by slack or mail for any additional questions or things you would like to 
discuss.

BRs,

Jakov

On 19. 10. 2020. 20:26, Ernie Burghardt wrote:
Hi Jakov,

I'm looking into your question(s)... curious if you've run into this in a 
non-k8s cluster?
Might help focus the investigation...

Thanks,
EB

On 10/13/20, 7:51 AM, "Jakov Varenina" 
mailto:jakov.varen...@est.tech>> wrote:

Hi all,

sorry for bothering, but we have noticed some differences in behavior
between 1.11 and 1.12 releases and need your help in understanding them.

First I would like to mention that we are running geode in Kubernetes.
We perform shutdown of the worker node that is hosting one member(e.g.
coordinator locator). Shutdown procedure affect member in a following way:

1. TCP shared unordered connections towards other members are terminated

2. Member receives graceful shut-down indication and starts with the
shut-down procedure

Usually connections starting to be terminated first and the shut-down
indication comes short after (e.g. ~10 milliseconds in difference). The
step 1. triggers availability check towards the other members for which
TCP connection has been previously lost. At this point of time
coordinator is unaware of ongoing shut-down and assumes that all other
members are actually having issues due to connection loss. Even after
coordinator receives the graceful shut-down indication this process of
availability check is not stopped. What happens later on is that
availability check fail for all members and coordinator initiates their
removal with RemoveMemberMessage. This message is succesfully received
on the other members forcing them to shut-down.

In geode 1.11 everything is same except the fact that availability check
pass and therefore removals aren't initiated.

In logs it can be seen that for both releases TCP availability check
fail, but HeartbeatMessageRequest/HearbeatMessage check fails only on
1.12 and pass on 1.11. In 1.12 release it can be seen that heartbeat
request and heartbeat messages are sent but does not reach their
destination members. RemoveMemberMessage which are sent later on reach
their destination successfully. Does anybody know what was changed in
1.12 that could lead to such difference in behavior?

Additionally, availability check is not stopped when graceful shutdown
is initiated. Do you think that this could be improved, so that member
stops ongoing availability check when detects gracefull shutdown? Just
to add that shutdown procedure is also delayed due to unsuccessful
attempts to estabilsh TCP connections towards the other members.

BRs,
Jakov

Re: [DISCUSS] proposal for WAN support of an ingress proxy

[Aaron Lindsey]

Sounds good to me. I like the idea of using a proxy instead of the 
--hostname-for-clients solution where you cannot specify the particular server 
of which to connect. And it seems good to use the same approach that was used 
for "off platform" clients.

Aaron


From: Bruce Schuchardt 
Sent: Monday, August 3, 2020 9:00 AM
To: dev@geode.apache.org
Subject: [DISCUSS] proposal for WAN support of an ingress proxy

In some environments it’s expensive to provide all server machines with 
externally-resolvable hostnames.  We recently added support for “off platform” 
clients to access servers through an ingress 
proxy
  for this type of environment.  I’m proposing to do the same for inter-cluster 
(“WAN”) communications.

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FWAN%2BConfiguration%2Bfor%2Ban%2BIngress%2BProxydata=02%7C01%7Calindsey%40vmware.com%7C0b91131a69294c16c85808d837c657c1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637320672304750396sdata=V839395xkQA3i2UChAUpFpTnvOGVP%2B8xTqZlHQSE69M%3Dreserved=0

This improvement will allow one cluster to forward events to another cluster 
even though the other cluster’s machines do not have resolvable hostnames.  
Communications will go through a Proxy process hosted in the other cluster with 
a resolvable hostname.  The user-visible changes in this proposal are small, 
changing the “remote-locators” setting to allow for a new syntax.

I’m not proposing to implement a Proxy.  There are numerous Proxy products 
available that could fit into this scheme such as HAProxy, Envoy, and NGIX.

I will probably start with an SNI-based implementation and leverage work 
already done for client/server communications.

Please provide feedback by the end of 14 august.  Thanks!

Re: [DISCUSSION] Stop using the Geode Repository for Feature/WIP Branches

[Aaron Lindsey]

I'm on board with using forks — the exception being Naba's use case for long 
running feature branches where developers actually want to open a PR into the 
branch


From: Bruce Schuchardt 
Sent: Wednesday, June 3, 2020 8:23 AM
To: dev@geode.apache.org
Subject: Re: [DISCUSSION] Stop using the Geode Repository for Feature/WIP 
Branches

Jake, you make some good points that I hadn't considered before.

On 6/2/20, 3:42 PM, "Jacob Barrett"  wrote:

I know this has been brought up multiple times without resolution. I want 
us resolve to ban the use of Geode repository for work in progress, feature 
branches, or any other branches that are not release or support branches. There 
is no reason given the nature of GitHub why you can’t fork the repository to 
contribute.

* Work done on these branches results in the ASF bots updating the 
associated JIRAs and email blasting all of us with your work.

* People don’t clean up these branches, which leads to a mess of branches 
on everyones clones and in the UI.

* All your intermediate commits get synced to the repo, which bloats the 
repo for everyone else. Even your commits you rebase over and force push are 
left in the repo. When you delete your branch these commits are not removed. 
There is no way for us to prune unreferenced commits. Nobody else needs your 
commits outside of what was merged to a production branch.

If anyone has a use case for working directly from Geode repo that can’t 
work from a fork please post it here so we can resolve.

Thanks,
Jake

Re: RFC - Logging to Standard Out

[Aaron Lindsey]

I think this could be moved to "In Development" since there is consensus. I
created a JIRA for it: https://issues.apache.org/jira/browse/GEODE-8077

Re: [DISCUSS] Replace UDP messaging for membership with TCP

[Aaron Lindsey]

This proposal sounds good to me. +1 to using standard security implementation 
based on TLS

> On Apr 1, 2020, at 3:20 PM, Dan Smith  wrote:
> 
>> When we move from a reliable UDP implementation to one based on TCP, we
>> need to think about how to provide reliability on top of TCP.  If you dig
>> into TCP, you’ll find that it tries really hard (sometimes up to 15
>> minutes!!) but doesn’t guarantee message delivery.  Does this matter in
>> practice?  Yes it does--I’ve worked on geode issues where a faulty network
>> cable eventually caused a cluster hang because of a dropped TCP packet.
>> 
> 
> +1. Yes, we are proposing that the new messenger needs to be able to
> recreate the TCP connection and resend any unacked messages.
> 
> -Dan

Re: [DISCUSS] Redundancy Gfsh Commands

[Aaron Lindsey]

Yes, thanks for clarifying.

> On Apr 2, 2020, at 10:12 AM, Donal Evans  wrote:
> 
> Re-sending this from the correct email address. I think the original got
> eaten.
> 
> 
>> From the RFC:
>>> The command will return error status if:
>> I assume this means ERROR or FAILURE (non-success) status. It seems a
>> little confusing that there are both ERROR and FAILURE statuses. Maybe you
>> could add another section, “the command will return failure status if:” to
>> make it clear when it will be returning an error vs a failure. I think this
>> will be important for users who are triggering the redundancy restore
>> operation programmatically to understand the difference.
> 
> 
> This is a little confusing, unfortunately. The issue arises from the fact
> that in addition to the Status of the RestoreRedundancyResults, which can
> be SUCCESS, FAILURE or ERROR and describes the status of a restore
> redundancy operation on one member, a gfsh command can only return either
> SUCCESS or ERROR result statuses when it is called, which represent the
> status of the command across all members. The result status of the command
> and the Status of the RestoreRedundancyResults are separate things,
> although the command's result status will be partially determined from the
> Status values of the RestoreRedundancyResults objects returned by function
> execution on each member.
> 
> The differentiation between FAILURE and ERROR status for
> RestoreRedundancyResults is relevant, since there should be different
> behaviour for "we were unable to create a redundant copy for all buckets in
> a region," which may be due to not enough members hosting that region or
> some other relatively benign problem, and "we encountered an exception
> while attempting to restore redundancy and were unable to complete the
> operation," which could be indicative of a more serious issue.
> 
> I hope this clarifies things somewhat.
> 
> On Thu, Apr 2, 2020 at 8:44 AM Aaron Lindsey 
> wrote:
> 
>>> Would it be reasonable to return error in the case that
>>> all explicitly included region aren't found?
>> 
>> Yes, this sounds reasonable. Thanks for pointing out that subtlety and for
>> updating the RFC.
>> 
>> From the RFC:
>>> The command will return error status if:
>> 
>> I assume this means ERROR or FAILURE (non-success) status. It seems a
>> little confusing that there are both ERROR and FAILURE statuses. Maybe you
>> could add another section, “the command will return failure status if:” to
>> make it clear when it will be returning an error vs a failure. I think this
>> will be important for users who are triggering the redundancy restore
>> operation programmatically to understand the difference.
>> 
>>> On Apr 2, 2020, at 5:18 AM, Jacob Barrett  wrote:
>>> 
>>> 
>>> 
>>>> On Apr 1, 2020, at 10:46 AM, Donal Evans  wrote:
>>>> 
>>>> There's a subtlety with the second no-op case though, since you could
>> have
>>>> a situation where you call the command with no arguments (include all
>>>> regions) and don't find any partitioned regions, which would be fine
>>> 
>>> I think in this case it is not an error since all would mean all that
>> this may apply to.
>>> 
>>>> or you could have a situation where you explicitly include some regions
>> and
>>>> none of them are found, in which case I'm not sure that returning
>> success
>>>> would be correct. Would it be reasonable to return error in the case
>> that
>>>> all explicitly included region aren't found?
>>> 
>>> I believe this should be the behavior. If any one explicitly listed
>> region does not exist an error should result.
>>> 
>>> -Jake
>>> 
>> 
>> 

Re: [DISCUSS] Redundancy Gfsh Commands

[Aaron Lindsey]

> Would it be reasonable to return error in the case that
> all explicitly included region aren't found?

Yes, this sounds reasonable. Thanks for pointing out that subtlety and for 
updating the RFC.

From the RFC:
> The command will return error status if:

I assume this means ERROR or FAILURE (non-success) status. It seems a little 
confusing that there are both ERROR and FAILURE statuses. Maybe you could add 
another section, “the command will return failure status if:” to make it clear 
when it will be returning an error vs a failure. I think this will be important 
for users who are triggering the redundancy restore operation programmatically 
to understand the difference.

> On Apr 2, 2020, at 5:18 AM, Jacob Barrett  wrote:
> 
> 
> 
>> On Apr 1, 2020, at 10:46 AM, Donal Evans  wrote:
>> 
>> There's a subtlety with the second no-op case though, since you could have
>> a situation where you call the command with no arguments (include all
>> regions) and don't find any partitioned regions, which would be fine
> 
> I think in this case it is not an error since all would mean all that this 
> may apply to.
> 
>> or you could have a situation where you explicitly include some regions and
>> none of them are found, in which case I'm not sure that returning success
>> would be correct. Would it be reasonable to return error in the case that
>> all explicitly included region aren't found?
> 
> I believe this should be the behavior. If any one explicitly listed region 
> does not exist an error should result.
> 
> -Jake
> 

Re: [DISCUSS] Redundancy Gfsh Commands

[Aaron Lindsey]

> If at least one redundant copy exists for every bucket in the specified 
> regions, the status of the command will be success. If at least one bucket in 
> a region has zero redundant copies, if there is a member in the system with 
> an older version of Geode or if the restore redundancy function encounters an 
> exception, the command will return error status.

- If a PR is configured with redundant-copies=0 and I run a restore redundancy 
operation, will I get an error?
- Will I get an error if I run this operation when no partitioned regions exist?

I think the proposal should address what should happen in a no-op scenario like 
the two cases above. I think it should return success in these cases because 
the operation did not fail, it just didn’t have to do anything to achieve the 
desired outcome. The reasoning is that this operation will be invoked 
programmatically to ensure data is redundant before shutting down a member. So 
it would be run “just in case”, not because we actually know there is data 
which needs to have redundancy restored.

I also +1 Kirk’s idea of using CompletableFuture


Aaron


> On Mar 31, 2020, at 7:46 AM, Joris Melchior  wrote:
> 
> +1
> 
> I like this idea and Kirk's suggestion to use the CompletableFuture as a
> standard for asynchronous operations.
> 
> On Mon, Mar 30, 2020 at 2:47 PM Donal Evans  wrote:
> 
>> Hey everyone,
>> 
>> An RFC for adding gfsh commands to allow users to restore redundancy to
>> partitioned regions and to easily check the redundancy status of
>> partitioned regions has been posted:
>> https://cwiki.apache.org/confluence/display/GEODE/Redundancy+Gfsh+Commands
>> .
>> 
>> Please review and comment on this RFC by Monday, 04/06/2020.
>> 
>> Please also note that this RFC has been revised from an earlier draft
>> version that some of you may have seen, so the content may have changed.
>> 
>> Thanks,
>> Donal
>> 
> 
> 
> -- 
> *Joris Melchior *
> CF Engineering
> Pivotal Toronto
> 416 877 5427
> 
> “Programs must be written for people to read, and only incidentally for
> machines to execute.” – *Hal Abelson*
> 

Re: [VOTE] Apache Geode 1.12.0.RC4

[Aaron Lindsey]

+1 (non-binding)

Steps I took:
- Built from source and ran unit tests
- Used GFSH to create a locator and server and do some puts/gets
- Checked version in GFSH
- Built and ran all of the examples
- Verified SHAs and signatures

Aaron

> On Mar 30, 2020, at 7:54 AM, Dave Barnes  wrote:
> 
> +1
> Successfully built User Guide with the provided Docker-based toolset.
> Successfully built javadocs.
> 
> On Fri, Mar 27, 2020 at 2:56 PM Dan Smith  wrote:
> 
>> +1
>> 
>> Ran my release check
>> https://github.com/upthewaterspout/geode-release-check
>> 
>> -Dan
>> 
>> On Fri, Mar 27, 2020 at 1:33 PM John Blum  wrote:
>> 
>>> SDG continues to build with the Apache Geode 1.12.0 RC4 bits.
>>> 
>>> https://jenkins.spring.io/job/spring-data-geode/job/master-next/16/
>>> https://github.com/spring-projects/spring-data-geode/commits/master-next
>>> 
>>> +1
>>> 
>>> 
>>> On Fri, Mar 27, 2020 at 1:23 PM Anthony Baker  wrote:
>>> 
 +1
 
 Things I checked:
 - expected files present
 - hashes and signatures present and valid
 - geode, geode-benchmarks, and geode-examples build from source
 - no binaries present in source distributions
 - verified that geode source and binary distributions have the correct
 SHA's
 - able to create a cluster and do region operations using gfsh
 - brief license review, no Cat X dependencies found
 
 Nitpicking:
 
 1) Let's be consistent with names of source distribution and
>> extensions.
 2) NOTICE file copyright date needs to be updated for -native,
 -benchmark, -examples.
 3) Some of the version numbers in LICENSE need to updated to match,
 also need to add asm-5.0.4.jar
 
 Anthony
 
 On Fri, Mar 27, 2020 at 11:37 AM Ernest Burghardt <
>> eburgha...@pivotal.io
 
 wrote:
> 
> Hello Geode Dev Community,
> 
> This is a release candidate for Apache Geode version 1.12.0.RC4.
> Thanks to all the community members for their contributions to this
 release!
> 
> Please do a review and give your feedback, including the checks you
> performed.
> 
> Voting deadline:
> 3PM PST Mon, March 30, 2020.
> 
> Please note that we are voting upon the source tag:
> rel/v1.12.0.RC4
> 
> Release notes:
> 
 
>>> 
>> https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.12.0
> 
> Source and binary distributions:
> https://dist.apache.org/repos/dist/dev/geode/1.12.0.RC4/
> 
> Maven staging repo:
> 
>> https://repository.apache.org/content/repositories/orgapachegeode-1068
> 
> GitHub:
> https://github.com/apache/geode/tree/rel/v1.12.0.RC4
> https://github.com/apache/geode-examples/tree/rel/v1.12.0.RC4
> https://github.com/apache/geode-native/tree/rel/v1.12.0.RC4
> https://github.com/apache/geode-benchmarks/tree/rel/v1.12.0.RC4
> 
> Pipelines:
> 
 
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-12-0-main
> 
 
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-12-0-rc
> 
> Geode's KEYS file containing PGP keys we use to sign the release:
> https://github.com/apache/geode/blob/develop/KEYS
> 
> Command to run geode-examples:
> ./gradlew -PgeodeReleaseUrl=
> https://dist.apache.org/repos/dist/dev/geode/1.12.0.RC4
> -PgeodeRepositoryUrl=
> 
>> https://repository.apache.org/content/repositories/orgapachegeode-1068
> build runAll
> 
> Regards
> Ernie Burghardt
 
>>> 
>>> 
>>> --
>>> -John
>>> Spring Data Team
>>> 
>> 

Re: WAN replication issue in cloud native environments

[Aaron Lindsey]

I thought the deadline for comments was extended until today (27th), so I added 
a new comment on the RFC. I’m confused about the direction we are taking with 
this proposal.

- Aaron

> On Mar 27, 2020, at 11:14 AM, Dan Smith  wrote:
> 
> With this PR, it would be possible to identify servers running with the
>> same ip and port, because now they will be identified by member id. But
>> Bruce realized that it could be a problem if two servers are running in the
>> same JVM, as they will share the same member id. It seems its very unlikely
>> that people are doing it, but its not explicitly prohibited.
>> 
> 
> What is going to happen if a user does set things up this way? The things I
> can think of are:
> 
> 1. When a connection to one of the cache server fails, the client will
> close all of the connections to both. But this doesn't seem like a bad
> outcome, since it's likely the whole server crashed anyway.
> 2. Pings might not reach the correct server - but it looks like we have a
> single ClientHealthMonitor for the server process anyway? So I think the
> pings are getting to the right place.
> 
> If there aren't any other negative outcomes, I think it's ok to proceed
> with the current solution. But I'd also be ok going to ip+port+id.
> 
> I also agree that this use case of a single pool connecting to multiple
> cache servers in the same process doesn't make much sense.
> 
> -Dan

Re: [VOTE] Using Github issues and wiki for geode-kafka-connector project

[Aaron Lindsey]

YES +1

> On Mar 23, 2020, at 7:42 AM, Bruce Schuchardt  wrote:
> 
> +1
> 
> On 3/21/20, 5:17 PM, "Nabarun Nag"  wrote:
> 
>Hello team,
> 
>We are planning to experiment with using Github issues and wiki for the
>Apache project *Geode-Kafka-Connector. *(not Apache Geode project). Please
>do give your vote on this as we need to send the vote link to infra to
>activate it.
> 
>*Why are we doing this ? / Advantages* :
>1. *Unified location* to have documentation, code and issue tracking.
>2. Leverage Github tools like Github pages to create websites hosting
>information about the project.
>3. No separate JIRA accounts or permission required to create issues.
>4. This will have *no impact on the broader Geode community* as right now
>only 3-4 developers involved in this project.
>5. *This is an experiment.* If things do not work out we can always revert
>back to the traditional way of having separate JIRA, documentation,
>websites etc.
> 
>*Precedence*:
>1. Kubernetes uses the github issues
>2. RabbitMQ uses github issues.
> 
> 
>*NOTE: *- Please be cordial and do not use any condescending language and
>absolutely no bullying.
>- Please treat this email as a professional business email and maintain
>email etiquette while replying.
> 
>Regards
>Nabarun
> 
> 
> 

Re: Let's Deprecate the SECURITY_UDP_DHALGO Configuration Property

[Aaron Lindsey]

+1 to deprecating this feature in 1.12

As to whether we should remove this feature in a minor release, I think that, 
as Alexander pointed out, it hinges on the question of whether we perceive 
there to be some security issue in the feature’s implementation.

This statement from the proposal should raise concerns:

> Our algorithm provides only message privacy whereas DTLS provides privacy, 
> tamper-resistance, and message forgery protection

When designing cryptographic protocols we want to guarantee not only privacy of 
the message, but also the integrity and authenticity. TLS, SSH, and IPSec are 
examples of commonly used secure protocols which do just that. Users of 
cryptographic protocols usually expect these guarantees.

I would argue that a protocol such as this does offer a false sense of security 
in that “security” typically implies not just privacy but also integrity and 
authenticity.

- Aaron

> On Mar 2, 2020, at 3:30 PM, Alexander Murmann  wrote:
> 
> I am not sure I am following the argument for removal in a minor version.
> Let's expand the argument and make sure we are all basing this on the same
> assumptions:
> 
> Java has removed insecure encryption algorithms in patch releases. This was
> presumably done because it's really fixing a vulnerability, rather than
> removing a useful feature.
> 
> Are we in agreement that the UDP encryption as it currently exists does not
> deliver on providing security for data in transit? Bill's original
> description to me indicates that there are problems with the current
> implementation, but not that it creates a false sense of security. There
> seem to exist some intuitions around this, but in order to make the right
> call for our users, we might want to make that more concrete. It would be
> unfortunate to break the promises we make via SemVer unintentionally.
> 
> On Mon, Mar 2, 2020 at 11:38 AM Udo Kohlmeyer  wrote:
> 
>> I'm back tracking on my suggestion... Got stuck on the "removal" part.
>> 
>> +1 -> for deprecation for said feature in 1.12
>> 
>> -1 -> for removal until alternative/equivalent replacement has been
>> added (or major version release -- which ever comes first)
>> 
>> @Dan, are you thinking that secured intra-cluster communication needs to
>> be deprecated as a whole?
>> 
>> --Udo
>> 
>> On 3/2/20 10:52 AM, Anthony Baker wrote:
>>> I don’t think regressed is the right word.  There was an effort to
>> harden our udp p2p traffic using encryption based on a DH key exchange.
>> I’m not clear on how much that approach actually improved the security of
>> data in motion.  And I don’t believe it saw much if any use in practice.
>>> 
>>> Anthony
>>> 
>>> 
 On Mar 2, 2020, at 10:27 AM, Udo Kohlmeyer 
>> wrote:
 
 I think that if TCP connections are secured and UDP connections are
>> not, then we've regressed in our security.
 
 Is there a way that we could use DTLS?
 
 --Udo
 
 On 3/2/20 10:21 AM, Dan Smith wrote:
> Should we deprecate cluster ssl as well? Cluster ssl without udp
>> encryption
> means that not all P2P traffic will be encrypted. If we don't want to
> support P2P encryption, it seems like we should deprecate both?
> 
> -Dan
> 
> On Fri, Feb 28, 2020 at 1:40 PM Udo Kohlmeyer 
>> wrote:
> 
>> Yes, the proposal was deprecation notice in 1.12 and remove in 1.13..
>> 
>> That does not leave users much time to react. So if we remove in 1.14,
>> then users have at least 2 release cycles before we do it.
>> 
>> --Udo
>> 
>> On 2/28/20 1:11 PM, Owen Nichols wrote:
>>> Udo, are you proposing to deprecate in 1.12 and remove in 1.14?
>>> 
 On Feb 28, 2020, at 1:03 PM, Udo Kohlmeyer 
>> wrote:
 +1 to adding the deprecation. But I would prefer that we give users
>> more notice than 3 months.
 maybe we deprecate it in 1.14
 
 --Udo
 
 On 2/28/20 1:00 PM, Ernest Burghardt wrote:
> +1
> 
> seems reasonable to do this for 1.12 and be ahead of the game,
>> @Owen
>> would
> you please spawn that as a separate release 1.12 thread? thanks, eB
> 
> On Fri, Feb 28, 2020 at 11:56 AM Owen Nichols >> 
>> wrote:
>> +1
>> 
>> We should have done this as soon as SSL/TLS was ready.  Better
>> late
>> than
>> never!
>> 
>> While we normally wait until a major release to remove deprecated
>> stuff,
>> there is some precedent for removing insecure encryption
>> algorithms
>> sooner
>> (Java has done this even in patch releases).  We should consider
>> getting
>> the deprecation notice into 1.12 and removing in 1.13, rather than
>> waiting
>> for 2.0.
>> 
>>> On Feb 28, 2020, at 11:42 AM, Bill Burcham <
>> bill.burc...@gmail.com>
>> wrote:
>>> I propose we deprecate Geode’s proprietary UDP message 

Re: [DISCUSS] RFC: Shipping Geode patch releases

[Aaron Lindsey]

+1 to the proposal as it’s written.

I’m not sure about setting regular intervals for patch releases vs sticking 
with our current “on-demand” process. I don’t think this has to be spelled out 
in this proposal. I’d be fine with sticking to “on-demand” patch releases as 
we’ve been doing for now and seeing how that goes. If needed we could follow 
that up with a proposal for regular patch release intervals.

- Aaron

> On Feb 26, 2020, at 8:45 AM, Anthony Baker  wrote:
> 
> 
> 
>> On Feb 25, 2020, at 1:42 PM, Udo Kohlmeyer  wrote:
>> 
>> From the proposal it seems we are departing from the initial delivery 
>> paradigm of "always upgrade to the latest version, because all fixes are 
>> going in there", to the more product orientated approach of, there is a 
>> support lifespan for each {major}-{minor} version. Which is a more 
>> traditional product approach.
>> 
>> Is there a specific reason we advocating for moving away from "the fix will 
>> be in the latest release X:Y:Z" ?
>> 
> 
> From the proposal:
> 
> "The Geode project has been following a model of shipping releases quarterly 
> [1][2].  Using the SemVer [3] approach, these quarterly releases are labeled 
> as minor versions.  Occasionally, the project has shipped patch releases [4] 
> to address security vulnerabilities or really critical bugs.  However, on the 
> whole the project has asked users to wait until the next quarterly minor 
> release to receive improvements and fixes.
> 
> It would benefit our user community if we supported our releases for a period 
> of time by back porting important fixes and providing patch releases.”
> 
> I believe that taking this approach will clarify expectations and make it 
> easier for users to rely on Geode for their projects.
> 
> 
>> The current methodology or the community voting on changes being 
>> cherry-picked is related to fixes making it into an unreleased version of 
>> Geode. From the proposal, are we advocating for extra voting "Do we include 
>> fix GEODE-, into N , N-1 or N-2? " This does seem to be something that 
>> might be more work for the community, rather than just updating to latest 
>> RELEASE. Or is the suggestion that closer to each release period, a list of 
>> candidate tickets are proposed for back porting? Or is the back porting and 
>> vote on inclusion done based on discretion and on a ticket by ticket basis?
> 
> I think the process will follow our approach to back porting fixes onto a 
> release branch:
> 
> 1) When you fix an “important” issue, note that it will need to be back 
> ported onto the correct set of support branches.
> 2) The assigned release manager for the support branch will coordinate 
> community consensus and help with cherry-picking the change as well as 
> ensuring the pipelines are staying green.
> 
>> 
>> With the reduced scope of supported versions, does this also mean we reduce 
>> scope of backward compatibility testing between versions? i.e can we now 
>> change our backward compatibility testing to mimic the same behavior where 
>> we only test compatibility between, HEAD,N,N-1 and N-2?
>> 
> 
> Great question.  I think we continue to follow our SerVer approach to naming 
> versions, as well as following the backwards compatibility guidelines 
> described in [1[.  In short, I don’t think this N-2 changes our 
> requirements—this idea is more about what versions we will patch.
> 
> 
> Anthony
> 
> [1] 
> https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+Compatibility
> 
> 
> 
> 
>> --Udo
>> 
>> On 2/25/20 11:51 AM, Alexander Murmann wrote:
>>> Hi John,
>>> 
>>> I think what you are calling out in 1. and 2. matches what was discussed in
>>> the proposal and thread. Please call out if you disagree on a detail.
>>> 
>>> What's your reasoning behind 3?
>>> I see little reason to ship a patch release (which is significant work) if
>>> there was no important fix.
>>> Likewise I am concerned about waiting to ship a critical fix to our users
>>> or leave them with gaping security vulnerabilities when we have a fix, but
>>> the next patch release train doesn't depart for several weeks.
>>> 
>>> On Tue, Feb 25, 2020 at 11:41 AM John Blum  wrote:
>>> 
 Real quick thought... IMO:
 
 1. There should be patch (maintenance) releases for each major.minor, up to
 N-2 for a set period of time (e.g. 1.5 years), or until N-2 becomes N-3
 where N-3 is no longer supported.
 2. All important changes should be backported.  I say "important" loosely
 since that should be decided by the community, but in general, that means
 Blocker, Critical, Security fixes or other changes that can impact the
 contract, functionality or proper behavior of Apache Geode in whatever
 context.
 3. Patch (maintenance) releases should occur at a regular, "predictable"
 intervals (e.g. every 6 weeks), not on an adhoc basis.
 
 $0.02
 -John
 
 
 On Tue, Feb 25, 2020 at 11:23 AM Alexander Murmann 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

>
> Is it not the case currently? If I am working on a feature modifying class
> X and another developer makes some refactoring changes on class X and
> pushes it to develop, won't I have to resolve the merge commits anyway.


Yes, you will always have to deal with resolving conflicts with other
people's changes. The scenario I was describing was me having to resolve
conflicts between my own 2 changes that modify the same files. If I make a
refactor commit and a fix commit as two separate PRs that are each based on
develop (i.e. they are independent PRs), after I merge the first one to
develop the second one will have merge conflicts. The simplest way to avoid
this is to put the commits in the same PR and use rebase-and-merge.



On Tue, Dec 31, 2019 at 10:46 PM Owen Nichols  wrote:

> It sounds like there is value in being able to deliver un-squashed PRs, and
> we believe the richer commit message history outweighs any potential
> inconvenience to bisect operations (as Aaron pointed out, finer-grained
> commits should generally be to the benefit of bisect operations).
>
> We will leave all 3 merge options enabled in GitHub.
>
> On Tue, Dec 31, 2019 at 7:27 PM Dan Smith  wrote:
>
> > I also tend to do the same workflow Aaron described - make a refactoring
> > change to support a feature followed by the actual change I want to make.
> > It makes the history a lot clearer because refactoring tends to touch a
> lot
> > of files, so it's nice to have those changes clearly marked as a
> > refactoring that should not change behavior.
> >
> > It's possible to do this as to separate PRs, but that drags out the
> process
> > because you have to get the first in merged before you create the second.
> > That encourages bunching changes in a single squashed commit, which makes
> > git blame less useful.
> >
> >
> > -Dan
> >
> > On Tue, Dec 31, 2019, 6:36 PM Nabarun Nag  wrote:
> >
> > > Aaron ,
> > >
> > > Is it not the case currently? If I am working on a feature modifying
> > class
> > > X and another developer makes some refactoring changes on class X and
> > > pushes it to develop, won't I have to resolve the merge commits anyway.
> > >
> > >
> > > Regards
> > > Naba
> > >
> > >
> > > On Tue, Dec 31, 2019 at 6:01 PM Aaron Lindsey  >
> > > wrote:
> > >
> > > > Suppose I’m refactoring the same classes I’m touching for the
> feature.
> > I
> > > > do the refactoring on one branch and submit a PR for that. Then I
> > > implement
> > > > the feature on another branch which is based on develop and does not
> > have
> > > > the refactoring changes since those are not merged yet. I’ll have to
> > > > resolve conflicts on the second PR that I merge.
> > > >
> > > > Having interdependent PRs where one PR branch is based on another PR
> > > > branch is not something I’ve tried. That requires more overhead in
> > having
> > > > to create more PRs and branches. And if you have N interdependent
> PRs,
> > > you
> > > > have to do N-1 merges each time a PR is merged to develop (to update
> > the
> > > > other branches). It could be done, but is it worth the hassle?
> > > >
> > > > One more point about rebase-and-merge is that it seems like this
> would
> > > > make bisecting a failure easier since there would be smaller commits.
> > > >
> > > > Aaron
> > > >
> > > > > On Dec 31, 2019, at 5:41 PM, Owen Nichols 
> > wrote:
> > > > >
> > > > > Can you elaborate on why you would have to deal with conflicts if
> the
> > > > > refactoring work was kept as a separate PR from the fix?
> > > > >
> > > > > As with many git workflows, there’s an easy way and a hard way to
> > > manage
> > > > > interdependent PRs (tl;dr only merge, never rebase!). I wonder if
> > this
> > > > > points out an opportunity for a “tips and tricks” page on the Geode
> > > wiki.
> > > > >
> > > > > On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey <
> > aaronlind...@apache.org
> > > >
> > > > > wrote:
> > > > >
> > > > >> -0.9
> > > > >>
> > > > >> I’m not in favor of the revised proposal that disallows
> > > > rebase-and-merge.
> > > > >> Say I am working on a PR and I have a refactor commit and another
> &

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

Suppose I’m refactoring the same classes I’m touching for the feature. I do the 
refactoring on one branch and submit a PR for that. Then I implement the 
feature on another branch which is based on develop and does not have the 
refactoring changes since those are not merged yet. I’ll have to resolve 
conflicts on the second PR that I merge.

Having interdependent PRs where one PR branch is based on another PR branch is 
not something I’ve tried. That requires more overhead in having to create more 
PRs and branches. And if you have N interdependent PRs, you have to do N-1 
merges each time a PR is merged to develop (to update the other branches). It 
could be done, but is it worth the hassle?

One more point about rebase-and-merge is that it seems like this would make 
bisecting a failure easier since there would be smaller commits.

Aaron

> On Dec 31, 2019, at 5:41 PM, Owen Nichols  wrote:
> 
> Can you elaborate on why you would have to deal with conflicts if the
> refactoring work was kept as a separate PR from the fix?
> 
> As with many git workflows, there’s an easy way and a hard way to manage
> interdependent PRs (tl;dr only merge, never rebase!). I wonder if this
> points out an opportunity for a “tips and tricks” page on the Geode wiki.
> 
> On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey 
> wrote:
> 
>> -0.9
>> 
>> I’m not in favor of the revised proposal that disallows rebase-and-merge.
>> Say I am working on a PR and I have a refactor commit and another commit
>> which implements a new feature. I don’t want those commits to get squashed
>> because that makes it hard to understand the diff. However, if I make those
>> commits as two separate PRs then I am going to have to deal with conflicts.
>> 
>> I’m not sure when we made it a rule that every commit in develop had to
>> compile and pass tests. I know we implemented a rule that all PRs had to
>> pass certain checks, but I never thought that rule implied all commits had
>> to pass those checks.
>> 
>> In general I just don’t see the problem with rebase-and-merge and this
>> feels like an unnecessary restriction, but I will go with it if that’s what
>> everyone wants to do.
>> 
>> Aaron
>> 
>>> On Dec 31, 2019, at 3:09 PM, Owen Nichols  wrote:
>>> 
>>> To recap, this proposal is now revised to remove 2 of the 3 merge options
>>> from GitHub, leaving only Squash and Merge.  PR #4513
>>> <https://github.com/apache/geode/pull/4513> serves as an exhibit of
>> what is
>>> proposed (it is not to be merged unless discussion leads to consensus
>> and a
>>> successful vote).  Please use the dev list (not the PR) for all
>> discussion
>>> (and voting, if we get that far).
>>> 
>>> Squash and merge is already used almost exclusively by the Geode
>> community,
>>> with any exceptions tending to be accidental more often than intentional.
>>> However, some have raised the concern that implementing this restriction
>>> could result in harm or wasted time.  Can someone give an example of a
>> such
>>> a scenario?
>>> 
>>> It seems there is a divide here between junior and senior community
>>> members.  Newer committers appreciate additional guardrails to protect
>> them
>>> from accidentally doing the wrong thing, while those with more experience
>>> want to be able to work unencumbered by restrictions of any kind.
>>> 
>>> Our welcome email to new committers states “We like to work on trust
>> rather
>>> than unnecessary constraints...Being a committer enables you to more
>> easily
>>> make changes without needing to go through the patch submission process”.
>>> I can see this as an argument against this proposal (perhaps even an
>>> argument against any form of branch protection).
>>> 
>>> In the scheme of things, this proposal makes very little difference.
>> There
>>> are still other ways to get non-compiling commits onto develop (e.g.
>>> waiting a long time between running PR checks and merging to develop).
>>> What’s more important is working well together as a community. So,
>> perhaps
>>> what’s best for the community is to encourage working on trust rather
>> than
>>> unnecessary constraints.
>>> 
>>> -Owen
>>> 
>>> On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
>>> 
>>> I'm happy to file multiple PRs when I need to merge multiple commits to
>>> develop.
>>> 
>>> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:
>>> 
>>&

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

-0.9

I’m not in favor of the revised proposal that disallows rebase-and-merge. Say I 
am working on a PR and I have a refactor commit and another commit which 
implements a new feature. I don’t want those commits to get squashed because 
that makes it hard to understand the diff. However, if I make those commits as 
two separate PRs then I am going to have to deal with conflicts.

I’m not sure when we made it a rule that every commit in develop had to compile 
and pass tests. I know we implemented a rule that all PRs had to pass certain 
checks, but I never thought that rule implied all commits had to pass those 
checks.

In general I just don’t see the problem with rebase-and-merge and this feels 
like an unnecessary restriction, but I will go with it if that’s what everyone 
wants to do.

Aaron

> On Dec 31, 2019, at 3:09 PM, Owen Nichols  wrote:
> 
> To recap, this proposal is now revised to remove 2 of the 3 merge options
> from GitHub, leaving only Squash and Merge.  PR #4513
>  serves as an exhibit of what is
> proposed (it is not to be merged unless discussion leads to consensus and a
> successful vote).  Please use the dev list (not the PR) for all discussion
> (and voting, if we get that far).
> 
> Squash and merge is already used almost exclusively by the Geode community,
> with any exceptions tending to be accidental more often than intentional.
> However, some have raised the concern that implementing this restriction
> could result in harm or wasted time.  Can someone give an example of a such
> a scenario?
> 
> It seems there is a divide here between junior and senior community
> members.  Newer committers appreciate additional guardrails to protect them
> from accidentally doing the wrong thing, while those with more experience
> want to be able to work unencumbered by restrictions of any kind.
> 
> Our welcome email to new committers states “We like to work on trust rather
> than unnecessary constraints...Being a committer enables you to more easily
> make changes without needing to go through the patch submission process”.
> I can see this as an argument against this proposal (perhaps even an
> argument against any form of branch protection).
> 
> In the scheme of things, this proposal makes very little difference. There
> are still other ways to get non-compiling commits onto develop (e.g.
> waiting a long time between running PR checks and merging to develop).
> What’s more important is working well together as a community. So, perhaps
> what’s best for the community is to encourage working on trust rather than
> unnecessary constraints.
> 
> -Owen
> 
> On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
> 
> I'm happy to file multiple PRs when I need to merge multiple commits to
> develop.
> 
> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:
> 
> This change to disable all but squash-merge would be really easy to
> revert. How about we try it for a while and see? If people decide it is
> really limiting them, we can change it back. Let’s do it for 1 month and
> see how it goes. Does that sound reasonable?
> 
> Thanks,
> Mark
> 
> On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:
> 
> Given that we adopted <
> 
> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E
>> 
> and still wish to continue <
> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E
>> 
> having branch protection rules to ensure every commit landing in develop
> has passed the required PR checks, it seems like that decision alone
> mandates that we disable all merge mechanisms other than squash-and-merge.
> 
> 
> Allowing merge commits or rebase merges bypasses branch protection for
> 
> all commits other than the final one in the merge or rebase set.  Given
> that we decided <
> https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E
>> 
> that bypassing PR checks should never be allowed, keeping this loophole
> open seems untenable.
> 
> 
> This is not just hypothetical — this loophole is causing real problems.
> 
> We now have commits on develop that don’t compile.  For example:
> 
> git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6
> ./gradlew devBuild
> ...spotlessJava FAILED
> We implemented branch protections to make this impossible, right?
> 
> We can very easily close this loophole by disabling all but the
> 
> Squash button for PRs.  This will not make more work for any
> developer.  If you want to get multiple commits onto develop, simply submit
> each as a separate PR — that is the only way to assure that required PR
> checks have passed for each.
> 
> 
> On the other hand, if we as a Geode community feel strongly that
> 
> bypassing branch protection via merge commits and rebase commits is
> important to allow, why not also allow arbitrary overrides (or get 

Re: [DISCUSS] What should we do with @Ignore tests?

[Aaron Lindsey]

I’m in favor of deleting all except the ones that have JIRA tickets open for 
them, like Bruce said.

Also going forward I’d like to see us not be checking in @Ignored tests — just 
delete them instead. If we need to get it back we have revision history. Just 
my two cents.

Aaron

> On Dec 31, 2019, at 2:53 PM, Bruce Schuchardt  wrote:
> 
> I agree with deleting @Ignored tests except for the few that have JIRA 
> tickets open for them.  There are less than 1/2 dozen of these and we should 
> consider keeping them since we have a way of tracking them.
> 
> On 12/31/19 2:07 PM, Alexander Murmann wrote:
>> Here are a few things that are true for me or I believe are true in general:
>> 
>>- Our test suite is more flaky than we'd like it to be
>>- I don't believe that adding more Unit tests that follow existing
>>patterns buys us that much. I'd rather see something similar to what some
>>folks are doing with Membership right now where we isolate the code and
>>test it more systematically
>>- We have other testing gaps: We have benchmarks , but we are still
>>lacking coverage in that ares; our community is still lacking HA tests. 
>> I'd
>>rather fill those than bring back old DUnit tests that are chosen somewhat
>>at random.
>>- I'd rather be deliberate about what tests we introduce than wholesale
>>bring back a set of tests, since any of these re-introduced tests has a
>>potential to be flaky. Let's make sure our tests carry their weight.
>>- If we delete these tests, we can always go back to a SHA from today
>>and bring them back at a later date
>>- These tests have been ignored since a very long time and we've shipped
>>without them and nobody has missed them enough to bring them back.
>> 
>> Given all the above, my vote is for less noise in our code, which means
>> deleting all ignored tests. If we want to keep them, I'd love to hear a
>> plan of action on how we bring them back. Having a bunch of dead code helps
>> nobody.
>> 
>> On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson  wrote:
>> 
>>> Hi All,
>>> 
>>> As part of what I am doing to fix flaky tests, I periodically come across
>>> tests that are @Ignore’d. I am curious what we would like to do with them
>>> generally speaking. We could fix them, which would seem obvious, but we are
>>> struggling to fix flaky tests as it is.  We could delete them, but those
>>> tests were written for a reason (I hope).  Or we could leave them. This
>>> pollutes searches etc as inactive code requiring upkeep at least.
>>> 
>>> I don’t have an easy answer. Some have suggested deleting them. I tend to
>>> lean that direction, but I thought I would consult the community for a
>>> broader perspective.
>>> 
>>> Thanks,
>>> Mark

Re: [VOTE] Apache Geode 1.11.0.RC4

[Aaron Lindsey]

The release notes section on the wiki just says "tbd". Also, I cannot find 
Mark’s public key in any of the public PGP key servers (such as 
https://keyserver.ubuntu.com), but I did verify that the signatures in the 
source and binary distributions match the key that is listed in the geode KEYS 
file.

Other than those 2 things, everything else looks good. I checked the following:

- Built from source and ran unit tests
- Used GFSH to create a locator and server and do some puts/gets
- Checked version in GFSH
- Built and ran all of the examples
- Verified SHAs and signatures

Aaron

> On Dec 23, 2019, at 2:59 PM, Michael Oleske  wrote:
> 
> +1
> 
> - ran `./gradlew build` on Geode 1.11.0.RC4 on Mac and Windows
> - built Geode Native 1.11.0.RC4 against Geode 1.11.0.RC4 on Mac and Windows
> - ran `ctest -L STABLE -C Debug` on cppcache/integration-test on Mac and
> Windows
> - ran `ctest` on cppcache/integration/test on Mac and Windows (Did not test
> IPV6 support as I don't generally use that)
> - ran `ctest -L STABLE -C Debug` on clicache/integration-test on Windows
> - ran XUnit tests in Rider on clicache/integrations-test2 on Windows
> - installed and built Geode Native 1.11.0.RC4 examples on Windows
> 
> -michael
> 
> 
> On Fri, Dec 20, 2019 at 2:49 PM Mark Hanson  wrote:
> 
>> Given it is the holidays, perhaps more time is in order. I am bumping the
>> voting deadline to Friday, December 27th, 2019.
>> 
>> Thanks,
>> Mark
>> 
>> 
>> 
>>> On Dec 20, 2019, at 2:46 PM, Mark Hanson  wrote:
>>> 
>>> Subject: [VOTE] Apache Geode 1.11.0.RC4
>>> Hello Geode Dev Community,
>>> 
>>> This is a release candidate for Apache Geode version 1.11.0.RC4.
>>> Thanks to all the community members for their contributions to this
>> release!
>>> 
>>> Please do a review and give your feedback, including the checks you
>> performed.
>>> 
>>> Voting deadline:
>>> 3PM PST Friday, December 27 2019.
>>> 
>>> Please note that we are voting upon the source tag:
>>> rel/v1.11.0.RC4
>>> 
>>> Release notes:
>>> 
>> https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.11.0
>>> 
>>> Source and binary distributions:
>>> https://dist.apache.org/repos/dist/dev/geode/1.11.0.RC4/
>>> 
>>> Maven staging repo:
>>> https://repository.apache.org/content/repositories/orgapachegeode-1064
>>> 
>>> GitHub:
>>> https://github.com/apache/geode/tree/rel/v1.11.0.RC4
>>> https://github.com/apache/geode-examples/tree/rel/v1.11.0.RC4
>>> https://github.com/apache/geode-native/tree/rel/v1.11.0.RC4
>>> 
>>> Pipelines:
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-11-0-main
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-11-0-rc
>>> 
>>> Geode's KEYS file containing PGP keys we use to sign the release:
>>> https://github.com/apache/geode/blob/develop/KEYS
>>> 
>>> Command to run geode-examples:
>>> ./gradlew -PgeodeReleaseUrl=
>> https://dist.apache.org/repos/dist/dev/geode/1.11.0.RC4
>> -PgeodeRepositoryUrl=
>> https://repository.apache.org/content/repositories/orgapachegeode-1064
>> build runAll
>>> 
>>> Regards
>>> Mark Hanson
>>> 
>> 
>> 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

Just to be clear, this proposal wouldn't require anyone to squash their commits 
before merging a PR. All it’s saying is that if you do have multiple commits in 
your PR, you would have to rebase those commits onto develop before merging to 
ensure a linear commit history.

- Aaron

> On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> 
>  

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

+1 to (1) and (3)

I’m on board with (1). I’m hesitant about agreeing to (2) because it seems 
harder to “accidentally” do a merge commit via the command line, and I don’t 
want to add unnecessary restrictions. (3) has needed to be done for some time 
now, so I’m happy to see a proposal to change that.

In which case would an explicit merge commit be preferred/required to a “rebase 
and merge”? In my experience working on Geode, I’ve never needed to create an 
explicit merge commit. I have, however, seen people do it by accident. As a 
reminder, “rebase and merge" sill allows you to keep all of the individual 
commits from your PR.

- Aaron

> On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> 
> +1
> 
> On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:
> 
>> Hi Bruce, this proposal will not waste a single second of your time.  It
>> just prevents people from accidentally pressing a button that we have
>> already agreed should never be pressed, but because we never configured our
>> GitHub to match our stated policy, is currently the default.
>> 
>> However, it will save a lot of time and frustration for anyone needing to
>> bisect failures, revert, or cherry-pick changes, which has merit even if
>> you personally never do any of those three things.
>> 
>> Please start a separate thread if you would like to revisit the community
>> decision to require passing PR checks.
>> 
>>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> I agree with Jake.  I would go further by saying that I see very little
>> merit in this proposal.  I think we're getting more and more bureaucratic
>> in our process and that it stifles productivity.  I was recently forced to
>> spend three days fixing tests in which I had changed an import statement
>> before they would pass stress testing.  I'm glad the tests now pass
>> reliably but I was very frustrated by the process.
>>> 
>>> On 12/19/19 4:49 PM, Jacob Barrett wrote:
 I’m in agreement with Dan. Changes to the infrastructure to flat out
>> prevent things that should be self policing is annoying. This PR review
>> lock we have had already cost us valuable time waiting for PR pipelines to
>> pass that have no relevance to the commit, like CI work: I’d hat to see yet
>> another process enforced that Kees us from getting work done when necessary.
 
 -Jake
 
 
> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> 
> -1 to (1) and (2).
> 
> I think merge commits are appropriate in certain circumstances, so I
>> don't
> think we should make a blanket restriction. In fact I think our release
> process involves some merges.
> 
> I think setting standards on what is reasonable to be an individual
>> commit
> will do a lot more to clean up our history than blocking merges. We'd
> rather not see commits like "Spotless Apply" in the history, but if
> reasonably separate and well written commits come in as part of a
>> merge, I
> think that's fine.
> 
> -Dan
> 
>> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
>> wrote:
>> 
>> +1
>> 
>>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
>> wrote:
>>> 
>>> I’d like to advance this topic from an informal request/discussion
>> to a
>>> discussion of a concrete proposal.
>>> 
>>> To recap, it sounds like there is general agreement that commit
>> history
>> on
>>> develop should be linear (no merge commits), and that the biggest
>> obstacle
>>> to this is that the PR merge button in GitHub creates a merge commit
>> by
>>> default.
>>> 
>>> I propose the following changes:
>>> 1) Change GitHub settings to remove the ability to create a merge
>> commit.
>>> This will make Squash & Merge the default.
>>> 
>>> 2) Change GitHub settings to require linear history on develop.  This
>>> prevents merge commits via command-line (not recommended, but wiki
>> still
>>> has instructions for merging PRs this way).
>>> 
>>> 3) Update the PR template to change the text "Is your initial
>> contribution
>>> a single, squashed commit” to “Is your initial contribution squashed
>> for
>>> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus
>> a
>>> ‘fix’ commit)"
>>> 
>>> For clarity, I am proposing no-change in the following areas:
>>> i) Leave Rebase & Merge as an option for PRs that have been
>> structured to
>>> benefit from it (this can make it easier in a bisect to see whether
>> the
>>> refactoring or the “fix” broke something).
>>> ii) Leave existing wording in the wiki as-is [stating that squashing
>> is
>>> preferred].
>>> 
>>> 
>>> Please comment via this email thread.
>>> -Owen
>>> 
>>> 
>>> 
 On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:
 
 I think it's already resolved Udo ;)
 
 Here's the problem, if I fixup a dunit test by removing all uses 

Re: Review for #4387

[Aaron Lindsey]

I added a review, but mostly from a code cleanliness and testing perspective. I 
hope someone with more knowledge of the use case and WAN can provide their 
feedback.

- Aaron

> On Dec 16, 2019, at 1:14 AM, Mario Ivanac  wrote:
> 
> Hi all,
> 
> please could someone review https://issues.apache.org/jira/browse/GEODE-7458
> [GEODE-7458] Adding additional option in gfsh command "start gateway sender" 
> to control clearing of existing queues - ASF 
> JIRA
> This Jira has been LDAP enabled, if you are an ASF Committer, please use your 
> LDAP Credentials to login. Any problems email us...@infra.apache.org
> issues.apache.org
> PR https://github.com/apache/geode/pull/4387
> 
> 
> Thanks,
> Mario

Re: [DISCUSS] Replacing singleton PoolManager

[Aaron Lindsey]

+1

Aaron

> On Dec 6, 2019, at 10:49 AM, Jacob Barrett  wrote:
> 
> 
> 
>> On Dec 6, 2019, at 9:40 AM, Dan Smith  wrote:
>> 
>> Regarding changing PoolManager to
>> an interface, I guess originally I wasn't thinking we would still be
>> backwards compatible if we did that. But as I think about it I think that
>> might be ok. One slight issue with that approach is that we have to come up
>> with new names for the methods - we can't have both an instance and a
>> static method with the same name and args. Maybe still worth it
> 
> Doh! I didn’t think about that. It sort of defeats the purpose of reusing the 
> class. So going with a whole new class probably makes more sense to remove 
> confusion.
> 
> -Jake
> 

Re: [DISCUSS] is overriding a PR check ever justified?

[Aaron Lindsey]

One case when it might be acceptable to overrule a PR check is reverting a
commit. Before the branch protection was enabled, a committer could revert
a commit without a PR. Now that PRs are mandatory, we have to wait for the
checks to run in order to revert a commit. Usually we are reverting a
commit because it's causing problems, so I think overruling the PR checks
may be acceptable in that case.

- Aaron


On Wed, Oct 30, 2019 at 11:11 AM Owen Nichols  wrote:

> Our new branch-protection rules can sometimes lead to unexpected obstacles
> when infrastructure issues impede the intended process.  Should we discuss
> such cases as they come up, and should overruling the result of a PR check
> ever be an option on the table?
>
> -Owen

Re: [DISCUSS] Tweak to branch protection rules

[Aaron Lindsey]

+1

- Aaron


On Wed, Oct 30, 2019 at 8:02 AM Ju@N  wrote:

> Perfect Naba, thanks for answering this.
> My vote is +1 then.
>
> On Wed, Oct 30, 2019 at 2:37 PM Nabarun Nag  wrote:
>
> > The check box Dan is mentioning will just not invalidate any approved
> > review if the code is changed.
> > If a change is requested, the button will remain disabled.
> >
> > Regards
> > Naba
> >
> >
> > On Wed, Oct 30, 2019 at 6:27 AM Joris Melchior 
> > wrote:
> >
> > > +1
> > >
> > > On Wed, Oct 30, 2019 at 5:27 AM Ju@N  wrote:
> > >
> > > > Question: this only applies for *approvals*, not for *refusals*,
> > right?;
> > > I
> > > > mean, the *merge pull request* button will remain blocked if there
> were
> > > > some changes requested by reviewers and the author of the PR adds new
> > > > commits (either addressing those requested changes or not)?.
> > > > If the answer to the above is "yes", then +1.
> > > >
> > > > On Wed, Oct 30, 2019 at 1:44 AM Nabarun Nag  wrote:
> > > >
> > > > > +1
> > > > >
> > > > > On Tue, Oct 29, 2019 at 6:21 PM Darrel Schneider <
> > > dschnei...@pivotal.io>
> > > > > wrote:
> > > > >
> > > > > > +1
> > > > > >
> > > > > > On Tue, Oct 29, 2019 at 6:08 PM Owen Nichols <
> onich...@pivotal.io>
> > > > > wrote:
> > > > > >
> > > > > > > +1 …this has already bitten me a few times
> > > > > > >
> > > > > > > > On Oct 29, 2019, at 6:01 PM, Dan Smith 
> > > wrote:
> > > > > > > >
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > It seems we've configured our branch protection rules such
> that
> > > > > > pushing a
> > > > > > > > change to a PR that has been approved invalidates the
> previous
> > > > > > approval.
> > > > > > > >
> > > > > > > > I think we should turn this off - it looks like it's an
> > optional
> > > > > > feature.
> > > > > > > > We should trust people to rerequest reviews if needed. Right
> > now
> > > > this
> > > > > > is
> > > > > > > > adding busywork for people to reapprove minor changes (Fixing
> > > merge
> > > > > > > > conflicts, spotless, etc.)
> > > > > > > >
> > > > > > > > If you all agree I'll ask infra to uncheck "Dismiss stale
> pull
> > > > > request
> > > > > > > > approvals when new commits are pushed." in our branch
> > protection
> > > > > rules.
> > > > > > > >
> > > > > > > > -Dan
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Ju@N
> > > >
> > >
> > >
> > > --
> > > *Joris Melchior *
> > > CF Engineering
> > > Pivotal Toronto
> > > 416 877 5427
> > >
> > > “Programs must be written for people to read, and only incidentally for
> > > machines to execute.” – *Hal Abelson*
> > > 
> > >
> >
>
>
> --
> Ju@N
>

Re: [PSA] Github branch protection

[Aaron Lindsey]

@Owen I'm fine with following the "requesting changes is the same as -1"
rule, but I don't think there is consensus from the whole community on this
yet. I was previously told that contributors should make every effort to
address the requested changes, but unless a committer actually comments
"-1" on the PR, the author retains the ability to reject the requested
changes. This probably deserves a separate discussion at some point.

My original concern has been addressed since Helena pointed out that a
review can be dismissed. I agree this power should probably only be used if
the reviewer cannot be contacted.

- Aaron


On Thu, Oct 24, 2019 at 5:04 PM Owen Nichols  wrote:

> >
> > @Owen It's unclear to me whether "requesting changes" is the same thing
> as
> > a -1 vote. I had previously discussed this with some other committers who
> > were under the impression that they were not the same thing.
> >
>
> If that’s not a -1, what is?
>
> Many apache projects started out long ago when patches were submitted by
> email and voted over email.  We have adopted modern GitHub tools to
> streamline this process, but the concept is still the same: reviewers can
> give a +1 (green check) review, or a -1 (with explanation of what it would
> take to get their approval).
>
> One time when I was a relatively new committer, I merged a PR while
> someone still had changes requested on it, and I was admonished: “it sets a
> bad precedent for merges to occur on PRs that have unresolved reviews. <
> https://github.com/apache/geode/pull/3597#issuecomment-493624748>”.
> GitHub will also permanently record that the PR was merged in spite of an
> outstanding request for changes, leading to life-long shame.
>
> If someone has requested changes on your PR, you should make every effort
> to understand and address their concern.  If you have pushed additional
> changes since their review, remind them by clicking the re-request review
> button next to their name.
>
> On the flip side, if you request changes on a PR, please clearly explain
> what it would take to gain your approval, and please make an effort to
> re-review in a timely fashion after changes are made.  More guideline for
> good PR review practices can be found here <
> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c
> >.
>
> The option to dismiss a review should be a last resort, for example if the
> reviewer went on vacation and cannot be contacted.
>
> -Owen
>
> > - Aaron
> >
> >
> > On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag  wrote:
> >
> >> @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
> >> cache gets timers" which can be merged? I can get some more idea when I
> can
> >> see whats going on.
> >>
> >> Regards
> >> Naba
> >>
> >> @Kirk : let me run some experiments.
> >>
> >> Regards
> >> Naba
> >>
> >>
> >> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales  wrote:
> >>
> >>> To Kirk's point, there is actually a way to dismiss requests for
> review.
> >>> Info here:
> >>>
> >>>
> >>
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> >>> There's instructions in there for how to dismiss a request for changes.
> >> Not
> >>> everyone can do that, so if you aren't a contributor yet you'll
> probably
> >>> have to hit up a current contributor to get any requests for changes
> >>> dismissed.
> >>>
> >>> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund  wrote:
> >>>
>  One side effect is that any single request for changes will now
> >>> completely
>  block merging the PR. I'm not certain this was intentional? One rogue
>  developer could block the merging of any or every PR. I'm not sure one
>  person should have that much power...
> 
>  On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag  wrote:
> 
> > Hi, Geode dev Community,
> >
> > This is an announcement that the GitHub branch protection rules are
> >>> *now
> > active* on develop branch for Apache Geode.
> >
> > The following rules are currently active :
> > - Require pull request reviews before merging - at least 1
> > - Require status checks to pass before merging
> > [Only for
> >- concourse-ci/Build
> >   - concourse-ci/UnitTestOpenJDK11
> >   - concourse-ci/UnitTestOpenJDK8
> >   - concourse-ci/StressNewTestOpenJDK11]
> >
> > After we stabilize the remaining test suites, we can add them to
> >> these
>  rule
> > sets.
> >
> > Also reminding the community to use squash merge while closing pull
> > requests.
> >
> > Regards
> > Naba
> >
> 
> >>>
> >>
>
>

Re: [PSA] Github branch protection

[Aaron Lindsey]

@Naba That's the one. It was approved shortly after I sent that message
though. It should be reproducible by requesting changes on a PR with no
other reviews.

@Owen It's unclear to me whether "requesting changes" is the same thing as
a -1 vote. I had previously discussed this with some other committers who
were under the impression that they were not the same thing.

@Helena Thanks! I didn't know that was possible.

- Aaron


On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag  wrote:

> @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
> cache gets timers" which can be merged? I can get some more idea when I can
> see whats going on.
>
> Regards
> Naba
>
> @Kirk : let me run some experiments.
>
> Regards
> Naba
>
>
> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales  wrote:
>
> > To Kirk's point, there is actually a way to dismiss requests for review.
> > Info here:
> >
> >
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> > There's instructions in there for how to dismiss a request for changes.
> Not
> > everyone can do that, so if you aren't a contributor yet you'll probably
> > have to hit up a current contributor to get any requests for changes
> > dismissed.
> >
> > On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund  wrote:
> >
> > > One side effect is that any single request for changes will now
> > completely
> > > block merging the PR. I'm not certain this was intentional? One rogue
> > > developer could block the merging of any or every PR. I'm not sure one
> > > person should have that much power...
> > >
> > > On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag  wrote:
> > >
> > > > Hi, Geode dev Community,
> > > >
> > > > This is an announcement that the GitHub branch protection rules are
> > *now
> > > > active* on develop branch for Apache Geode.
> > > >
> > > > The following rules are currently active :
> > > > - Require pull request reviews before merging - at least 1
> > > > - Require status checks to pass before merging
> > > >  [Only for
> > > > - concourse-ci/Build
> > > >- concourse-ci/UnitTestOpenJDK11
> > > >- concourse-ci/UnitTestOpenJDK8
> > > >- concourse-ci/StressNewTestOpenJDK11]
> > > >
> > > > After we stabilize the remaining test suites, we can add them to
> these
> > > rule
> > > > sets.
> > > >
> > > > Also reminding the community to use squash merge while closing pull
> > > > requests.
> > > >
> > > > Regards
> > > > Naba
> > > >
> > >
> >
>

Re: [PSA] Github branch protection

[Aaron Lindsey]

Thanks, Naba. Are we now blocking merges for PRs that have
changes requested?

I have a PR right now where instead of the merge button it has the message,
"Merging can be performed automatically once the requested changes are
addressed." It also happens to not have any approvals, so it's possible
that the merge is actually blocked because there are no approvals. In this
case I find the message misleading.

- Aaron


On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag  wrote:

> Hi, Geode dev Community,
>
> This is an announcement that the GitHub branch protection rules are *now
> active* on develop branch for Apache Geode.
>
> The following rules are currently active :
> - Require pull request reviews before merging - at least 1
> - Require status checks to pass before merging
>  [Only for
> - concourse-ci/Build
>- concourse-ci/UnitTestOpenJDK11
>- concourse-ci/UnitTestOpenJDK8
>- concourse-ci/StressNewTestOpenJDK11]
>
> After we stabilize the remaining test suites, we can add them to these rule
> sets.
>
> Also reminding the community to use squash merge while closing pull
> requests.
>
> Regards
> Naba
>

Reviewers for GEODE-7184: Fix failing Windows acceptance tests

[Aaron Lindsey]

Could one or two others please review and/or merge this PR? It fixes
failures in Windows acceptance test jobs in the develop CI pipeline.

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

- Aaron

Re: [DISCUSS] Blocking merge button in PR

[Aaron Lindsey]

+1

- Aaron


On Mon, Oct 21, 2019 at 11:53 AM Udo Kohlmeyer  wrote:

> BIG +1 (Yes, I'm changing my -1)
>
> @Naba, thank you for the offline chat. It seems that the proposal of
> Github enforcing our good practices is a great option.
>
> 2 merged PR's without a full green pipeline, since 18 Oct 2019, shocking
> and disgusting!!
>
> I would just like to reiterate to ALL committers, you have a
> responsibility towards the project to commit/merge only the best code
> possible. If things break, fix them. Don't merge and hope that it goes
> away and someone else fixes it.
>
> I really don't want to support a mechanism where the project has become
> so prescriptive and restrictive, all because we have a few committers
> who will not follow the established and agreed processes. BUT, once
> again, the masses are impacted due to a few.
>
> Thank you Naba for following this through.
>
> --Udo
>
> On 10/21/19 11:05 AM, Nabarun Nag wrote:
> > @Karen
> >
> > Thank you so much for the feedback. I understand that committers must
> trust
> > each other and I agree entirely with that. This proposal is just
> preventing
> > us from making mistakes. Its just guard rails. As you can see in the
> email
> > chain that this mistake was made multiple times and this has let to a lot
> > of engineers give up their time and detecting and fixing this issue. We
> > still trust our committers, we are just enabling some features to avoid
> > time being wasted on avoidable mistakes and use that time in
> > improving Geode. I hope I can persuade you to change your opinion.
> >
> > @Owen
> > Thank you for accepting the baby step proposal. Yes, let's see in some
> time
> > if this is successful. We can always undo this if needed.
> >
> > @Rest
> > - Blaming people etc. will be very detrimental to the community, I do not
> > propose that.
> > - This proposal was not just my idea but from feedback from lot of
> > developers who contribute to Geode on a frequent basis.
> > - It is very* unfair *to the engineers to go and investigate and find out
> > what caused the failure and then send emails etc, their time can be used
> in
> > doing something more valuable.
> > - This is not something new, we have had this issue for over 3 years now,
> > and maintaining this as the status quo is not healthy. Let us try this
> > solution, the committers, and developers who contribute on a regular
> basis
> > will immediately see if it works or does not work and can revert it if
> this
> > does not.
> > - There is a problem and this is an attempt at the solution. If it does
> not
> > work, we can revert it. For the sake of all the developers who are in
> pain
> > and helping them spend the time on more productive tasks, let us just
> give
> > this proposal a chance. If there are any issues we can revert it.
> >
> >
> > *Reiterating the proposal:*
> > Github branch protection rule for :
> > - at least one review
> > - Passing build, unit and stress test.
> >
> >
> > In our opinion, no committer would want to check-in code with failing any
> > of the above.
> >
> > Regards
> > Nabarun
> >
> > On Mon, Oct 21, 2019 at 10:28 AM Alexander Murmann 
> > wrote:
> >
> >> Udo, I think you are making a great point! I am fully bought into
> trusting
> >> our committers to know how many reviews are needed for their PRs. We
> should
> >> be able to have the same trust into knowing when it's OK to merge. If a
> >> committer wants to they can after all, always merge and push without a
> PR.
> >> That's because we trust them.
> >>
> >> If we are seeing that our committers merge without sufficient review
> (via
> >> human or automated means), is this an education problem we can solve?
> >>
> >> On Mon, Oct 21, 2019 at 10:18 AM Udo Kohlmeyer  wrote:
> >>
> >>> I must agree with @Karen here..
> >>>
> >>> All committers are trusted to do the right thing. One of those things
> is
> >>> to commit (or not commit) PR's.
> >>>
> >>> Now we are discussing disabling the button when tests are failing. Why
> >>> stop there? Why not, that the submitter of the said commit does not get
> >>> to merge their own PR?
> >>>
> >>> Now, that of course is taking it to the extreme, that we don't want (at
> >>> least I don't want to be THAT over prescriptive).. So why do we want to
> >>> now limit when I can merge? It remains the committers responsibility to
> >>> merge commits into the project, that are of the expected quality. IF it
> >>> so happens that one, by accident, has merged a PR before it was green,
> >>> revert it. All committers have the power to do so.
> >>>
> >>> So from my perspective, a -1 on disabling the Merge button, just
> because
> >>> someone was not careful in merging and without following our protocol
> of
> >>> waiting for an "All green".
> >>>
> >>> --Udo
> >>>
> >>> On 10/21/19 10:11 AM, Ernest Burghardt wrote:
>  +1 to enacting this immediately... just this weekend a PR was merged
> >> with
>  failures on all of the following:
>  * 

Re: Changes to recorded execution times for function stats

[Aaron Lindsey]

I haven't heard any objections to this, so we plan to move forward with
these changes.

- Aaron


On Tue, Oct 15, 2019 at 4:29 PM Aaron Lindsey  wrote:

> Hi Geode devs,
>
> This PR <https://github.com/apache/geode/pull/4135> makes small changes
> to the execution times recorded by function stats. I wanted to run this by
> the dev list to make sure this will not cause problems for users who rely
> on these stats:
>
>- In ServerFunctionExecutor and ServerRegionFunctionExecutor, we are
>now including validateExecution in the total execution time.
>- In MemberFunctionStreamingMessage, we are now including the code
>leading up to creating the function context in the total execution time.
>
> This would potentially make times reported by function stats appear longer
> than prior to this change. Please reply or comment on the PR if anyone sees
> a problem with this.
>
> Thanks,
> Aaron
>

Changes to recorded execution times for function stats

[Aaron Lindsey]

Hi Geode devs,

This PR  makes small changes to
the execution times recorded by function stats. I wanted to run this by the
dev list to make sure this will not cause problems for users who rely on
these stats:

   - In ServerFunctionExecutor and ServerRegionFunctionExecutor, we are now
   including validateExecution in the total execution time.
   - In MemberFunctionStreamingMessage, we are now including the code
   leading up to creating the function context in the total execution time.

This would potentially make times reported by function stats appear longer
than prior to this change. Please reply or comment on the PR if anyone sees
a problem with this.

Thanks,
Aaron

Re: [VOTE] Apache Geode 1.10.0.RC2

[Aaron Lindsey]

+1

   - Built from source and ran unit tests
   - Used GFSH to create a locator and server and do some puts/gets
   - Checked version in GFSH
   - Built and ran all of the examples
   - Verified SHAs and signatures

The luceneSpatial project in geode-examples fails for me when running...
>

To address my earlier comment, I was able to run all of the examples
successfully after I did a full gradle clean and build in the
geode-examples repo. I guess the error I saw was due to some leftover
artifacts from a previous build.

- Aaron


On Tue, Sep 24, 2019 at 10:30 AM Dan Smith  wrote:

> +1
>
> Ran geode-release-check (checks signatures, source build, simple gfsh test)
>
> -Dan
>
> On Mon, Sep 23, 2019 at 3:20 PM Jacob Barrett  wrote:
>
> > +1
> >
> > Still think it is ok to ship but found another annoying issue. When
> > running Geode Native C++ integration tests (new framework) an indented
> > disabled test will execute and fail. The test was not properly disabled.
> On
> > the develop branch this test is fixed and is no longer disabled. If there
> > is another RC we could easily port the fix or properly disable the test.
> >
> > -Jake
> >
> > > On Sep 23, 2019, at 2:36 PM, Jacob Barrett 
> wrote:
> > >
> > > +1
> > >
> > > Only tested geode-native on MacOS. Unit and integration tests based.
> > >
> > > One issues worth mentioning, while we don’t explicitly state support
> for
> > specific platforms, MacOs with the latest Xcode will have some troubles
> > getting off the ground. The updated Clang compiler in Xcode is less
> > forgiving and has some new warnings/errors. This same issue will likely
> > exist in other platforms where newer versions of Clang are in use.
> Adding a
> > few warning exclusions on your CMake configuration command will get you
> > past the errors.
> > >
> > > cmake ... -DCMAKE_CXX_FLAGS="-Wno-defaulted-function-deleted
> > -Wno-c++2a-compat"
> > >
> > > -Jake
> > >
> > >
> > >
> >
> >
>

Re: [VOTE] Apache Geode 1.10.0.RC2

[Aaron Lindsey]

The luceneSpatial project in geode-examples fails for me when running:

./gradlew -PgeodeReleaseUrl=
https://dist.apache.org/repos/dist/dev/geode/1.10.0.RC2
-PgeodeRepositoryUrl=
https://repository.apache.org/content/repositories/orgapachegeode-1059
:luceneSpatial:build :luceneSpatial:runAll

In the server logs, I see the following fatal message:

[fatal 2019/09/23 12:34:38.673 PDT :41000 shared unordered uid=1
port=57924> tid=0x24] Error deserializing message
org.apache.geode.SerializationException: Could not create an instance of
org.apache.geode.internal.cache.MemberFunctionStreamingMessage .
at
org.apache.geode.internal.InternalDataSerializer.invokeFromData(InternalDataSerializer.java:2389)
at
org.apache.geode.internal.DSFIDFactory.create(DSFIDFactory.java:997)
at
org.apache.geode.internal.InternalDataSerializer.readDSFID(InternalDataSerializer.java:2516)
at
org.apache.geode.internal.InternalDataSerializer.readDSFID(InternalDataSerializer.java:2528)
at
org.apache.geode.internal.tcp.Connection.readMessage(Connection.java:3111)
at
org.apache.geode.internal.tcp.Connection.processInputBuffer(Connection.java:2920)
at
org.apache.geode.internal.tcp.Connection.readMessages(Connection.java:1745)
at
org.apache.geode.internal.tcp.Connection.run(Connection.java:1577)
at java.lang.Thread.run(Thread.java:748)
Caused by: java.io.InvalidClassException:
org.apache.geode.cache.configuration.CacheElement; local class
incompatible: stream classdesc serialVersionUID = -320845719772712775,
local class serialVersionUID = 3127544615832500564
at
java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:699)
at
java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1885)
at
java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1751)
at
java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1885)
at
java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1751)
at
java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2042)
at
java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1573)
at
java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2287)
at
java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2211)
at
java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2069)
at
java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1573)
at java.io.ObjectInputStream.readObject(ObjectInputStream.java:431)
at
org.apache.geode.internal.InternalDataSerializer.readSerializable(InternalDataSerializer.java:2846)
at
org.apache.geode.internal.InternalDataSerializer.basicReadObject(InternalDataSerializer.java:2790)
at
org.apache.geode.DataSerializer.readObject(DataSerializer.java:2968)
at
org.apache.geode.internal.cache.MemberFunctionStreamingMessage.fromData(MemberFunctionStreamingMessage.java:288)
at
org.apache.geode.internal.InternalDataSerializer.invokeFromData(InternalDataSerializer.java:2372)
... 8 more

Does this happen for anyone else?

- Aaron


On Fri, Sep 20, 2019 at 1:06 PM Dave Barnes  wrote:

> +1
> I downloaded the 1.10.0 RC2 Apache Geode release candidate, and also the
> Geode Native Client source bundle.
> Successfully built the user guides and API docs in both repos and
> spot-checked the contents for correct version strings and paths.
>
> On Fri, Sep 20, 2019 at 9:26 AM Dick Cavender 
> wrote:
>
> > Hello Geode dev community,
> >
> > This is a release candidate for Apache Geode, version 1.10.0.RC2.
> > Thanks to all the community members for their contributions to this
> > release!
> >
> > Please do a review and give your feedback including the checks performed.
> > The voting deadline is *3PM PST Wed, September 25 2019*.
> >
> > This candidate has passed RC Qualification checks (NEW) which can be
> found
> > at:
> >
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-10-0-rc
> >
> > Release notes can be found at:
> >
> >
> https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.10.0
> >
> > Please note that we are voting upon the source tags: *rel/v1.10.0.RC2*
> >
> > Apache Geode:
> > https://github.com/apache/geode/tree/rel/v1.10.0.RC2
> > Apache Geode examples:
> > https://github.com/apache/geode-examples/tree/rel/v1.10.0.RC2
> > Apache Geode native:
> > https://github.com/apache/geode-native/tree/rel/v1.10.0.RC2
> >
> > Source and binary files:
> > https://dist.apache.org/repos/dist/dev/geode/1.10.0.RC2/
> >
> > Maven staging repo:
> > https://repository.apache.org/content/repositories/orgapachegeode-1059
> >
> > Geode's KEYS file containing PGP keys we use to sign the release:
> > https://github.com/apache/geode/blob/develop/KEYS
> >
> > PS: Command to run geode-examples: ./gradlew -PgeodeReleaseUrl=
> > 

Re: Question about excluding serialized classes

[Aaron Lindsey]

Hi Jake,

I think this thread has moved a bit off-topic, but I appreciate you
pointing out that scenario. We'll have to consider that when re-working
this PR.

- Aaron


On Tue, Sep 17, 2019 at 9:42 AM Jacob Barrett  wrote:

>
> > On Sep 17, 2019, at 9:36 AM, Aaron Lindsey  wrote:
> >
> >>
> >> Not all functions are registered. I can invoke a function with
> >> Execution.execute(Function) from the client, the Function is serialized
> and
> >> executed on the server, the class need only exist on the server for
> >> deserialization. Must a function be registered now to get metrics?
> >>
> >
> > For this meter we're only interested in timing registered functions.
>
> Can you explain why ya’ll aren’t interested in the more common use case?
>
> Also, are you aware this will likely result in a mismatch in meters for
> the same function between the client and the server. If the client has the
> function registered and the function is executed by name then the internals
> fetch the registered function object and effectively executes as
> Execution.execute(Function). This means that the client side will update
> meters for this function but when deserialized on the server to execute it
> would not update meters on the server. Only the case where the client
> doesn't have the Function registered and executes by name but the server
> does have it registered and looks up the instance would you end up with
> metrics updated on both sides.
>
> -Jake
>
>

Re: Question about excluding serialized classes

[Aaron Lindsey]

>
> Not all functions are registered. I can invoke a function with
> Execution.execute(Function) from the client, the Function is serialized and
> executed on the server, the class need only exist on the server for
> deserialization. Must a function be registered now to get metrics?
>

For this meter we're only interested in timing registered functions.
- Aaron


On Tue, Sep 17, 2019 at 9:18 AM Dale Emery  wrote:

>
> > On Sep 16, 2019, at 7:54 PM, Jacob Barrett  wrote:
> >
> > The current implementation has statistics without decoration.
>
> For our meters, we want to make a distinction that the current stats
> implementation does not: We want to measure only non-internal functions. It
> isn’t clear (yet) how we can inform the function stats constructor so that
> it knows whether to create the meters.
>
> Cheers,
> Dale
>
> —
> Dale Emery
> dem...@pivotal.io
>
>

Re: Question about excluding serialized classes

[Aaron Lindsey]

Jake — We're not constructing a new object each time a function is invoked
because the "decorated" functions are only created when a function is
registered, but we'll keep your concern in mind.

Thanks for all of the feedback from everyone. I think we have the
information we need to move forward. In summary, we'll have to change our
implementation such that the methods in FunctionService still return the
same objects they did before our changes.

- Aaron


On Mon, Sep 16, 2019 at 7:54 PM Jacob Barrett  wrote:

> Sorry I am entering this late in the game but why do we need to decorate
> the function at all for metrics. The current implementation has statistics
> without decoration. All the concerns raised in this thread concern me as
> well as the cost of constructing yet another object every time a function
> is invoked by serialized function, Execution.execute(Function). Each
> allocation puts more pressure on the GC, decreases our throughput and
> increases our latency.
>
> -Jake
>
>
> > On Sep 11, 2019, at 2:29 PM, Dale Emery  wrote:
> >
> > The stats use the ID of the function, and each TimingFunction reports
> the same ID as the function it wraps. So I think the stats would look like
> they always did.
> >
> > Dale
> >
> > —
> > Dale Emery
> > dem...@pivotal.io
> >
> >
> >
> >> On Sep 11, 2019, at 12:14 PM, Anthony Baker  wrote:
> >>
> >> I think the Decorator approach you outlined could have other impacts as
> well.  Would I still be able to see specific function executions in
> statistics or would they all become “TImingFunction”?
> >>
> >> Anthony
> >>
> >>
> >>> On Sep 11, 2019, at 12:00 PM, Aaron Lindsey 
> wrote:
> >>>
> >>> Thanks for your response, Dan.
> >>>
> >>> The second scenario you mentioned (i.e. users calling
> >>> FunctionService.getFunction(String)) worries me because our PR changes
> the
> >>> FunctionService so they would now get back an instance of the decorator
> >>> function instead of the specific instance they registered by calling
> >>> FunctionService.registerFunction(Function). Therefore, any explicit
> casts
> >>> to a Function implementation like (MyFunction)
> >>> FunctionService.getFunction("MyFunction") would fail. Does that mean
> this
> >>> be a breaking change? The FunctionService class does not specify that
> >>> getFunction must return the same type function as the one passed to
> >>> registerFunction, but I could see how users might be relying on that
> >>> behavior since there is no other way to get a specific function type
> out of
> >>> the FunctionService without doing a cast.
> >>>
> >>> - Aaron
> >>>
> >>>
> >>> On Wed, Sep 11, 2019 at 10:52 AM Dan Smith  wrote:
> >>>
> >>>> Functions are serialized when you call Execution.execute(Function)
> instead
> >>>> of Execution.execute(String). Invoking execute on a function object
> >>>> serializes the function and executes it on the remote side. Functions
> >>>> executed this way don't have be registered.
> >>>>
> >>>> Users can also get registered function objects directly from the
> function
> >>>> service using FunctionService.getFunction(String) and do whatever
> they want
> >>>> with them, which I guess could include serializing them.
> >>>>
> >>>> Hope that helps!
> >>>> -Dan
> >>>>
> >>>> On Wed, Sep 11, 2019 at 10:27 AM Aaron Lindsey 
> >>>> wrote:
> >>>>
> >>>>> As part of a PR to add Micrometer timers for function executions
> >>>>> <https://github.com/apache/geode/pull/4038>, we implemented a
> decorator
> >>>>> Function that wraps all registered non-internal functions and adds
> >>>>> instrumentation. This PR is
> >>>>> failing AnalyzeSerializablesJUnitTest.testSerializables because the
> >>>>> decorator class is a new Serializable.
> >>>>>
> >>>>> I'm not sure if it would be OK to add this class to
> excludedClasses.txt
> >>>>> because I don't know whether this function will ever be serialized.
> If it
> >>>>> will be serialized, then I'm concerned that this might break
> backwards
> >>>>> compatibility because we're changing the serialized form of
> registered
> >>>>> functions. If this is the case, then we could implement custom logic
> for
> >>>>> serializing the decorator class which would replace its serialized
> form
> >>>>> with the serialized form of the inner class. Again, I'm not sure if
> that
> >>>>> would be necessary because I don't know the conditions under which a
> >>>>> function would be serialized.
> >>>>>
> >>>>> Could someone help me understand when functions would be persisted or
> >>>> sent
> >>>>> over the wire so I can determine if this change would break
> >>>> compatibility?
> >>>>>
> >>>>> Thanks,
> >>>>> Aaron
> >>>>>
> >>>>
> >>
> >
>
>

Re: Question about excluding serialized classes

[Aaron Lindsey]

Thanks for your response, Dan.

The second scenario you mentioned (i.e. users calling
FunctionService.getFunction(String)) worries me because our PR changes the
FunctionService so they would now get back an instance of the decorator
function instead of the specific instance they registered by calling
FunctionService.registerFunction(Function). Therefore, any explicit casts
to a Function implementation like (MyFunction)
FunctionService.getFunction("MyFunction") would fail. Does that mean this
be a breaking change? The FunctionService class does not specify that
getFunction must return the same type function as the one passed to
registerFunction, but I could see how users might be relying on that
behavior since there is no other way to get a specific function type out of
the FunctionService without doing a cast.

- Aaron


On Wed, Sep 11, 2019 at 10:52 AM Dan Smith  wrote:

> Functions are serialized when you call Execution.execute(Function) instead
> of Execution.execute(String). Invoking execute on a function object
> serializes the function and executes it on the remote side. Functions
> executed this way don't have be registered.
>
> Users can also get registered function objects directly from the function
> service using FunctionService.getFunction(String) and do whatever they want
> with them, which I guess could include serializing them.
>
> Hope that helps!
> -Dan
>
> On Wed, Sep 11, 2019 at 10:27 AM Aaron Lindsey 
> wrote:
>
> > As part of a PR to add Micrometer timers for function executions
> > <https://github.com/apache/geode/pull/4038>, we implemented a decorator
> > Function that wraps all registered non-internal functions and adds
> > instrumentation. This PR is
> > failing AnalyzeSerializablesJUnitTest.testSerializables because the
> > decorator class is a new Serializable.
> >
> > I'm not sure if it would be OK to add this class to excludedClasses.txt
> > because I don't know whether this function will ever be serialized. If it
> > will be serialized, then I'm concerned that this might break backwards
> > compatibility because we're changing the serialized form of registered
> > functions. If this is the case, then we could implement custom logic for
> > serializing the decorator class which would replace its serialized form
> > with the serialized form of the inner class. Again, I'm not sure if that
> > would be necessary because I don't know the conditions under which a
> > function would be serialized.
> >
> > Could someone help me understand when functions would be persisted or
> sent
> > over the wire so I can determine if this change would break
> compatibility?
> >
> > Thanks,
> > Aaron
> >
>

Question about excluding serialized classes

[Aaron Lindsey]

As part of a PR to add Micrometer timers for function executions
, we implemented a decorator
Function that wraps all registered non-internal functions and adds
instrumentation. This PR is
failing AnalyzeSerializablesJUnitTest.testSerializables because the
decorator class is a new Serializable.

I'm not sure if it would be OK to add this class to excludedClasses.txt
because I don't know whether this function will ever be serialized. If it
will be serialized, then I'm concerned that this might break backwards
compatibility because we're changing the serialized form of registered
functions. If this is the case, then we could implement custom logic for
serializing the decorator class which would replace its serialized form
with the serialized form of the inner class. Again, I'm not sure if that
would be necessary because I don't know the conditions under which a
function would be serialized.

Could someone help me understand when functions would be persisted or sent
over the wire so I can determine if this change would break compatibility?

Thanks,
Aaron

Re: [VOTE] Apache Geode 1.10.0.RC1

[Aaron Lindsey]

+1

   - Built from source and ran unit tests
   - Used GFSH to create a locator and server and do some puts/gets
   - Checked version in GFSH
   - Built and ran all of the examples
   - Verified SHAs and signatures

- Aaron


On Thu, Sep 5, 2019 at 10:16 AM Nabarun Nag  wrote:

> Hello,
>
> I was able to notice that after running the testSerializable JUnit test,
> the generated actualSerializables.dat and the
> sanctioned-geode-core-serializables.txt do not match. There are certain
> classes mentioned in sanctioned-geode-core-serializables.txt that are not
> present in actualSerializables.dat file
>
>  - EvictionAttributes
>  - ExpirationAttributes
>  - MembershipAttributes
>  - SubscriptionAttributes
>  - EvictionAttributesImpl
>  - PartitionAttributesImpl
>  - CacheRealizaitonFunction
>
>
> But removing them causes testSanctionedClassesExistAndDoDeserialize() test
> to fail.
>
> I am not sure if this is harmless or has some adverse consequences. I would
> like to know why it's designed this way.
>
> Regards
> Nabarun Nag
>
>
>
> On Wed, Sep 4, 2019 at 4:14 PM Dick Cavender  wrote:
>
> > We manually signed the apache-geode-1.10.0-src.tgz dist and uploaded the
> > asc file.
> >
> > Unclear on why this is no longer automatically generated as part of the
> > build step as 1.9.1 it was generated correctly. We have worked around it
> in
> > the prepare_rc.sh adding a check for it going forward and generating it
> if
> > missing.
> >
> >
> > On Wed, Sep 4, 2019 at 3:32 PM Dan Smith  wrote:
> >
> > > I don't see a .asc signature file for apache-geode-1.10.0-src.tgz. Did
> we
> > > miss that signature file somehow?
> > >
> > > -Dan
> > >
> > > On Wed, Sep 4, 2019 at 9:33 AM Dick Cavender 
> > wrote:
> > >
> > > > The apache-geode-native-1.10.0-src.tar.gz dist has been fixed in RC1
> > and
> > > > can be found at:
> > > https://dist.apache.org/repos/dist/dev/geode/1.10.0.RC1/
> > > > Please continue to review RC1 as a viable 1.10 RC. The voting
> deadline
> > > > remains 3PM PST Thursday Sept 5th.
> > > >
> > > > -Dick
> > > >
> > > >
> > > > On Tue, Sep 3, 2019 at 3:09 PM Dan Smith  wrote:
> > > >
> > > > > Everything but the missing native source looks good. If we can fix
> > > that,
> > > > > I'll +1 this RC.
> > > > >
> > > > > -Dan
> > > > >
> > > > > On Tue, Sep 3, 2019 at 2:26 PM Dan Smith 
> wrote:
> > > > >
> > > > > > -1 It looks like this RC is also missing the native source, just
> > like
> > > > > > 1.9.1.RC3. The tar file is there, but it is empty.
> > > > > >
> > > > > > -Dan
> > > > > >
> > > > > > On Fri, Aug 30, 2019 at 2:06 PM Dick Cavender <
> > dcaven...@pivotal.io>
> > > > > > wrote:
> > > > > >
> > > > > >> Hello Geode dev community,
> > > > > >>
> > > > > >> This is a release candidate for Apache Geode, version
> 1.10.0.RC1.
> > > > > >> Thanks to all the community members for their contributions to
> > this
> > > > > >> release!
> > > > > >>
> > > > > >> Please do a review and give your feedback. The deadline is 3PM
> PST
> > > > > >> Thursday
> > > > > >> Sept 5th.
> > > > > >> Release notes can be found at:
> > > > > >>
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.10.0
> > > > > >>
> > > > > >> Please note that we are voting upon the source tags:
> > rel/v1.10.0.RC1
> > > > > >>
> > > > > >> Apache Geode:
> > > > > >> https://github.com/apache/geode/tree/rel/v1.10.0.RC1
> > > > > >> Apache Geode examples:
> > > > > >> https://github.com/apache/geode-examples/tree/rel/v1.10.0.RC1
> > > > > >> Apache Geode native:
> > > > > >> https://github.com/apache/geode-native/tree/rel/v1.10.0.RC1
> > > > > >>
> > > > > >> Source and binary files:
> > > > > >> https://dist.apache.org/repos/dist/dev/geode/1.10.0.RC1/
> > > > > >>
> > > > > >> Maven staging repo:
> > > > > >>
> > > >
> https://repository.apache.org/content/repositories/orgapachegeode-1058
> > > > > >>
> > > > > >> Geode's KEYS file containing PGP keys we use to sign the
> release:
> > > > > >> https://github.com/apache/geode/blob/develop/KEYS
> > > > > >>
> > > > > >> PS: Command to run geode-examples: ./gradlew -PgeodeReleaseUrl=
> > > > > >> https://dist.apache.org/repos/dist/dev/geode/1.10.0.RC1
> > > > > >> -PgeodeRepositoryUrl=
> > > > > >>
> > > >
> https://repository.apache.org/content/repositories/orgapachegeode-1058
> > > > > >> build runAll
> > > > > >>
> > > > > >> Regards
> > > > > >> Dick Cavender
> > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] RFC - Move membership code to a separate gradle sub-project

[Aaron Lindsey]

+1 — I'm happy to see us move toward better testability for the membership
code!

I also left my "+1" on the wiki page comments. I noticed that the
"Lightweight RFC Process" document states that we're supposed to have
discussions on the [DISCUSS] thread:

In addition a [DISCUSS] email should be send to the dev mailing list.
> Discussion will take place in the email thread.
>

Can we clarify how to have discussions on proposals? Personally, I don't
care either way as long as it's clear how the process is supposed to work.

- Aaron


On Wed, Sep 4, 2019 at 4:04 PM Bruce Schuchardt 
wrote:

> Thanks to folks who put in review comments today.  We're shooting for
> closing this RFC this week.
>
>
> On 8/30/19 3:50 PM, Dan Smith wrote:
> > Hi all,
> >
> > We added the following RFC to the wiki about moving Geode's membership
> > system to a separate gradle sub-project. Please review and comment by
> > 9/6/2019.
> >
> > https://cwiki.apache.org/confluence/x/WRB4Bw
> >
> > Thanks!
> > -Dan
> >
>

Re: [DISCUSS] Add a public API to add endpoints to Geode's HTTP server

[Aaron Lindsey]

Would it be practical to remove the dependency on Jetty from the
HttpService interface? I admit that I don't know a lot about this area of
the code, but I noticed that the current HttpService public interface
doesn't have any dependencies on Jetty (except the getHttpService() method,
which would be changed by this proposal). It seems desirable to decouple
the interface from the underlying web server in case we need to change it
later.

I like this proposal's intent and think it's an important area of work,
because any Geode user who wants to add a custom HTTP endpoint currently
has to rely on unstable internal APIs.

- Aaron


On Tue, Aug 20, 2019 at 1:34 PM Dale Emery  wrote:

> I’ve drafted a proposal to add a public API to add endpoints to Geode’s
> HTTP server.
>
>
> https://cwiki.apache.org/confluence/display/GEODE/%5BDraft%5D+Public+API+to+Add+Endpoints+to+Geode+HTTP+Server
> <
> https://cwiki.apache.org/confluence/display/GEODE/[Draft]+Public+API+to+Add+Endpoints+to+Geode+HTTP+Server
> 
> >
>
> Currently it is possible to serve additional HTTP content only by creating
> a separate server or by using internal Geode APIs to add endpoints to the
> existing server.
>
> We would like to give users a public way to serve additional HTTP content.
> For example, a user may wish to use a PrometheusMeterRegistry to publish
> Geode’s Micrometer-based metrics via HTTP. Geode’s existing HTTP server
> would be a convenient way to publish that information.
>
> I invite your feedback and ideas.
>
> Dale
>
> —
> Dale Emery
> dem...@pivotal.io
>
>

Re: Propose fix for 1.10 release: Prevent NPE in getLocalSize()

[Aaron Lindsey]

It sounds like there is consensus on adding this fix. Could someone please 
cherry-pick this for me?

Thanks,
Aaron

> On Aug 14, 2019, at 1:13 PM, Udo Kohlmeyer  wrote:
> 
> @Aaron,Kirk - thank you for the clarification.
> 
> +1 to include the fix, as reverting GEODE-7001 would be more effort :)
> 
> --Udo
> 
> On 8/14/19 9:25 AM, Aaron Lindsey wrote:
>> @Udo, I think Kirk explained it well — This issue was introduced very 
>> recently (right before we cut the release branch) and it has serious 
>> consequences (requires restarting the server).
>> 
>> - Aaron
>> 
>>> On Aug 14, 2019, at 9:06 AM, Kirk Lund  wrote:
>>> 
>>> +1 to include this fix in 1.10.0
>>> 
>>> FYI: The race condition for this code path to throw NPE (which is
>>> catastrophic and requires restarting the server) was introduced by commit
>>> 279fa0 on July 31 for GEODE-7001.
>>> 
>>> On Tue, Aug 13, 2019 at 6:22 PM Anthony Baker  wrote:
>>> 
>>>> Given that we’re trying to stabilize the release branch and this fix seems
>>>> to *help* that I’m in favor of merging it.
>>>> 
>>>> Anthony
>>>> 
>>>> 
>>>>> On Aug 13, 2019, at 5:32 PM, Udo Kohlmeyer  wrote:
>>>>> 
>>>>> @Aaron, is this an existing issue (i.e this was not introduced in a
>>>> current refactor)?
>>>>> If the answer is anything other that "This will make the system stop
>>>> working", I would vote: -1
>>>>> If this is an existing issue and has been around for a while, I think we
>>>> hold off including this.
>>>>> I think the boat has sailed on the inclusion of issues into the 1.10
>>>> release. Sorry...
>>>>> --Udo
>>>>> 
>>>>> On 8/13/19 4:58 PM, Aaron Lindsey wrote:
>>>>>> I’d like to propose including
>>>> https://github.com/apache/geode/pull/3913/commits/6f1814d1f719cc06b13769c40a9d6d01f99f927c
>>>> <
>>>> https://github.com/apache/geode/pull/3913/commits/6f1814d1f719cc06b13769c40a9d6d01f99f927c>
>>>> in the Geode 1.10 release.
>>>>>> This commit fixes an issue where a NullPointerException is thrown from
>>>> PartitionedRegion.getLocalSize() when the statistics callback sampler is
>>>> invoked before a PartitionedRegion is initialized.
>>>>>> - Aaron
>>>>>> 
>>>>>> 
>>>>>> 
>>>> 

Re: Propose fix for 1.10 release: Prevent NPE in getLocalSize()

[Aaron Lindsey]

@Udo, I think Kirk explained it well — This issue was introduced very recently 
(right before we cut the release branch) and it has serious consequences 
(requires restarting the server).

- Aaron

> On Aug 14, 2019, at 9:06 AM, Kirk Lund  wrote:
> 
> +1 to include this fix in 1.10.0
> 
> FYI: The race condition for this code path to throw NPE (which is
> catastrophic and requires restarting the server) was introduced by commit
> 279fa0 on July 31 for GEODE-7001.
> 
> On Tue, Aug 13, 2019 at 6:22 PM Anthony Baker  wrote:
> 
>> Given that we’re trying to stabilize the release branch and this fix seems
>> to *help* that I’m in favor of merging it.
>> 
>> Anthony
>> 
>> 
>>> On Aug 13, 2019, at 5:32 PM, Udo Kohlmeyer  wrote:
>>> 
>>> @Aaron, is this an existing issue (i.e this was not introduced in a
>> current refactor)?
>>> 
>>> If the answer is anything other that "This will make the system stop
>> working", I would vote: -1
>>> 
>>> If this is an existing issue and has been around for a while, I think we
>> hold off including this.
>>> 
>>> I think the boat has sailed on the inclusion of issues into the 1.10
>> release. Sorry...
>>> 
>>> --Udo
>>> 
>>> On 8/13/19 4:58 PM, Aaron Lindsey wrote:
>>>> I’d like to propose including
>> https://github.com/apache/geode/pull/3913/commits/6f1814d1f719cc06b13769c40a9d6d01f99f927c
>> <
>> https://github.com/apache/geode/pull/3913/commits/6f1814d1f719cc06b13769c40a9d6d01f99f927c>
>> in the Geode 1.10 release.
>>>> 
>>>> This commit fixes an issue where a NullPointerException is thrown from
>> PartitionedRegion.getLocalSize() when the statistics callback sampler is
>> invoked before a PartitionedRegion is initialized.
>>>> 
>>>> - Aaron
>>>> 
>>>> 
>>>> 
>> 
>> 

Propose fix for 1.10 release: Prevent NPE in getLocalSize()

[Aaron Lindsey]

I’d like to propose including 
https://github.com/apache/geode/pull/3913/commits/6f1814d1f719cc06b13769c40a9d6d01f99f927c
 

 in the Geode 1.10 release.

This commit fixes an issue where a NullPointerException is thrown from 
PartitionedRegion.getLocalSize() when the statistics callback sampler is 
invoked before a PartitionedRegion is initialized.

- Aaron

Failing LGTM Check

[Aaron Lindsey]

My PR has a LGTM check that shows “Failing” on the PR and LGTM site, but the 
LGTM logs say “Succeeded”:
Failing PR check: https://github.com/apache/geode/pull/3913/checks 

Failing check on LGTM site: 
https://lgtm.com/projects/g/apache/geode/rev/pr-f50cce2f02aa0f88f0d6beecbf236b29bc29683a
 

LGTM logs with “Succeeded” message: 
https://lgtm.com/projects/g/apache/geode/logs/rev/pr-f50cce2f02aa0f88f0d6beecbf236b29bc29683a/lang:java/stage:Preparing%20merge%20commit_6f1814d1f719cc06b13769c40a9d6d01f99f927c
 


How should I proceed with fixing this failing check?

- Aaron

Re: [DISCUSS] Release Geode 1.9.1 with logging improvements

[Aaron Lindsey]

Thanks, Udo.

+1 for doing a 1.9.1 patch release, assuming there is enough time for SBDG to 
include it in their 1.2.x line.

- Aaron 

> On Aug 13, 2019, at 12:38 PM, Udo Kohlmeyer  wrote:
> 
> No, 1.9.1 IS something we require. SBDG 1.2 CAN use 1.9.1, we'd have to wait 
> for SBDG 1.3 to use 1.10 or 1.11
> 
> SBDG 1.3 is still a few months off, so maybe getting critical fixes in patch 
> versions is required.
> 
> On 8/13/19 11:26 AM, Kirk Lund wrote:
>> Udo, Thanks for the info! Sounds like we shouldn't bother with Geode 1.9.1
>> then. If I'm misinterpreting what you wrote, let me know.
>> 
>> On Tue, Aug 13, 2019 at 10:36 AM Udo Kohlmeyer  wrote:
>> 
>>> The latest version of SBDG 1.2 is already in RC stage. Which means the
>>> dependent Geode version cannot be changed any more. Currently SBDG 1.2
>>> is based on Geode 1.9. This will not change. Patch versions to 1.9 are
>>> supported, but not changes to 1.10 or later.
>>> 
>>> THUS,
>>> 
>>> Once SBDG 1.3 (Neuman) is released, it will be based on the latest GA of
>>> Geode, which at this point would be 1.10 or possibly 1.11 depending on
>>> release cycles.
>>> 
>>> In addition...
>>> 
>>> @Aaron, Whilst it would also be possible to override the underlying
>>> Geode version that SBDG uses, to a later version, I would just like to
>>> point out that all testing of SBDG will be against a named supported
>>> version of Geode / GemFire. Which means, if failures arise using SBDG /
>>> SDG with a non-supported version of Geode / GemFire would effectively be
>>> unsupported. (due diligence to confirm origin of failure will of course
>>> be applied)
>>> 
>>> Hope this helps...
>>> 
>>> --Udo
>>> 
>>> On 8/13/19 10:03 AM, Aaron Lindsey wrote:
>>>> Assuming Geode 1.10 is released with the three logging fixes in Kirk’s
>>> message, can the next GA release of Spring Boot Data Geode consume 1.10
>>> instead of 1.9? Also, when would SBDG need this patch release by (whether
>>> we do a 1.9.1 release or 1.10 release)?
>>>> - Aaron
>>>> 
>>>>> On Aug 13, 2019, at 9:31 AM, Bruce Schuchardt 
>>> wrote:
>>>>> If we release a 1.9.1 I'd like to include the SSL/NIO fix. Cluster SSL
>>> communications with conserve-sockets=false is currently broken in 1.9.
>>>>> On 8/13/19 9:25 AM, Kirk Lund wrote:
>>>>>> I'd like to discuss if and how we can release Geode 1.9.1 with logging
>>>>>> improvements. This is primarily to provide a patch release for Spring
>>> Data
>>>>>> Geode and Spring Boot to ensure a smoother User experience out-of-the
>>> box.
>>>>>> They have very near-future releases that need this as soon as possible.
>>>>>> 
>>>>>> The specific tickets and commits that would be back-ported are:
>>>>>> 
>>>>>> *1. GEODE-7058 Log4j-core dependency should be optional in geode-core*
>>>>>> 
>>>>>> commit 413800bc16d05df689a2af5c30797f180aad6088
>>>>>> Author: Kirk Lund 
>>>>>> Date:   Wed Aug 7 14:33:21 2019 -0700
>>>>>> 
>>>>>>  GEODE-7058: Mark log4j-core optional in geode-core
>>>>>> 
>>>>>>  Note: this change requires all commits from GEODE-2644 and
>>> GEODE-6122.
>>>>>> *2. GEODE-7050 Log4jAgent should avoid casting non-log4j loggers*
>>>>>> 
>>>>>> commit e5c9c420f462149fd062847904e3435fbe99afb4
>>>>>> Author: Kirk Lund 
>>>>>> Date:   Thu Aug 8 18:17:32 2019 -0700
>>>>>> 
>>>>>>  GEODE-7050: Use Log4jAgent only if Log4j is using Log4jProvider
>>> (#3892)
>>>>>>  This change prevents Geode from using Log4jAgent if Log4j Core is
>>>>>>  present but not using Log4jProvider.
>>>>>> 
>>>>>>  For example, Log4j uses SLF4JProvider when log4j-to-slf4j is in
>>> the
>>>>>> classpath.
>>>>>> 
>>>>>>  By disabling Log4jAgent when other Log4j Providers are in use,
>>> this
>>>>>>  prevents problems such as ClassCastExceptions when attemping to
>>> cast
>>>>>>  loggers from org.apache.logging.slf4j.SLF4JLogger to
>>>>>>  org.apache.logging.log4j.core.Logger to get the LoggerConfig or
>>>>>>  LoggerContext.
>>>>>> 
>>>>>>  Co-Authored-By: Aaron Lindsey 
>>>>>> 
>>>>>> *3. GEODE-6959 NPE if AlertAppender is not defined*
>>>>>> 
>>>>>> commit dd15fec1f2ecbc3bc0cdfc42072252c379e0bb89
>>>>>> Author: Kirk Lund 
>>>>>> Date:   Thu Aug 8 14:59:44 2019 -0700
>>>>>> 
>>>>>>  GEODE-6959: Prevent NPE in GMSMembershipManager for null
>>> AlertAppender
>>>>>> (#3899)
>>>>>> 
>>>>>>  If a custom log4j2.xml is used without specifying the Geode
>>>>>> AlertAppender,
>>>>>>  GMSMembershipManager may throw a NullPointerException when
>>> invoking
>>>>>>  AlertAppender.getInstance().stopSession() during a
>>> forceDisconnect. This
>>>>>>  change prevents the NullPointerException allowing forceDisconnect
>>> to
>>>>>> finish.
>>>>>> 
>>>>>>  Users using Spring Boot with Logback are more likely to hit this
>>> bug.
>>>>>>  Co-authored-by: Mark Hanson mhan...@pivotal.io
>>>>>> 

Re: [DISCUSS] Release Geode 1.9.1 with logging improvements

[Aaron Lindsey]

Assuming Geode 1.10 is released with the three logging fixes in Kirk’s message, 
can the next GA release of Spring Boot Data Geode consume 1.10 instead of 1.9? 
Also, when would SBDG need this patch release by (whether we do a 1.9.1 release 
or 1.10 release)?

- Aaron

> On Aug 13, 2019, at 9:31 AM, Bruce Schuchardt  wrote:
> 
> If we release a 1.9.1 I'd like to include the SSL/NIO fix. Cluster SSL 
> communications with conserve-sockets=false is currently broken in 1.9.
> 
> On 8/13/19 9:25 AM, Kirk Lund wrote:
>> I'd like to discuss if and how we can release Geode 1.9.1 with logging
>> improvements. This is primarily to provide a patch release for Spring Data
>> Geode and Spring Boot to ensure a smoother User experience out-of-the box.
>> They have very near-future releases that need this as soon as possible.
>> 
>> The specific tickets and commits that would be back-ported are:
>> 
>> *1. GEODE-7058 Log4j-core dependency should be optional in geode-core*
>> 
>> commit 413800bc16d05df689a2af5c30797f180aad6088
>> Author: Kirk Lund 
>> Date:   Wed Aug 7 14:33:21 2019 -0700
>> 
>> GEODE-7058: Mark log4j-core optional in geode-core
>> 
>> Note: this change requires all commits from GEODE-2644 and GEODE-6122.
>> 
>> *2. GEODE-7050 Log4jAgent should avoid casting non-log4j loggers*
>> 
>> commit e5c9c420f462149fd062847904e3435fbe99afb4
>> Author: Kirk Lund 
>> Date:   Thu Aug 8 18:17:32 2019 -0700
>> 
>> GEODE-7050: Use Log4jAgent only if Log4j is using Log4jProvider (#3892)
>> 
>> This change prevents Geode from using Log4jAgent if Log4j Core is
>> present but not using Log4jProvider.
>> 
>> For example, Log4j uses SLF4JProvider when log4j-to-slf4j is in the
>>classpath.
>> 
>> By disabling Log4jAgent when other Log4j Providers are in use, this
>> prevents problems such as ClassCastExceptions when attemping to cast
>> loggers from org.apache.logging.slf4j.SLF4JLogger to
>> org.apache.logging.log4j.core.Logger to get the LoggerConfig or
>> LoggerContext.
>> 
>> Co-Authored-By: Aaron Lindsey 
>> 
>> *3. GEODE-6959 NPE if AlertAppender is not defined*
>> 
>> commit dd15fec1f2ecbc3bc0cdfc42072252c379e0bb89
>> Author: Kirk Lund 
>> Date:   Thu Aug 8 14:59:44 2019 -0700
>> 
>> GEODE-6959: Prevent NPE in GMSMembershipManager for null AlertAppender
>> (#3899)
>> 
>> If a custom log4j2.xml is used without specifying the Geode
>> AlertAppender,
>> GMSMembershipManager may throw a NullPointerException when invoking
>> AlertAppender.getInstance().stopSession() during a forceDisconnect. This
>> change prevents the NullPointerException allowing forceDisconnect to
>> finish.
>> 
>> Users using Spring Boot with Logback are more likely to hit this bug.
>> 
>> Co-authored-by: Mark Hanson mhan...@pivotal.io
>> 

Re: Fix for NPE during forceDisconnect candidate for 1.10.0

[Aaron Lindsey]

+1

> On Aug 13, 2019, at 9:59 AM, Kirk Lund  wrote:
> 
> I guess we need at least one more vote to get this fix copied to the 1.10.0
> release branch. We just confirmed that this fix is NOT on that branch yet.
> 
> On Thu, Aug 8, 2019 at 11:03 AM Juan José Ramos  wrote:
> 
>> +1
>> 
>> On Thu, Aug 8, 2019 at 7:02 PM John Blum  wrote:
>> 
>>> +1 for Kirk's changes in 1.10.  This will be critical for SD Neuman and
>>> SBDG 1.3.
>>> 
>>> On Thu, Aug 8, 2019 at 10:57 AM Owen Nichols 
>> wrote:
>>> 
 Hi Kirk and Mark, thank you for bringing your concern.
 
 Our “critical fixes” rule allows critical fixes to be brought to the
 release branch by proposal on the dev list, as you have just done.  If
 there is consensus from the Geode community that this NPE fix satisfies
>>> the
 “critical fixes” rule, Dick or I will be happy to bring it to the
>> 1.10.0
 release branch.
 
 -Owen
 
> On Aug 8, 2019, at 10:54 AM, Kirk Lund  wrote:
> 
> I'd like to propose including the fix for GEODE-6959 in 1.10.0.
> 
> Spring Boot users are very likely to hit this NPE during
>>> forceDisconnect.
> 
> If a custom log4j2.xml is used without specifying the Geode
 AlertAppender,
> GMSMembershipManager may throw a NullPointerException when invoking
> AlertAppender.getInstance().stopSession() during a forceDisconnect.
>>> This
> change prevents the NullPointerException allowing forceDisconnect to
 finish.
> Tests are included with this fix.
> 
> PR: https://github.com/apache/geode/pull/3899
> GEODE-6959: NPE if AlertAppender is not defined
> 
> Thanks,
> Kirk and Mark
 
 
>>> 
>>> --
>>> -John
>>> john.blum10101 (skype)
>>> 
>> 
>> 
>> --
>> Juan José Ramos Cassella
>> Senior Technical Support Engineer
>> Email: jra...@pivotal.io
>> Office#: +353 21 4238611
>> Mobile#: +353 87 2074066
>> After Hours Contact#: +1 877 477 2269
>> Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
>> How to upload artifacts:
>> https://support.pivotal.io/hc/en-us/articles/204369073
>> How to escalate a ticket:
>> https://support.pivotal.io/hc/en-us/articles/203809556
>> 
>> [image: support]  [image: twitter]
>>  [image: linkedin]
>>  [image: facebook]
>>  [image: google plus]
>>  [image: youtube]
>> 
>> 

Re: [DISCUSS] Geode dependency update process (review by 8/28/2019)

[Aaron Lindsey]

I like the idea of proactively updating dependencies after each release. For 
this to work we would have to know whether the next release will be a major or 
minor release directly after each GA release (so that we can bump either major 
or minor versions, as appropriate). How and when do we currently determine our 
major release cycle? Would we know at the time of a GA release whether the next 
release would be a major or minor release?

- Aaron

> On Aug 13, 2019, at 9:22 AM, Nicholas Vallely  wrote:
> 
> https://cwiki.apache.org/confluence/display/GEODE/%5BDraft%5D+Geode+dependency+update+process
> 
> Here is the content of the wiki proposal above for a discussion:
> Problem
> 
> We recently updated the dependencies for the log4j version used in Geode to
> keep up with Spring Boot Data Geode's logging dependencies. As far as I
> know, we do not have a process to keep dependencies up to date or regularly
> scheduled updates to them. Currently, I believe this is handled ad-hoc
> which hasn't necessarily caused any major issues but could.
> Anti-GoalsSolution
> 
> *Directly after GA release of Geode minor version:*
> The release manager for the most recently released version of Geode would
> review any dependencies in the Geode project (presumably this will/could be
> automated).
> 
>   - For a minor release, only minor version dependency updates should be
>   considered
>   - For a major release, major versions should be considered
> 
> The release manager would submit a PR to update dependencies and then the
> community should pitch in to tackle any subsequent issues that arise from
> the updating of dependencies. *Note the first time this happens maybe
> painful*
> 
> *In-between releases:*
> We keep doing what we are doing:
> 
>   - Ad-hoc dependency updates as necessary
> 
> *When a new release manager is chosen:*
> The release manager would send out an email as the last call for dependency
> updates that would coincide with a proposed release branch cut date. This
> would give everyone a reminder that if they need to update a dependency
> prior to the release there is limited time left in order to do so.
> Changes and Additions to Public Interfaces
> 
> *n/a*
> Performance Impact
> 
> *Potentially a new version of a dependency could cause a performance impact
> and we should run a performance test suite on the recently released version
> vs the updated dependency version*
> Backwards Compatibility and Upgrade Path
> 
> *In a minor release, minor version dependency updates shouldn't cause
> compatibility issues.*
> Prior Art
> 
> *What would be the alternatives to the proposed solution? What would happen
> if we don’t solve the problem? Why should this proposal be preferred?*
> 
> *If we continue to do this ad-hoc, there is a greater likelihood of CVE's
> or mismatching versions of conflicts between Geode and dependent projects.*
> 
> 
> *Nick*

Re: Draft of Apache Geode Quarterly Report (Aug 2019) for your review

[Aaron Lindsey]

Kind of a small thing, but the Committer-to-PMC ratio seems more like 2:1 based 
on the numbers of committers and PMC members below.

- Aaron

> On Aug 12, 2019, at 2:22 PM, Dave Barnes  wrote:
> 
> Reformatted for more amenable viewing:
> 
> ## Description:
> 
> The mission of Apache Geode is the creation and maintenance of software
> related
> 
> to a data management platform that provides real-time, consistent access to
> 
> data-intensive applications throughout widely distributed cloud
> architectures.
> 
> 
> ## Issues:
> 
> There are no issues requiring board attention at this time.
> 
> 
> ## Membership Data:
> 
> Apache Geode was founded 2016-11-15 (3 years ago) There are currently 102
> 
> committers and 50 PMC members in this project. The Committer-to-PMC ratio is
> 
> roughly 7:4.
> 
> 
> Community changes, past quarter:
> 
> - No new PMC members. Last additions were Juan Ramos and Robert Houghton on
> 
>  2019-02-20
> 
> - Dale Emery was added as committer on 2019-06-19
> 
> - Mark Hanson was added as committer on 2019-06-19
> 
> 
> ## Project Activity:
> 
> Release 1.10 is in progress.
> 
> 
> An Apache Geode Summit is scheduled the day before the SpringOne Platform
> 2019
> 
> conference, on 7 October 2019, in Austin, TX. Several Geode contributors
> will
> 
> present talks and sessions aimed at a variety of levels.
> 
> 
> ## Community Health:
> 
> Mailing lists remain active and productive.
> 
>  - dev@geode.apache.org had a 13% increase in traffic in the past quarter
> 
>  (505 emails compared to 443)
> 
> 
> JIRA tickets show that issues continue to be identified and resolved.
> 
>  - 289 issues opened in JIRA in the past quarter
> 
>  - 296 issues closed in   JIRA in the past quarter
> 
>  - 454 commits in the past quarter
> 
> 
> The community is actively contributing to the Apache Geode code base:
> 
>  - 62 code contributors in the past quarter
> 
>  - 344 PRs opened on GitHub in the past quarter
> 
>  - 356 PRs closed on GitHub in the past quarter
> 
> On Mon, Aug 12, 2019 at 2:10 PM Dave Barnes  wrote:
> 
>> Please review by Noon PDT, Tuesday, Aug 13. Thanks!
>> 
>> ## Description:
>> 
>> The mission of Apache Geode is the creation and maintenance of software
>> related
>> to a data management platform that provides real-time, consistent access
>> to
>> data-intensive applications throughout widely distributed cloud
>> architectures.
>> 
>> ## Issues: There are no issues requiring board attention at this time. ##
>> Membership Data: Apache Geode was founded 2016-11-15 (3 years ago) There
>> are currently 102 committers and 50 PMC members in this project. The
>> Committer-to-PMC ratio is roughly 7:4. Community changes, past quarter: -
>> No new PMC members. Last additions were Juan Ramos and Robert Houghton on
>> 2019-02-20 - Dale Emery was added as committer on 2019-06-19 - Mark Hanson
>> was added as committer on 2019-06-19 ## Project Activity: Release 1.10 is
>> in progress. An Apache Geode Summit is scheduled the day before the
>> SpringOne Platform 2019 conference, on 7 October 2019, in Austin, TX.
>> Several Geode contributors will present talks and in traffic in the past
>> quarter (505 emails compared to 443) JIRA tickets show that issues continue
>> to be identified and resolved. - 289 issues opened in JIRA in the past
>> quarter - 296 issues closed in JIRA in the past quarter - 454 commits in
>> the past quarter The community is actively contributing to the Apache
>> Geode code base: - 62 code contributors in the past quarter - 344 PRs
>> opened on GitHub in the past quarter - 356 PRs closed on GitHub in the past
>> quarter
>> 

Re: New release branch for Apache Geode 1.10.0

[Aaron Lindsey]

+1 to including this PR in the 1.10.0 release.

Just to elaborate on the issue: If the stats sampler calls 
LocalRegion.getLocalSize before that region is initialized, it will throw a 
NullPointerException. This can happen because we have escaping references to 
“this” in the LocalRegion constructor that allow the stats sampler to call 
getLocalSize before the LocalRegion object is fully constructed.

- Aaron

> On Aug 2, 2019, at 5:21 PM, Michael Oleske  wrote:
> 
> Naba's concern was this PR: https://github.com/apache/geode/pull/3880 which
> has been merged to develop
> 
> -michael
> 
> On Fri, Aug 2, 2019 at 4:33 PM Owen Nichols  wrote:
> 
>> Hi Naba, thank you for bringing your concern.
>> 
>> Our current process <
>> https://lists.apache.org/thread.html/d36a63c3794d13506ecad3d52a2aca938dcf0f8509b61860bbbc50cd@%3Cdev.geode.apache.org%3E>
>> dictates a time-based schedule to cut release branches.  Once cut, the
>> “critical fixes” rule allows critical fixes to be brought to the release
>> branch by proposal on the dev list.
>> 
>> Currently the 1.10.0 release pipeline <
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-10-0-main>
>> is all green.  Perhaps one way to convince the Geode community that your
>> fix satisfies the “critical fixes” rule might be to share a link to a
>> github commit adding tests that expose the issue.
>> 
>> Thanks,
>> -Owen
>> 
>> 
>>> On Aug 2, 2019, at 12:55 PM, Nabarun Nag  wrote:
>>> 
>>> Hi,
>>> 
>>> We believe that the current release branch has an issue of getting a
>>> NullPointerException when we try calling LocalRegion.getLocalSize when
>> the
>>> region is in the process of being initialized. The fix for this issue is
>>> very critical and we should re-create the branch when the fix is placed
>> in
>>> develop.
>>> We need to hold any pipeline testing on the branch till this fix is in.
>>> 
>>> Regards
>>> Nabarun Nag
>>> 
>>> On Fri, Aug 2, 2019 at 11:17 AM Dick Cavender  wrote:
>>> 
 Hello Geode Dev Community,
 
 We have created a new release branch for Apache Geode 1.10.0 -
 "release/1.10.0"
 
 Please do review and raise any concern with the release branch. If no
 concerns are raised, we will start with the voting for the release
 candidate soon.
 
 Regards
 
 Dick Cavender
 Apache Geode 1.10.0 Release Manager
 
>> 
>> 

PR Reviews

[Aaron Lindsey]

Would anyone be able to review and/or merge the following 2 PRs?
GEODE-6298: Fix flaky test scanMovesRecentlyUsedNodeToTail 

GEODE-7003: Fix flaky tests in GemFireTransactionDataSourceIntegrationTest 


Thanks,
Aaron

Re: IntelliJ setup for develop

[Aaron Lindsey]

I had the same issue. Now I use the same configuration as Jens and that fixed 
the issue. The only problem is that I feel the Gradle build takes longer than 
IntelliJ’s build.

- Aaron

> On Jul 24, 2019, at 2:12 PM, Jens Deppe  wrote:
> 
> I'd suggest not trying to build with IntelliJ, but delegate to Gradle.
> (Search for 'gradle' in Settings and then set 'Build and run using *Gradle*').
> You can still run tests with IntelliJ (this is my preference as I feel it's
> faster).
> 
> On Wed, Jul 24, 2019 at 2:03 PM Sai Boorlagadda 
> wrote:
> 
>> Building/Rebuilding in IntelliJ fails with error "Cannot start compilation:
>> the output path is not specified for modules". Any help would be
>> appreciated. I have been following BUILDING.md[1] to setup intellij.
>> 
>> [1] https://github.com/apache/geode/blob/develop/BUILDING.md
>> 

Re: [DISCUSS] RFC 0: Lightweight RFC Process

[Aaron Lindsey]

This is great! Thanks!

- Aaron

> On Jul 12, 2019, at 1:43 PM, Alexander Murmann  wrote:
> 
> Thanks, Dan!
> 
> On Fri, Jul 12, 2019 at 1:35 PM Mark Hanson  wrote:
> 
>> Thanks for taking the initiative Dan!
>> 
>>> On Jul 12, 2019, at 12:57 PM, Dan Smith  wrote:
>>> 
>>> Following up on this, I took a stab at organizing our old proposals into
>>> the buckets on the wiki. We now have:
>>> 
>>> Under Discussion - Draft and In Discussion proposals
>>> In Development - proposals under active development
>>> Active - Proposals that are completely implemented
>>> Dropped - Proposals that were not approved or development stalled out.
>>> 
>>> If I moved your proposal to "Dropped" erroneously, please feel free to
>> move
>>> it back! I moved things there that did not appear to have been
>> implemented
>>> or have any recent activity.
>>> 
>>> I put a few things in "Unknown State." If you know what state these
>>> proposals are in, please move them!
>>> 
>>> 
>> https://cwiki.apache.org/confluence/display/GEODE/Project+Proposals+and+Specifications
>>> 
>>> On Wed, Jun 26, 2019 at 11:20 AM Alexander Murmann 
>>> wrote:
>>> 
 Given discussion here and previous discussion on the PR, I consider this
 proposal approved and updated its state accordingly.
 
 I also incorporated Dan's suggestion of moving deprecated proposals and
 added a reference to the new process at the top of the Project Proposals
 and Specifications page
 <
 
>> https://cwiki.apache.org/confluence/display/GEODE/Project+Proposals+and+Specifications
> 
 .
 
 Thank you all for you great feedback throughout this process!
 
 On Tue, Jun 25, 2019 at 10:07 AM Dan Smith  wrote:
 
>> 
>> Will moving the page around on the wiki result in dead links to the
 draft
>> version?
>> 
> 
> No. If you use the share button in the wiki, you get a permanent link
>> to
> the page. Even if you just copy the URL from the address bar it doesn't
> include the folder the page is in.
> 
> -Dan
> 
 
 
 --
 Alexander J. Murmann
 (650) 283-1933
 
>> 
>> 

Re: [PROPOSAL]: Improve OQL Method Invocation Security

[Aaron Lindsey]

+1

I just re-reviewed this proposal and it looks good to me.

- Aaron

> On Jul 12, 2019, at 6:29 AM, Juan José Ramos  wrote:
> 
> Hello Mike,
> 
> Agreed, we'll probably need to create an enhancement request for this
> feature in JIRA.
> Cheers.
> 
> On Thu, Jul 11, 2019 at 5:37 PM Michael Stolz  wrote:
> 
>> One thing I will mention regarding DATA:READ:RegionName allowing query
>> behavior is that we have been asked by some users already to separate
>> DATA:READ:RegionName from DATA:QUERY:RegionName. This request is to protect
>> against arbitrary query execution by administrators that can cause huge
>> resource consumption.
>> 
>> So regardless of all the rest of the proposal, that's something we should
>> probably consider standardizing on.
>> 
>> --
>> Mike Stolz
>> Principal Engineer, Pivotal Cloud Cache
>> Mobile: +1-631-835-4771
>> 
>> 
>> 
>> On Thu, Jul 11, 2019 at 11:36 AM Juan José Ramos 
>> wrote:
>> 
>>> Hello all,
>>> 
>>> Friendly reminder regarding the deadline to rise concerns and/or
>> objections
>>> regarding the *OQL Method InvocationSecurity Proposal [1]*, I'll go ahead
>>> and move it to *Development* on July 13th.
>>> Best regards.
>>> 
>>> [1]:
>>> 
>>> 
>> https://cwiki.apache.org/confluence/display/GEODE/OQL+Method+Invocation+Security#OQLMethodInvocationSecurity-PriorArt
>>> 
>>> 
>>> On Mon, Jul 8, 2019 at 3:29 PM Juan José Ramos 
>> wrote:
>>> 
 Done [1]!.
 Please remember that, if no major concerns arise before Friday this
>> week,
 I'll go ahead and move the proposal to *Development* on July 13th.
 Best regards.
 
 [1]:
 
>>> 
>> https://cwiki.apache.org/confluence/display/GEODE/OQL+Method+Invocation+Security#OQLMethodInvocationSecurity-PriorArt
 
 On Fri, Jul 5, 2019 at 3:48 PM Jacob Barrett 
>>> wrote:
 
> Can you please add a Prior Art section to your proposal discussing
>> these
> alternative solutions and why they are insufficient?
> 
> Thanks,
> Jake
> 
> 
>> On Jul 5, 2019, at 10:41 AM, Juan José Ramos 
>>> wrote:
>> 
>> Hello Jake,
>> 
>> I've replied something similar *here [1]*.
>> Long story short, I haven't found anything that really applies to
>> our
> use
>> case. The "most similar solution" is *Spring Method Security [2]*,
>>> which
>> basically implies annotating methods with explicit configuration
>> about
> the
>> roles required to execute them. The same goes for *Shiro
> **Annotation-based
>> Authorization [3]*. The *AnnotationBasedMethodAuthorize**r [3]*
>>> approach
>> from the proposal is somewhat similar to this, but I've discarded it
>> because if forces the user to annotate classes with our own
>>> annotations,
>> basically forcing them to modify their domain model.
>> The proposal basically allows our users to use one of the default of
>>> the
>> box implementations and, if they don't like them for whatever
>> reason,
>>> is
>> flexible enough so they can ultimately provide their own.
>> Hope this helps.
>> Cheers.
>> 
>> [1]:
>> 
> 
>>> 
>> https://markmail.org/message/ekons7ixtz4jtf7n#query:+page:1+mid:snxgpsqd3yuppmsc+state:results
>> [2]:
>> 
> 
>>> 
>> https://docs.spring.io/spring-security/site/docs/5.1.5.RELEASE/reference/html/jc.html#jc-method
>> [3]:
>> 
> 
>>> 
>> https://shiro.apache.org/authorization.html#Authorization-AnnotationbasedAuthorization
>> [4]:
>> 
> 
>>> 
>> https://cwiki.apache.org/confluence/display/GEODE/OQL+Method+Invocation+Security#OQLMethodInvocationSecurity-AnnotationBasedMethodAuthorizer
>> 
>> On Fri, Jul 5, 2019 at 1:46 PM Jacob Barrett 
> wrote:
>> 
>>> So if we don’t want to use the Java built in SecurityManager to
>> solve
>>> this, because we feel it's too big or too inflexible for our needs,
> have
>>> other projects implemented something we can borrow? We can’t be the
> first
>>> to need something like this if Java’s solution isn’t a good fit.
>>> 
>>> Again I want to avoid inventing something new. What prior art is
>> out
> there?
>>> 
>>> 
 On Jul 4, 2019, at 1:29 PM, Juan José Ramos 
> wrote:
 
 Hello all,
 
 If you haven't added my email to the spam folder already :-), then
>>> I'd
>>> like
 to let you know that I've update again the *Proposal [1]* and
>>> incorporated
 most of the feedback provided, along with some additional
>>> information
> and
 context I missed on the previous versions, thanks all that brought
>>> concerns
 and suggestions to the discussion. Please take some time to review
>>> it
 thoroughly, adding comments and/or concerns *only on this email
> thread*,
 all feedback is more than welcome.
 If no major concerns arise before July 12th 2019, I'll go ahead
>> and
> mark
 move the proposal to 

Re: [DISCUSS] Adoption of a Coding Standard

[Aaron Lindsey]

+1 to having a recommended reading list. At first glance, the SEI standard
seems like an extremely useful resource, but I am hesitant to adopt it as
our "coding standard" without carefully reading all the way through it. I
would, however, be comfortable adding it to a recommended reading list.

- Aaron


On Mon, Jun 24, 2019 at 4:28 PM Owen Nichols  wrote:

> I like the idea of a recommended reading list for Geode contributors.
>
> My concerns around adopting broad standards and guidelines that can’t be
> automatically checked & applied are twofold:
>
>  a) what is the policy regarding existing code?  Is every PR going forward
> expected to bring every file it touches into conformance?
>
>  b) how do we avoid PR reviews devolving into style-nitpicking?  If a PR
> clearly fixes a bug, but someone doesn’t like a variable name, would that
> be reason to give it a -1?
>
> Even better would be leading by example [if we can’t overhaul the entire
> codebase, maybe at least be able to point to one file that really embodies
> our desired coding standards].
>
> People over process.  Code is either readable or it isn’t.  If you are
> reviewing a PR and the proposed changes are difficult to follow, a simple
> and kind explanation is probably more constructive than citing the offender
> with the chapter and section of their violation.
>
>
>
> > On Jun 24, 2019, at 3:31 PM, Alexander Murmann 
> wrote:
> >
> >> Is there an entry in the Coding Standard's Rules section that you feel
> is
> > irrelevant or incorrect? Please pick an example with a link to it so we
> can
> > discuss it.
> >
> > I haven't seen any rules in there that I think are irrelevant or
> incorrect.
> > My reasoning is a little different from that:
> > I think there are two different, competing goals we can optimize for:
> > a) The rules should be as complete as possible.
> > b) New contributors should be able to quickly catch up to what the coding
> > standard is for this project.
> >
> > I'd rather optimize for a) over b). To that end I'd prefer to leave
> > absolutely valid, but arguably obvious rules like MET02-J. Do not use
> > deprecated or obsolete classes or methods
> > <
> https://wiki.sei.cmu.edu/confluence/display/java/MET02-J.+Do+not+use+deprecated+or+obsolete+classes+or+methods
> >
> > or IDS00-J. Prevent SQL injection
> > <
> https://wiki.sei.cmu.edu/confluence/display/java/IDS00-J.+Prevent+SQL+injection
> >
> > off
> > and instead highlight the ways we are more opinionated then some other
> Java
> > projects (e.g. "Avoid introducing new statics", "limit method length",
> > "don't use abbreviated names")
> >
> > We could also go down the path of a compromise and highlight the rules we
> > care most about that will allow someone who is already a competent
> > developer to get up to speed quickly on our project and refer to the SEI
> > CERT rules as a recommended read for developers who are newer to Java.
> The
> > difference to your proposal would be that the Geode wiki page would
> > highlight what's most important and potentially surprising rather than be
> > an addendum to the already very large corpus of SEI CERT rules.
> >
> >
> > On Mon, Jun 24, 2019 at 3:15 PM Kirk Lund  wrote:
> >
> >> Java is complicated and Apache Geode is complicated, hence it's a large
> >> Coding Standard. *Effective Java* is similarly *large* if you compare
> it to
> >> the *Google Java Style Guide*.
> >>
> >> The "*rules*" are "*guidelines*" -- I think you're being too literal.
> Also,
> >> please remember what I said:
> >>
> >> I'm not proposing we rigidly and blindly follow this Coding Standard. We
> >>> can extend or even supersede portions of the adopted Coding Standard
> with
> >>> our own Addendum. The Coding Standard Addendum would exist on the
> Apache
> >>> Geode Wiki to define Geode-specific rules or recommendations. What I'd
> >> like
> >>> to see happen is for us to use the SEI CERT Coding Standard for Java
> as a
> >>> starting point for our own Coding Standard. The resulting Coding
> Standard
> >>> for Geode can be as static or as living and evolving as we wish.
> >>>
> >>
> >> Is there an entry in the Coding Standard's Rules section that you feel
> is
> >> irrelevant or incorrect? Please pick an example with a link to it so we
> can
> >> discuss it.
> >>
> >> PS: There aren't any other Coding Standards that I would personally
> write
> >> or recommend.
> >>
> >> On Mon, Jun 24, 2019 at 2:50 PM Alexander Murmann 
> >> wrote:
> >>
> >>> Hi Kirk,
> >>>
> >>> I think having a coding standard that goes beyond a formatting style
> >> guide
> >>> is a great idea. There are many interesting things in the SEI CERT
> >>> standard. However, it's also massive. I see 13 rules just about
> methods.
> >>> Yet some guidelines that would be most important to me like limiting
> >> method
> >>> length and number of parameters are missing.
> >>>
> >>> I wonder if we might be better off taking the rules we like from SEI
> >> CERT,
> >>> adding our own and aiming 

Re: [DISCUSS] RFC 0: Lightweight RFC Process

[Aaron Lindsey]

+1 looks good to me

- Aaron


On Tue, Jun 25, 2019 at 8:52 AM Jacob Barrett  wrote:

>
>
> > On Jun 24, 2019, at 3:42 PM, Dan Smith  wrote:
> >
> >>
> >> Just to make sure I got this 100% right, you mean the work related as
> part
> >> of the proposal would be under development, correct?
> >
> >
> > Yes! And I like your suggestion to just create a couple of buckets on the
> > wiki, rather than one for each state.
>
> Will moving the page around on the wiki result in dead links to the draft
> version?
>
> -Jake

Re: [PROPOSAL] Instrumenting Geode Code

[Aaron Lindsey]

+1

I like this approach compared to the previous proposals because it's
simpler (doesn't require a custom registry) and makes it more
straightforward to replace stats with Micrometer meters in the future.

- Aaron


On Fri, Jun 7, 2019 at 4:58 PM Dale Emery  wrote:

> Proposal for instrumenting Geode
>
>
> *Goals*
>
> - Allow routing measurements to statistics, Micrometer meters, or both as
> appropriate, to allow publishing each measurement to the appropriate places.
>
> - Minimize the intrusiveness of new and existing metrics instrumentation.
>
> - Minimize the amount of code that must know how to route measurements to
> stats and meters.
>
> - Maintain Geode performance.
>
> - Not preclude any options for deprecating and removing Geode’s statistics
> mechanism in the future. And take steps to make deprecating and removing
> the existing statistics mechanism easier.
>
>
> *Proposal*
>
> - Continue to use Geode’s existing style of instrumentation class (e.g.
> CachePerfStats) to instrument code.
>
> - Enhance the instrumentation classes to create and register meters when
> appropriate, and to route each measurement to the appropriate stats and
> meters.
>
> - When we want to route a given measurement to both a statistic and a
> meter, use the "legacy meter" wrapper mechanism (described below).
>
> - Incrementally improve the instrumentation classes to focus more on
> reporting domain events (e.g. regionAdded()) and less on reporting
> measurements (e.g. incRegions()).
>
>
> *Legacy Meter Wrappers*
>
> To route a given measurement to both a Micrometer meter and a Geode
> statistic, do the following in the instrumentation class's constructor:
>
> - Create and register a normal Micrometer meter in the cache's meter
> registry.
>
> - Wrap the registered meter in a custom "legacy meter" that reads and
> writes the stat, and also writes to the registered meter.
>
> In the instrumentation methods (e.g. endPut()):
>
> - Use the legacy meter to report measurements.
>
> I've attached two diagrams below to show how the wrapper mechanism works
> for "counter" style stats and meters. The first is a class diagram, showing
> how the parts relate in general. The second is a sequence diagram that
> shows in some detail how the parts interact to route a given measurement to
> both a meter and a stat.
>
> If you want even more details, Jake Barrett has created a nice "proof of
> concept" implementation:
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/LegacyMeterBuilder
>
>
>
> Please let us know if you have questions, comments, or concerns.
>
>
> Cheers,
> Dale
>
> —
> Dale Emery
> dem...@pivotal.io
>
>
>
>
>
>
>
>
>

Re: what is the best way to update a geode pull request

[Aaron Lindsey]

+1 to updating the PR template. I've noticed that few people actually
follow it and sometimes they just remove it altogether.
+1 to pushing PR changes as separate commits. This makes PR review easier.

Sometimes it's helpful to me as a reviewer for the initial PR to be split
into multiple commits as well. Also, I like Robert's suggestion of merge
instead of rebase when the PR is out-of-date with develop.

- Aaron


On Fri, May 31, 2019 at 2:50 PM Jacob Barrett  wrote:

>
>
> > On May 31, 2019, at 2:40 PM, Udo Kohlmeyer  wrote:
> >
> > If we are concerned about the single line that can break the product,
> then our testing has failed us on so many levels, that there is no hope.
>
> Sorry, I used a hyperbolic statement about looking for 1 line out of 1000.
> The point was “formatting” or “cleanup” style commits are better left
> separate because looking for the real change in that sea of change is hard.
>
> > But looking forward to see how long one can sustain the "factor ->
> commit -> make changes required to fulfill JIRA -> commit -> manual merge"…
>
> It’s only a problem if you are cleaning up lots a code. Not a bad problem
> to have and the future looks brighter each time.
>
> > Also, who reviews the refactor, because even that can introduce
> unintentional bugs... same effort as single commit. In single commit, if
> the refactor has made code cleaner, clearer and simpler, maybe 1 commit is
> easier to follow.
>
> I think there is a distinction between a refactor and cleanup. Consider
> the time we decide to reformat all the code, that was a cleanup. Now as we
> are going through the code and IJ reports every other line as a static
> analyzer warning, fixing that is a cleanup. All these cleanups have been
> reviews like any other PR. Th point being made was that they are done in a
> way that allows the reviewer to review the clean and the change
> independently.
>
> A refactor would would be a complete reorganization of code and should
> have the tests, reviews, etc. that go with it.
>
> Regardless, all are reviewed.
>
> -Jake
>
>

Propose GEODE-6544 fix for release 1.9.0

[Aaron Lindsey]

We'd like to propose including this fix into 1.9.0 as well:
https://github.com/apache/geode/pull/3362
https://jira.apache.org/jira/browse/GEODE-6544

This fixes a memory leak caused by a thread leak in Pulse. Every time a
user signs out, a thread is left running. The fix for GEODE-6544 stops that
thread when the user signs out.

- Aaron and Kirk

Re: Jira Permission Request

[Aaron Lindsey]

Sorry, I forgot to add that my username is aaronlindsey

On Wed, Apr 10, 2019 at 3:17 PM Aaron Lindsey  wrote:

> Hi,
>
> I would like to request permission to create and edit tickets and assign
> them to myself on the following JIRA:
> https://issues.apache.org/jira/
>
> Thanks,
> Aaron Lindsey
>


-- 
- Aaron

Jira Permission Request

[Aaron Lindsey]

Hi,

I would like to request permission to create and edit tickets and assign
them to myself on the following JIRA:
https://issues.apache.org/jira/

Thanks,
Aaron Lindsey

Permission request

[Aaron Lindsey]

Hi,

I would like to be able to edit the wiki here:
https://cwiki.apache.org/confluence/display/GEODE

My username is aaronlindsey.

Thanks,
Aaron Lindsey