Re: [VOTE] Apache Geode 1.15.0.RC1

[Bill Burcham]

+1

on macOS…

downloaded examples and verified they all run and pass

BUILD SUCCESSFUL in 18m 58s
267 actionable tasks: 267 executed

Downloaded Geode tarball and verified SHA on
https://dist.apache.org/repos/dist/dev/geode/1.15.0.RC1/apache-geode-1.15.0-src.tgz.sha256
matches checksum on
https://dist.apache.org/repos/dist/dev/geode/1.15.0.RC1/apache-geode-1.15.0-src.tgz

Verified product build: ./gradlew build -x test

BUILD SUCCESSFUL in 4m 57s
525 actionable tasks: 517 executed, 8 from cache

Verified this this distributed test passed:

○ → ./gradlew :geode-core:distributedTest --tests
org.apache.geode.distributed.internal.P2PMessagingConcurrencyDUnitTest

> Task :combineReports
All test reports at
/Users/bburcham/Downloads/geode-1-15/apache-geode-1.15.0-src/build/reports/combined

BUILD SUCCESSFUL in 1m 22s
49 actionable tasks: 2 executed, 47 up-to-date

Verified that changes in latest commit:

https://github.com/apache/geode/tree/rel/v1.15.0.RC1
https://github.com/apache/geode/commit/1869f2c06681bb73de727d2080d76c6215db9fb9

are represented in the downloaded source.

On Fri, Jun 17, 2022 at 10:35 AM Donal Evans 
wrote:

> +1
>
> Verified that performance across a variety of workloads is on par with
> previous releases.
> 
> From: Owen Nichols 
> Sent: Friday, June 17, 2022 9:22 AM
> To: dev@geode.apache.org 
> Subject: [VOTE] Apache Geode 1.15.0.RC1
>
> ⚠ External Email
>
> Hello Geode Dev Community,
>
> This is a release candidate for Apache Geode version 1.15.0.RC1.  Thank
> you to all
> community members for your contributions to this release over the past 490
> days!
>
> Please do a review and give your feedback, including the checks you
> performed.
>
> Voting deadline:
> 3PM PST Wed, June 22 2022.
>
> Please note that we are voting upon the source tag:
> rel/v1.15.0.RC1
>
> Release notes:
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FRelease%2BNotes%23ReleaseNotes-1.15.0data=05%7C01%7Cdoevans%40vmware.com%7Cf13795dd8e85426d373108da507da954%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63791079797735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=1Jh2qL5j5uLqpcT7UsH%2BAxSd9SAbTvtbJAloOvt95Nc%3Dreserved=0
>
> Source and binary distributions:
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache.org%2Frepos%2Fdist%2Fdev%2Fgeode%2F1.15.0.RC1%2Fdata=05%7C01%7Cdoevans%40vmware.com%7Cf13795dd8e85426d373108da507da954%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63791079797735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=hg5QXyPY9tGEXVHeMXOkjICIZn7p4sM%2BVx3yAFLUKYQ%3Dreserved=0
>
> Maven staging repo:
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Frepository.apache.org%2Fcontent%2Frepositories%2Forgapachegeode-1138data=05%7C01%7Cdoevans%40vmware.com%7Cf13795dd8e85426d373108da507da954%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63791079797735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=Dqp5pW5UFWg4P%2Fl0j1rnq1YjhGVyiuHuNdymAQuBDmU%3Dreserved=0
>
> GitHub:
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Ftree%2Frel%2Fv1.15.0.RC1data=05%7C01%7Cdoevans%40vmware.com%7Cf13795dd8e85426d373108da507da954%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63791079797735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=pA2U7tRLdfcQvJfE1Jkkcs5PPOj9EzcUDugPFmg1WPs%3Dreserved=0
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-examples%2Ftree%2Frel%2Fv1.15.0.RC1data=05%7C01%7Cdoevans%40vmware.com%7Cf13795dd8e85426d373108da507da954%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63791079797735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=A9jlDbACLA7dckkqgEI2yKb1zy7gVUMBP6S4%2FA959Io%3Dreserved=0
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Ftree%2Frel%2Fv1.15.0.RC1data=05%7C01%7Cdoevans%40vmware.com%7Cf13795dd8e85426d373108da507da954%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63791079797735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=ds5BxaLZ%2BdCJtzgdUdG%2FYZORZB2vPE2bplcGNgKwfqA%3Dreserved=0
>
> 

Re: [Discuss] New feature work approval state in Geode project?

[Bill Burcham]

We have an RFC process "to get to consensus faster and ultimately get to
execution faster". But I think in general folks tend to only do an RFC for
a "big" change. Bug tickets, and particularly feature tickets are left as a
gray area.

A google search for "most successful open source project" lists Apache
Cassandra at the top. When a bug is submitted to the Cassandra Jira it
starts in the TRIAGE NEEDED state. Ostensibly after triage, it moves to the
OPEN state which is described as:

| "the issue is open and ready for the assignee to start work on it."

I think introducing a starting state like TRIAGE NEEDED in the Geode Jira
system, and instituting a (gating) triage process would be a good way to
keep the Geode architecture cohesive and also to maximize the impact of our
contributor's efforts.

Perhaps that triage process would be run by code owners in the area(s)
affected by the ticket (bug or feature).

-Bill

On Fri, May 28, 2021 at 10:36 AM Mark Hanson  wrote:

> Hi All,
>
> There has been some discussion about adding a new state of approved to
> Geode Jira for features or something like it, to help prevent work being
> done that doesn’t make it into the project. What to people think?
>
> Thanks,
> Mark
>

Re: JDK 16 Support?

[Bill Burcham]

John, this doesn't speak to the general question of JDK version support.
But the particular stack trace you showed makes me wonder if the fix for
GEODE-9081 (landed on 3/30/21 on the develop branch) might get you a little
bit further. That commit 7ac9d7e4f0d04c99298067ca0611d9326e96d9cf
eliminated the reflective field access in favor of some simpler
down-casting.

On Wed, May 5, 2021 at 9:06 AM John Blum  wrote:

> Hi Anthony-
>
> Thank you for the quick reply.
>
> The Spring Data Team is currently looking ahead towards Java 16 when
> building and running Spring Data examples to get a sense for what works and
> what doesn't, now that Java 16 is GA along with anticipation for users with
> questions or problems.
>
> Spring Framework itself aligns and is based on LTS Java versions only,
> currently Java 8 with Spring Framework 5.  Spring Framework 6 will likely
> move the baseline to Java 11 or possibly even Java 17, we are not sure
> which yet.
>
> Just want to share our findings and give advanced notice.
>
> Thanks,
> John
>
> 
> From: Anthony Baker 
> Sent: Wednesday, May 5, 2021 8:14 AM
> To: u...@geode.apache.org 
> Cc: geode 
> Subject: Re: JDK 16 Support?
>
> Thanks for reporting this John.  The next LTS version of Java (17) is due
> later this year.  I think Geode needs to at least support every LTS version
> of Java and clearly we would need to fix errors like this. Do you see a
> need to support Java 16 now?
>
> Anthony
>
>
> On May 5, 2021, at 7:57 AM, John Blum  jb...@vmware.com>> wrote:
>
> What is the plan to support Java 16 for Apache Geode?  Timeframe?
>
> Running Apache Geode on a Java 16 Runtime produces errors like the
> following:
>
>
> - org.apache.geode.InternalGemFireException: unable to retrieve underlying
> byte buffer
> -  at org.apache.geode.internal.net
> .BufferPool.getPoolableBuffer(BufferPool.java:346)
> -  at org.apache.geode.internal.net
> .BufferPool.releaseBuffer(BufferPool.java:310)
> -  at org.apache.geode.internal.net
> .BufferPool.releaseSenderBuffer(BufferPool.java:213)
> -  at
> org.apache.geode.internal.tcp.MsgStreamer.release(MsgStreamer.java:100)
> -  at
> org.apache.geode.internal.tcp.MsgStreamer.writeMessage(MsgStreamer.java:256)
> -  at
> org.apache.geode.distributed.internal.direct.DirectChannel.sendToMany(DirectChannel.java:306)
> -  at
> org.apache.geode.distributed.internal.direct.DirectChannel.sendToOne(DirectChannel.java:182)
> -  at
> org.apache.geode.distributed.internal.direct.DirectChannel.send(DirectChannel.java:511)
> -  at
> org.apache.geode.distributed.internal.DistributionImpl.directChannelSend(DistributionImpl.java:346)
> -  at
> org.apache.geode.distributed.internal.DistributionImpl.send(DistributionImpl.java:291)
> -  at
> org.apache.geode.distributed.internal.ClusterDistributionManager.sendViaMembershipManager(ClusterDistributionManager.java:2050)
> -  at
> org.apache.geode.distributed.internal.ClusterDistributionManager.sendOutgoing(ClusterDistributionManager.java:1978)
> -  at
> org.apache.geode.distributed.internal.ClusterDistributionManager.sendMessage(ClusterDistributionManager.java:2015)
> -  at
> org.apache.geode.distributed.internal.ClusterDistributionManager.putOutgoing(ClusterDistributionManager.java:1083)
> -  at
> org.apache.geode.distributed.internal.StartupMessage.process(StartupMessage.java:279)
> -  at
> org.apache.geode.distributed.internal.DistributionMessage.scheduleAction(DistributionMessage.java:376)
> -  at
> org.apache.geode.distributed.internal.DistributionMessage$1.run(DistributionMessage.java:441)
> -  at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
> -  at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
> -  at
> org.apache.geode.distributed.internal.ClusterOperationExecutors.runUntilShutdown(ClusterOperationExecutors.java:441)
> -  at
> org.apache.geode.distributed.internal.ClusterOperationExecutors.doWaitingThread(ClusterOperationExecutors.java:410)
> -  at
> org.apache.geode.logging.internal.executors.LoggingThreadFactory.lambda$newThread$0(LoggingThreadFactory.java:119)
> -  at java.base/java.lang.Thread.run(Thread.java:831)
> - Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make
> public java.lang.Object java.nio.DirectByteBuffer.attachment() accessible:
> module java.base does not "opens java.nio" to unnamed module @40f9161a
> -  at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
> -  at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
> -  at
> java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
> -  at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
> -  at org.apache.geode.internal.net
> .BufferPool.getPoolableBuffer(BufferPool.java:343)
> -  ... 22 common frames omitted
> - 2021-04-30 14:57:13,638  INFO 

Re: [VOTE] Requiring final keyword on every parameter and local variable

[Bill Burcham]

+1

I am 100% for putting final everywhere it is possible to put it. Call this
the "hard final" position.

What I've been advocating for, intermittently, in PRs (for example, in PR
#6310 ) is
more of a "soft final" position which I can formulate as:

The final keyword is valuable everywhere. When we encounter it in code, we
assume that the author put it there intentionally to communicate that the
variable will not change. Since the code compiled before our change, we
know that the variable does not change (the compiler has proven it!) When
modifying code, we do not remove the final keyword unless we have to (to
get the code to compile).

So that's the "soft final" position. But I'm all for Kirk's "hard final" if
others agree. I just don't see any downside—only upside on this one.

The final keyword is a vehicle, like types, for programmers to communicate
their intentions. In both cases, the compiler can provide valuable
validation. There is, unfortunately, some visual noise entailed (lots of
occurrences of "final" in the code). But this makes non-final (mutable)
variables much more apparent. Since they are (or should be) in the
minority, this is an aid to maintainers. This becomes apparent in longer,
more complex methods.

On Wed, Apr 14, 2021 at 12:56 PM Kirk Lund  wrote:

> Our coding standard and our Design Decisions does NOT require using final
> on parameters and local variables.
>
> Please do NOT request that I add or keep final on parameters or local
> variables unless the community votes and decides that every parameter and
> local variable should require the final keyword. I coded this way in the
> past and I found that it resulted in noisy code with no benefit. We can
> argue about using this keyword all you want but the fact is I'm against it,
> and I will not embrace it unless this community decides that we need to use
> it.
>
> Using final on instance or class fields does have a concurrency benefit and
> I support that only.
>
> If you want to add final to every single parameter and local var in the
> Geode codebase, then now is your chance to vote on it. Please vote.
>
> Thanks,
> Kirk
>

Re: Question regarding VersionRequest/VersionResponse messages

[Bill Burcham]

Jakov, VersionRequest/VersionResponse is understood by the 8.1.x/GFE_81
product versions.

I don't know what Geode's stance is on supporting old locator versions. I
see the recent:

GEODE-8837 - Establishes GFE_81 as the oldest supported client.

Does that imply that GFE_81 is now the oldest supported version for
locators in rolling upgrade too? Seems reasonable to me. What do others
think?




On Tue, Mar 9, 2021 at 12:37 AM Jakov Varenina 
wrote:

> Hi community,
>
> I have one question regarding VersionRequest/VersionResponse messages.
>
> Before member sends actual message, it has to first determine the remote
> member version. This is done by exchanging
> /VersionRequest///VersionResponse/ messages using function
> /getServerVersion() /from class /TcpClient.java/. There is part of code
> in /getServerVersion()/ for which I'm unsure which case is actually
> covering:
>
>
> https://github.com/apache/geode/blob/854456c81ca7b9545eba252b6fa075318bb33af8/geode-tcp-server/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java#L289
>
>
> try {
>final Object readObject =objectDeserializer.readObject(versionedIn); if
> (!(readObjectinstanceof VersionResponse)) {
>  throw new IllegalThreadStateException(
>  "Server version response invalid: This could be the result of
> trying to
> connect a non-SSL-enabled client to an SSL-enabled locator."); }
>
>final VersionResponse response = (VersionResponse) readObject;
> serverVersion = response.getVersionOrdinal(); serverVersions.put(address,
> serverVersion); return serverVersion; }catch (EOFException ex) {
>// old locators will not recognize the version request and will close
> the connection }
> ...
> return KnownVersion.OLDEST.ordinal();
>
> The case is when /readObject()/ try to read /VersionResponse/ and then
> throws /EOFException/. As you can see, there is comment in catch
> statement explaining the case, but I'm not sure that I understand it
> correctly. What I assume is that member with old version (less or equal
> to /KnownVersion.OLDEST/) of Geode does not support /VersionRequest/
> message, and will close connection if receive such message. This will
> result with /EOFException/ on remote member that sent /VersionRequest/.
> Is this correct understanding? Is this case still valid with the latest
> non-deprecated version set to /GFE_81/?
>
> The reason why I'm asking this is because in some cases in kuberentes
> cluster, it is possible to get /EOFException/ when remote member is not
> yet accessible. In this case member will still try to send message (e.g.
> /RemoteLocatorJoinRequest/) thinking that it is old member on the other
> side, and that will then result with /ClassCastException/ when reading
> response (e.g. /RemoteLocatorJoinResponse/).
>
> BRs,
>
> Jakov
>
>

Re: geode docker question

[Bill Burcham]

Great (and timely) question Barry! And great answers from Mark and John.

The question is timely because we recently addressed some similar needs in
the development of the new Dockerized acceptance tests for the new TLS SNI
features. I've taken a stab at a tutorial that leverages some of those
ideas to hopefully answer Barry's question here:

https://cwiki.apache.org/confluence/display/GEODE/Using+the+Official+Geode+Docker+Image

It's not perfect (at all) but hopefully it'll give you a few techniques you
can start using!

On Tue, Jul 7, 2020 at 3:21 PM John Blum  wrote:

> Hi Barry-
>
> Have a look at this...
>
>
> https://docs.spring.io/spring-boot-data-geode-build/1.3.x/reference/html5/#geode-docker
>
> I recently put this together as part of the SBDG 1.3 GA release.  It
> contains references to other pertinent documentation as well.
>
> There aren't any pre-canned Docker Images with a Locator/Locators or
> Server/Servers running, unfortunately.
>
> However, I intend to tackle that concern as part of the STDG project (
> https://github.com/spring-projects/spring-test-data-geode).  See Issue
> #19 (https://github.com/spring-projects/spring-test-data-geode/issues/19)
> by myself and David Turanski.
>
> Some early experimentation has already taken place.
>
> Regards,
> John
>
>
>
> 
> From: Barry Barrios 
> Sent: Tuesday, July 7, 2020 10:26 AM
> To: dev@geode.apache.org 
> Subject: geode docker question
>
> Do you have Apache Geode examples using docker? Is there a way to run a
> docker container that automatically creates locator and server by
> specifying some parameters without typing them in the gfsh shell prompt?
> Also, same for deploying jars, is there a way to automatically deploy jars
> when running docker? I was browsing through your apache geode examples
> github repo to see if there were examples but didn't find what I was
> looking for.
>
> Best,
> Barry
>

Back-Port GEODE-8240 to 1.12, 1.13

[Bill Burcham]

I'd like permission to back-port the fix for rolling upgrade bug GEODE-8240
to support/1.12 and support/1.13

-Bill

Re: Problem in rolling upgrade since 1.12

[Bill Burcham]

I made us a ticket for the SerializationException Alberto Gomez reported
https://issues.apache.org/jira/browse/GEODE-8251

On Thu, Jun 11, 2020 at 6:47 AM Alberto Gomez 
wrote:

> Thanks for the info, Bill.
>
> I have found another issue in rolling upgrade since 1.12.
>
> I have observed that when there are custom jars previously deployed, the
> locator is not able be started in the new version and the following
> exception is thrown:
>
> Exception in thread "main" org.apache.geode.SerializationException: Could
> not create an instance of
> org.apache.geode.management.internal.configuration.domain.Configuration .
>
> I have pushed another commit in the draft I sent before containing the new
> test case.
>
> /Alberto G.
> 
> From: Bill Burcham 
> Sent: Thursday, June 11, 2020 1:53 AM
> To: dev@geode.apache.org 
> Subject: Re: Problem in rolling upgrade since 1.12
>
> Ernie made us a ticket for this issue:
> https://issues.apache.org/jira/browse/GEODE-8240
>
> On Mon, Jun 8, 2020 at 12:59 PM Alberto Gomez 
> wrote:
>
> > Hi Ernie,
> >
> > I have seen this problem in the support/1.13 branch and also on develop.
> >
> > Interestingly, the patch I sent is applied seamlessly in my local repo
> set
> > to the develop branch.
> >
> > The patch modifies the
> > RollingUpgradeRollServersOnPartitionedRegion_dataserializable test case
> by
> > running "list members" on an upgraded system is
> > RollingUpgradeRollServersOnPartitionedRegion_dataserializable. I run it
> > manually with the following command:
> >
> > ./gradlew geode-core:upgradeTest
> > --tests=RollingUpgradeRollServersOnPartitionedRegion_dataserializable
> >
> > I see it failing when upgrading from 1.12.
> >
> > I created a draft PR where you can see also the changes in the test case
> > that manifest the problem.
> >
> > See: https://github.com/apache/geode/pull/5224
> >
> >
> > Please, let me know if you need any more information.
> >
> > BR,
> >
> > Alberto
> > 
> > From: Ernie Burghardt 
> > Sent: Monday, June 8, 2020 9:04 PM
> > To: dev@geode.apache.org 
> > Subject: Re: Problem in rolling upgrade since 1.12
> >
> > Hi Alberto,
> >
> > I’m looking at this, came across a couple blockers…
> > Do you have branch that exhibits this problem? Draft PR maybe?
> > I tried to apply you patch to latest develop, but the patch doesn’t pass
> > git apply’s check….
> > Also these tests pass on develop, would you be able to check against the
> > latest and update the diff?
> > I’m very interested in reproducing the issue you have observed.
> >
> > Thanks,
> > Ernie
> >
> > From: Alberto Gomez 
> > Reply-To: "dev@geode.apache.org" 
> > Date: Monday, June 8, 2020 at 12:31 AM
> > To: "dev@geode.apache.org" 
> > Subject: Re: Problem in rolling upgrade since 1.12
> >
> > Hi,
> >
> > I attach a diff for the modified test case in case you would like to use
> > it to check the problem I mentioned.
> >
> > BR,
> >
> > Alberto
> > 
> > From: Alberto Gomez 
> > Sent: Saturday, June 6, 2020 4:06 PM
> > To: dev@geode.apache.org 
> > Subject: Problem in rolling upgrade since 1.12
> >
> > Hi,
> >
> > I have observed that since version 1.12 rolling upgrades to future
> > versions leave the first upgraded locator "as if" it was still on version
> > 1.12.
> >
> > This is the output from "list members" before starting the upgrade from
> > version 1.12:
> >
> > Name | Id
> >  | ---
> > vm2  | 192.168.0.37(vm2:29367:locator):41001 [Coordinator]
> > vm0  | 192.168.0.37(vm0:29260):41002
> > vm1  | 192.168.0.37(vm1:29296):41003
> >
> >
> > And this is the output from "list members" after upgrading the first
> > locator from 1.12 to 1.13/1.14:
> >
> > Name | Id
> >  |
> >
> 
> > vm2  | 192.168.0.37(vm2:1453:locator):41001(version:GEODE 1.12.0)
> > [Coordinator]
> > vm0  | 192.168.0.37(vm0:810):41002(version:GEODE 1.12.0)
> > vm1  | 192.168.0.37(vm1:849):41003(version:GEODE 1.12.0)
> >
> >
> > Finally this is the output in gfsh once the rolling upgrade has been
> > completed (locators and servers upgraded):
> >

Re: Problem in rolling upgrade since 1.12

[Bill Burcham]

Ernie made us a ticket for this issue:
https://issues.apache.org/jira/browse/GEODE-8240

On Mon, Jun 8, 2020 at 12:59 PM Alberto Gomez 
wrote:

> Hi Ernie,
>
> I have seen this problem in the support/1.13 branch and also on develop.
>
> Interestingly, the patch I sent is applied seamlessly in my local repo set
> to the develop branch.
>
> The patch modifies the
> RollingUpgradeRollServersOnPartitionedRegion_dataserializable test case by
> running "list members" on an upgraded system is
> RollingUpgradeRollServersOnPartitionedRegion_dataserializable. I run it
> manually with the following command:
>
> ./gradlew geode-core:upgradeTest
> --tests=RollingUpgradeRollServersOnPartitionedRegion_dataserializable
>
> I see it failing when upgrading from 1.12.
>
> I created a draft PR where you can see also the changes in the test case
> that manifest the problem.
>
> See: https://github.com/apache/geode/pull/5224
>
>
> Please, let me know if you need any more information.
>
> BR,
>
> Alberto
> 
> From: Ernie Burghardt 
> Sent: Monday, June 8, 2020 9:04 PM
> To: dev@geode.apache.org 
> Subject: Re: Problem in rolling upgrade since 1.12
>
> Hi Alberto,
>
> I’m looking at this, came across a couple blockers…
> Do you have branch that exhibits this problem? Draft PR maybe?
> I tried to apply you patch to latest develop, but the patch doesn’t pass
> git apply’s check….
> Also these tests pass on develop, would you be able to check against the
> latest and update the diff?
> I’m very interested in reproducing the issue you have observed.
>
> Thanks,
> Ernie
>
> From: Alberto Gomez 
> Reply-To: "dev@geode.apache.org" 
> Date: Monday, June 8, 2020 at 12:31 AM
> To: "dev@geode.apache.org" 
> Subject: Re: Problem in rolling upgrade since 1.12
>
> Hi,
>
> I attach a diff for the modified test case in case you would like to use
> it to check the problem I mentioned.
>
> BR,
>
> Alberto
> 
> From: Alberto Gomez 
> Sent: Saturday, June 6, 2020 4:06 PM
> To: dev@geode.apache.org 
> Subject: Problem in rolling upgrade since 1.12
>
> Hi,
>
> I have observed that since version 1.12 rolling upgrades to future
> versions leave the first upgraded locator "as if" it was still on version
> 1.12.
>
> This is the output from "list members" before starting the upgrade from
> version 1.12:
>
> Name | Id
>  | ---
> vm2  | 192.168.0.37(vm2:29367:locator):41001 [Coordinator]
> vm0  | 192.168.0.37(vm0:29260):41002
> vm1  | 192.168.0.37(vm1:29296):41003
>
>
> And this is the output from "list members" after upgrading the first
> locator from 1.12 to 1.13/1.14:
>
> Name | Id
>  |
> 
> vm2  | 192.168.0.37(vm2:1453:locator):41001(version:GEODE 1.12.0)
> [Coordinator]
> vm0  | 192.168.0.37(vm0:810):41002(version:GEODE 1.12.0)
> vm1  | 192.168.0.37(vm1:849):41003(version:GEODE 1.12.0)
>
>
> Finally this is the output in gfsh once the rolling upgrade has been
> completed (locators and servers upgraded):
>
> Name | Id
>  |
> 
> vm2  | 192.168.0.37(vm2:1453:locator):41001(version:GEODE 1.12.0)
> [Coordinator]
> vm0  | 192.168.0.37(vm0:2457):41002
> vm1  | 192.168.0.37(vm1:2576):41003
>
> I verified this by running manual tests and also by running the following
> upgrade test (had to stop it in the middle to connect via gfsh and get the
> gfsh outputs):
>
> RollingUpgradeRollServersOnPartitionedRegion_dataserializable.testRollServersOnPartitionedRegion_dataserializable
>
> After the rolling upgrade, the shutdown command fails with the following
> error:
> Member 192.168.0.37(vm2:1453:locator):41001 could not be found.
> Please verify the member name or ID and try again.
>
> The only way I have found to come out of the situation is by restarting
> the locator.
> Once restarted again, the output of gfsh shows that all members are
> upgraded to the new version, i.e. the locator does not show anymore that it
> is on version GEODE 1.12.0.
>
> Anybody has any clue why this is happening?
>
> Thanks in advance,
>
> /Alberto G.
>

Re: geode-assembly fails with SIGABRT

[Bill Burcham]

Kirk: what happens when you run ./gradlew :geode-assembly:docker in your
local dev environment? Does it break the same way? If not, then I surmise
this may be due to a recent configuration change to the AWS AMI used for CI.

This bug report on Docker credentials helper has some stack traces that
look very similar to the one you provided above:

https://github.com/docker/docker-credential-helpers/issues/103

There are a number of people weighing in on that ticket with fixes.

On Sat, Jun 6, 2020 at 5:50 AM Kirk Lund  wrote:

> The docker image (or something related) seems to be broken for
> geode-assembly. Anyone working on fixing this or know how to fix it?
>
> https://concourse.apachegeode-ci.info/builds/163375
>
> Something in the docker setup SIGABRTs...
>
> > Task :geode-assembly:docker
> free(): invalid pointer
> SIGABRT: abort
> PC=0x7fdda9240e97 m=0 sigcode=18446744073709551610
> signal arrived during cgo execution
>
> goroutine 1 [syscall, locked to thread]:
> runtime.cgocall(0x4afd50, 0xc420051cc0, 0xc420051ce8)
> /usr/lib/go-1.8/src/runtime/cgocall.go:131 +0xe2 fp=0xc420051c90
> sp=0xc420051c50
>
> github.com/docker/docker-credential-helpers/secretservice._Cfunc_free(0x2670270)
>
> github.com/docker/docker-credential-helpers/secretservice/_obj/_cgo_gotypes.go:111
> +0x41
> 
> fp=0xc420051cc0 sp=0xc420051c90
>
> github.com/docker/docker-credential-helpers/secretservice.Secretservice.List.func5(0x2670270)
>
> /build/golang-github-docker-docker-credential-helpers-cMhSy1/golang-github-docker-docker-credential-helpers-0.5.0/obj-x86_64-linux-gnu/src/
> 
>
> github.com/docker/docker-credential-helpers/secretservice/secretservice_linux.go:96
> +0x60
> 
> fp=0xc420051cf8 sp=0xc420051cc0
>
> github.com/docker/docker-credential-helpers/secretservice.Secretservice.List(0x0
> ,
> 0x756060, 0xc420012360)
>
> /build/golang-github-docker-docker-credential-helpers-cMhSy1/golang-github-docker-docker-credential-helpers-0.5.0/obj-x86_64-linux-gnu/src/
>
> github.com/docker/docker-credential-helpers/secretservice/secretservice_linux.go:97
> +0x217
> 
> fp=0xc420051da0 sp=0xc420051cf8
>
> github.com/docker/docker-credential-helpers/secretservice.(*Secretservice).List(0x77e548
> ,
> 0xc420051e88, 0x410022, 0xc4200122b0)
> :4 +0x46 fp=0xc420051de0 sp=0xc420051da0
> github.com/docker/docker-credential-helpers/credentials.List(0x756ba0,
> 0x77e548, 0x7560e0, 0xc42000e018, 0x0, 0x10)
>
> /build/golang-github-docker-docker-credential-helpers-cMhSy1/golang-github-docker-docker-credential-helpers-0.5.0/obj-x86_64-linux-gnu/src/
> github.com/docker/docker-credential-helpers/credentials/credentials.go:145
> +0x3e
> 
> fp=0xc420051e68 sp=0xc420051de0
>
> github.com/docker/docker-credential-helpers/credentials.HandleCommand(0x756ba0
> ,
> 0x77e548, 0x7ffea9775e1d, 0x4, 0x7560a0, 0xc42000e010, 0x7560e0,
> 0xc42000e018, 0x40e398, 0x4d35c0)
>
> /build/golang-github-docker-docker-credential-helpers-cMhSy1/golang-github-docker-docker-credential-helpers-0.5.0/obj-x86_64-linux-gnu/src/
> github.com/docker/docker-credential-helpers/credentials/credentials.go:60
> +0x16d
> 
> fp=0xc420051ed8 sp=0xc420051e68
> github.com/docker/docker-credential-helpers/credentials.Serve(0x756ba0,
> 0x77e548)
>
> /build/golang-github-docker-docker-credential-helpers-cMhSy1/golang-github-docker-docker-credential-helpers-0.5.0/obj-x86_64-linux-gnu/src/
> github.com/docker/docker-credential-helpers/credentials/credentials.go:41
> +0x1cb
> 
> fp=0xc420051f58 sp=0xc420051ed8
> main.main()
>
> /build/golang-github-docker-docker-credential-helpers-cMhSy1/golang-github-docker-docker-credential-helpers-0.5.0/secretservice/cmd/main_linux.go:9
> +0x4f fp=0xc420051f88 sp=0xc420051f58
> runtime.main()
> /usr/lib/go-1.8/src/runtime/proc.go:185 +0x20a fp=0xc420051fe0
> sp=0xc420051f88
> runtime.goexit()
> /usr/lib/go-1.8/src/runtime/asm_amd64.s:2197 +0x1 fp=0xc420051fe8
> sp=0xc420051fe0
>
> goroutine 17 [syscall, locked to thread]:
> runtime.goexit()
> /usr/lib/go-1.8/src/runtime/asm_amd64.s:2197 +0x1
>
> rax0x0
> rbx0x7ffea9775420
> rcx0x7fdda9240e97
> rdx0x0
> rdi0x2
> rsi0x7ffea97751b0
> rbp0x7ffea9775520
> rsp0x7ffea97751b0
> r8 0x0
> r9 

Re: [PROPOASAL] backport GEODE-8144

[Bill Burcham]

+1

On Thu, May 28, 2020 at 11:49 AM Ernie Burghardt 
wrote:

> +1
>
> On 5/27/20, 1:35 PM, "Bruce Schuchardt"  wrote:
>
> This ticket has two PRs.  One passed all normal CI runs but then we
> hit a faulty test that failed on a Windows machine.  There’s a new PR that
> fixes that test & has been merged.
>
> The PRs fixe endpoint verification problems in servers and locators.
> Without this fix it’s not possible to boot a locator/server with endpoint
> verification enabled.
>
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5164data=02%7C01%7Cburghardte%40vmware.com%7Ca40e55b9f1e54af3b44c08d8027d8349%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637262085382722407sdata=GBAPvBI8qMtsCObX6GYrowVlV9%2FmWq%2BV0yQLLKfpA%2BQ%3Dreserved=0
> fixes the failing test
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F5131data=02%7C01%7Cburghardte%40vmware.com%7Ca40e55b9f1e54af3b44c08d8027d8349%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637262085382722407sdata=X1TE4qizOKk400%2BR%2FUaOA%2FDm6FeVrJPAPMn%2FIl1ooqo%3Dreserved=0
> is the original PR
>
> both have been merged to develop
>
>
>
>

Re: RFC - Client side configuration for a SNI proxy

[Bill Burcham]

Sure Udo. Dan is exploring some ideas now.

All: let's consider this round of RFC closed (since it officially closed
yesterday) and we'll re-open it with a new deadline when we have those
changes in hand.

On Thu, Mar 12, 2020 at 8:26 AM Udo Kohlmeyer  wrote:

> Hi there Jake,
>
> Another twist to the story, but with a working (if unpolished ;) )
> prototype.
>
> It covers all bases of:
>
>   * Type safety
>   * Extensibility
>   * Simple API design
>   * API clarity
>
> It takes the best of all approaches.
>
> I like it!!
>
> +1 to this implementation.
>
> -1 to Bill's approach.
>
> @Bill, could we amend the RFC to favor this approach?
>
> --Udo
>
> On 3/11/20 11:30 PM, Jacob Barrett wrote:
> > -1
> >
> > I hate to do this but I really feel like we went backwards on this
> change.
> >
> >> On Mar 11, 2020, at 3:03 PM, Bill Burcham 
> wrote:
> >> PoolFactory {
> >>   setProxyAddress(String host, int port);
> >> }
> >>
> >> ClientCacheFactory {
> >>   setPoolProxyAddress(String host, int port);
> >> }
> > It gives the user no information about what type of proxy is in use. So
> documentation must specify that it is only SNI. It assumes that SNI and any
> other proxy would have a host and port only.
> >
> > Also consider if we used SRV records to discover the proxy, would I need
> to set hostname to null or the service name, what about port since it comes
> from the SRV record. Would I set port to 0?.
> >
> > What about default ports for well know proxy services like SOCKS? Again,
> 0?
> >
> >
> >> This should address Johns desire for an API that doesn't grow with each
> new
> >> proxy type. And it should address Udo's desire for extensibility.
> > I really struggle to see how this satisfies future extensibility given
> the parameter lock in. Providing a class allows the proxy to define all its
> options. Consider SniProxyConfig that takes no parameters, it could use the
> SRV records for “_sniproxy._tcp” queried on the current domain to discover
> the proxy. Or if given just a hostname could SRV query
> “_sniproxy._tcp.hostname” and fall back to A “hostname” on the default port.
> >
> >> This API sets the stage for a potential follow-on RFC to support other
> >> proxy types. One way that might work would be addition of
> >> setProxyType(String type)/setPoolProxyType(String type) methods. That
> RFC
> >> might reserve the "SNI" proxy type. That RFC might specify an SPI (per
> >> Jacob's proposal) that would let Geode users register their own proxy
> types
> >> e.g. SOCKS.
> > Adding disconnected method to set the proxy type by string doesn’t sit
> well for me. What strings are valid? When we add this method what does it
> mean if we don’t set it, its always SNI?
> >
> > How would you deprecate a proxy? If the proxy configuration is a class I
> can discover it in my IDE and deprecate the class when not supported
> anymore. Even the original Enum idea could have deprecated enum values.
> >
> > I would vote for the original RFC over this given that it weakens the
> type safety of the API.
> >
> > To illustrate my point on the SPI see
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi
> <
> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/proxy-spi>.
> Its a quick hack and some things might not be perfect, like discovering the
> factories, but it should get the idea across.
> >
> >
> > -Jake
> >
> >
>

Re: RFC - Client side configuration for a SNI proxy

[Bill Burcham]

rs will not know WHAT options are
> >>> available.. We could always have a static builder for our supported
> >>> options.
> >>>
> >>> --Udo
> >>>
> >>>> On 3/10/20 10:07 AM, Dan Smith wrote:
> >>>> Ok, how about this?
> >>>>
> >>>> setProxy(SniProxyConfiguration config)
> >>>>
> >>>> interface SniProxyConfiguration extends ProxyConfiguration {
> >>>>static SniProxyConfiguration create(String host, int port);
> >>>>String getHost();
> >>>>int getPort()
> >>>> }
> >>>>
> >>>> The main difference here from John's proposal being that setProxy
> >> takes a
> >>>> SniProxyConfiguration, rather than the base ProxyConfiguration. We can
> >>>> overload setProxy later if needed.
> >>>>
> >>>> The reason I'm not as keen on setProxy(ProxyConfiguration) is that it
> >> is
> >>>> hard for the user to discover the different types of
> ProxyConfiguration
> >>>> subclasses and know what is supported.
> >>>>
> >>>> -Dan
> >>>>
> >>>>> On Mon, Mar 9, 2020 at 11:23 PM John Blum  wrote:
> >>>>>
> >>>>> Corrections ( :-P ), my apologies...
> >>>>>
> >>>>> Iterable proxyConfigurations = ...;
> >>>>>
> >>>>> StreamSupport.stream(proxyConfiguration*s*.*spliterator*(), false)
> >>>>> .filter(config -> config.getType.isSecure()) *// This could be
> >>>>> improved; see below...*
> >>>>> .*forEach*(config -> // do something with *each* secure proxy
> >>>>> *config*).
> >>>>>
> >>>>> Although, I am sure you got the point, ;-)
> >>>>>
> >>>>> With improvement:
> >>>>>
> >>>>> interface ProxyConfiguration {
> >>>>>
> >>>>>   ProxyType getType();
> >>>>>
> >>>>>   // null-safe!
> >>>>>   default boolean isSecure() {
> >>>>> ProxyType type = getType();
> >>>>> return type != null && type.isSecure();
> >>>>>   }
> >>>>> }
> >>>>>
> >>>>> Then:
> >>>>>
> >>>>> StreamSupport.stream(..)
> >>>>> *.filter(ProxyConfiguration::isSecure)*
> >>>>> .forEach(...);
> >>>>>
> >>>>>
> >>>>> Again, completely contrived.
> >>>>>
> >>>>> Cheers!
> >>>>> -j
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Mon, Mar 9, 2020 at 11:14 PM John Blum  wrote:
> >>>>>>
> >>>>>> Yes, it's redundant (i.e. Enum + class type).
> >>>>>>
> >>>>>> However, having an Enum in addition to a specific type (1 reason I
> >>>>>> defaulted the getType() method) can still be useful, such as in a
> >>> switch
> >>>>>> statement for example.  Enums are, well, easier to enumerate (useful
> >> in
> >>>>>> Streams with filters).  Maybe you are going to classify certain
> >> Proxy's
> >>>>>> by Enum type, for example:
> >>>>>>
> >>>>>> enum ProxyType {
> >>>>>>
> >>>>>> ONE,
> >>>>>> TWO,
> >>>>>> THREE;
> >>>>>>
> >>>>>> // Contrived example
> >>>>>> public boolean isSecure() {
> >>>>>> return Arrays.asList(ONE, THREE).contains(this);
> >>>>>> }
> >>>>>> }
> >>>>>>
> >>>>>> Then:
> >>>>>>
> >>>>>> Iterable proxyConfigurations = ...;
> >>>>>>
> >>>>>> StreamSupport.stream(proxyConfiguration.spilterator(), false)
> >>>>>> .filter(config -> config.getType.isSecure())
> >>>>>> .ifPresent(config -> // do something with secure proxy).
> >>>>>>
> >>>>>>
> >>>>>> Food for thought.
> >>>>&

Re: RFC - Client side configuration for a SNI proxy

[Bill Burcham]

What I like about John's full-fledged-class-per-proxy-kind is that
everything that can potentially vary with proxy kind is all together in a
single object.

That being said, John, in your SniProxyConfiguration, it seems to me that
the class itself (SniProxyConfiguration) could easily stand for the proxy
"type". If that's true then we could view ProxyType as redundant and simply
eliminate it right?

On Mon, Mar 9, 2020 at 2:41 PM Jacob Barrett  wrote:

> +1 to Anthony and John. See similar comments in the RFC.
>
> > On Mar 9, 2020, at 12:08 PM, Anthony Baker  wrote:
> >
> > I’m not suggesting encoding the the proxy type in the URI.  Just
> wondering if we can support stronger typing than String for defining
> host/port/url configuration.  As John notes, later in the thread, perhaps
> using a configuration interface may help.
> >
> > Anthony
> >
> >
> >> On Mar 9, 2020, at 11:11 AM, Bill Burcham 
> wrote:
> >>
> >> Anthony and Jacob, I can see how the proposed ProxyType parameter could
> fit
> >> into the scheme part of a a URI. However, the problem that introduces is
> >> that we would then have to pick (named) URL schemes to support. But URL
> >> schemes are standardized and it's not obvious which of the standard ones
> >> might apply here.
> >>
> >> If we couldn't adopt a standard scheme, we'd have to make one up. At
> that
> >> point I question the value of putting the (made-up) scheme into a URI
> >> string.
> >>
> >> For this reason, I am a fan of the ProxyType parameter over a made-up
> URL
> >> scheme.
> >>
> >> That leaves open Anthony's other idea: eliminating the ProxyType
> parameter
> >> in favor of a separate method to set each kind of proxy. In the current
> >> RFC, that's just one, e.g. setPoolProxyWithSNI. I guess that comes down
> to:
> >> what's the likelihood of us supporting other proxy types soon, and then
> >> what's the value of having a single method (and multiple enums) versus
> >> multiple methods (and no enum). If we thought the proxyAddress parameter
> >> would carry different information across proxy types that might tilt us
> >> toward the separate methods. The two on the table, however, (SNI,
> SOCKS5)
> >> both have identical proxyAddress information.
> >>
> >> On Mon, Mar 9, 2020 at 10:54 AM Bill Burcham 
> wrote:
> >>
> >>> By popular demand we are extending the RFC review period. I know Udo
> asked
> >>> for Friday (and Joris +1'd it), but since this is a small RFC, we'd
> like to
> >>> try to close it by Wednesday, March 11, ok?
> >>>
> >>> On Mon, Mar 9, 2020 at 10:39 AM Jacob Barrett 
> wrote:
> >>>
> >>>> I raised similar concerns as a comment in the RFC.
> >>>>
> >>>>> On Mar 9, 2020, at 10:29 AM, Anthony Baker 
> wrote:
> >>>>>
> >>>>> Given this new API:
> >>>>>
> >>>>>  setPoolProxy(ProxyType type, String proxyAddress)
> >>>>>
> >>>>> The ProxyType enum seems to be a look ahead at supporting other kinds
> >>>> of proxies.  What is your thinking about using the enum vs specific
> named
> >>>> API’s (e.g. setPoolProxyWithSNI).
> >>>>>
> >>>>> Currently the definition of the proxyAddress seems to be dependent of
> >>>> the proxy type.  Did you consider stronger typing using an URI
> parameter
> >>>> type?
> >>>>>
> >>>>> Anthony
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Mar 6, 2020, at 11:04 AM, Bill Burcham 
> >>>> wrote:
> >>>>>>
> >>>>>> Please review the RFC for *Client side configuration for a SNI
> proxy*:
> >>>>>>
> >>>>>>
> >>>>
> https://cwiki.apache.org/confluence/display/GEODE/Client+side+configuration+for+a+SNI+proxy
> >>>>>>
> >>>>>> Please comment by Monday, March 9, 2020.
> >>>>>>
> >>>>>> Regards,
> >>>>>> Bill and Ernie
> >>>>>
> >>>>
> >>>
> >
>
>

Re: RFC - Client side configuration for a SNI proxy

[Bill Burcham]

Anthony and Jacob, I can see how the proposed ProxyType parameter could fit
into the scheme part of a a URI. However, the problem that introduces is
that we would then have to pick (named) URL schemes to support. But URL
schemes are standardized and it's not obvious which of the standard ones
might apply here.

If we couldn't adopt a standard scheme, we'd have to make one up. At that
point I question the value of putting the (made-up) scheme into a URI
string.

For this reason, I am a fan of the ProxyType parameter over a made-up URL
scheme.

That leaves open Anthony's other idea: eliminating the ProxyType parameter
in favor of a separate method to set each kind of proxy. In the current
RFC, that's just one, e.g. setPoolProxyWithSNI. I guess that comes down to:
what's the likelihood of us supporting other proxy types soon, and then
what's the value of having a single method (and multiple enums) versus
multiple methods (and no enum). If we thought the proxyAddress parameter
would carry different information across proxy types that might tilt us
toward the separate methods. The two on the table, however, (SNI, SOCKS5)
both have identical proxyAddress information.

On Mon, Mar 9, 2020 at 10:54 AM Bill Burcham  wrote:

> By popular demand we are extending the RFC review period. I know Udo asked
> for Friday (and Joris +1'd it), but since this is a small RFC, we'd like to
> try to close it by Wednesday, March 11, ok?
>
> On Mon, Mar 9, 2020 at 10:39 AM Jacob Barrett  wrote:
>
>> I raised similar concerns as a comment in the RFC.
>>
>> > On Mar 9, 2020, at 10:29 AM, Anthony Baker  wrote:
>> >
>> > Given this new API:
>> >
>> >setPoolProxy(ProxyType type, String proxyAddress)
>> >
>> > The ProxyType enum seems to be a look ahead at supporting other kinds
>> of proxies.  What is your thinking about using the enum vs specific named
>> API’s (e.g. setPoolProxyWithSNI).
>> >
>> > Currently the definition of the proxyAddress seems to be dependent of
>> the proxy type.  Did you consider stronger typing using an URI parameter
>> type?
>> >
>> > Anthony
>> >
>> >
>> >
>> >> On Mar 6, 2020, at 11:04 AM, Bill Burcham 
>> wrote:
>> >>
>> >> Please review the RFC for *Client side configuration for a SNI proxy*:
>> >>
>> >>
>> https://cwiki.apache.org/confluence/display/GEODE/Client+side+configuration+for+a+SNI+proxy
>> >>
>> >> Please comment by Monday, March 9, 2020.
>> >>
>> >> Regards,
>> >> Bill and Ernie
>> >
>>
>

Re: RFC - Client side configuration for a SNI proxy

[Bill Burcham]

By popular demand we are extending the RFC review period. I know Udo asked
for Friday (and Joris +1'd it), but since this is a small RFC, we'd like to
try to close it by Wednesday, March 11, ok?

On Mon, Mar 9, 2020 at 10:39 AM Jacob Barrett  wrote:

> I raised similar concerns as a comment in the RFC.
>
> > On Mar 9, 2020, at 10:29 AM, Anthony Baker  wrote:
> >
> > Given this new API:
> >
> >setPoolProxy(ProxyType type, String proxyAddress)
> >
> > The ProxyType enum seems to be a look ahead at supporting other kinds of
> proxies.  What is your thinking about using the enum vs specific named
> API’s (e.g. setPoolProxyWithSNI).
> >
> > Currently the definition of the proxyAddress seems to be dependent of
> the proxy type.  Did you consider stronger typing using an URI parameter
> type?
> >
> > Anthony
> >
> >
> >
> >> On Mar 6, 2020, at 11:04 AM, Bill Burcham 
> wrote:
> >>
> >> Please review the RFC for *Client side configuration for a SNI proxy*:
> >>
> >>
> https://cwiki.apache.org/confluence/display/GEODE/Client+side+configuration+for+a+SNI+proxy
> >>
> >> Please comment by Monday, March 9, 2020.
> >>
> >> Regards,
> >> Bill and Ernie
> >
>

RFC - Client side configuration for a SNI proxy

[Bill Burcham]

Please review the RFC for *Client side configuration for a SNI proxy*:

https://cwiki.apache.org/confluence/display/GEODE/Client+side+configuration+for+a+SNI+proxy

Please comment by Monday, March 9, 2020.

Regards,
Bill and Ernie

Let's Deprecate the SECURITY_UDP_DHALGO Configuration Property

[Bill Burcham]

I propose we deprecate Geode’s proprietary UDP message privacy algorithm
based on the Diffie-Hellman key exchange protocol. This would deprecate:

ConfigurationProperties.SECURITY_UDP_DHALGO

String DistributionConfig.getSecurityUDPDHAlgo()

void DistributionConfig.setSecurityUDPDHAlgo(String attValue)
DistributionConfig.SECURITY_UDP_DHALGO_NAME

Additionally we’d have to upate documentation to reflect deprecation.

>From ConfigurationProperties.java:


Application can set this property to valid symmetric key algorithm, to
encrypt udp messages in Geode. Geode will generate symmetric key using
Diffie-Hellman key exchange algorithm between peers. That key further used
by specified algorithm to encrypt the udp messages.

The property (and the feature) was added mid-2016. Unfortunately it was not
added as an “experimental” feature, so it cannot simply be removed.

Incidentally, the corresponding property for client-server communication,
SECURITY_CLIENT_DHALGO, is already deprecated. It was deprecated in Geode
1.5 in favor of SSL/TLS.

I am proposing deprecating the feature because:


   1.

   The feature has not proven popular with users.
   2.

   At least one hard-to-reproduce bug exists in the implementation:
   GEODE-6448 . We’ve
   burned a person-week trying to fix the problem (Bruce Schuchardt and me)
   and it’s not clear how much more time it will take. If we decide to
   deprecate the feature, fixing this problem would be de-prioritized
   accordingly.
   3.

   If we decide, in the future, that UDP message security is required, it
   would be better to implement a standard algorithm such as DTLS
   :
   1.

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

  DTLS is a standard
  3.

  There is some support for DTLS in the JDK (JEP-219
   delivered in JDK 9). It’s not a
  complete implementation e.g. guaranteed delivery is a do-it-yourself kit.


Actually implementing DTLS is out of scope for this proposal. Adding DTLS
would be a significant undertaking.

So, how do you feel about me making a GEODE ticket to deprecate the
SECURITY_UDP_DHALGO configuration property?

Re: [DISCUSS] Move TcpServer and friends to new geode-tcp-server module

[Bill Burcham]

Thanks for the feedback. We have rough consensus for the new module so we
will make it so.

On Mon, Nov 18, 2019 at 5:34 PM Joris Melchior  wrote:

> +1
>
> On Mon, Nov 18, 2019 at 6:22 PM Owen Nichols  wrote:
>
> > +1
> >
> > > On Nov 18, 2019, at 3:00 PM, Nabarun Nag  wrote:
> > >
> > > +1
> > >
> > > On Mon, Nov 18, 2019 at 2:21 PM Udo Kohlmeyer  wrote:
> > >
> > >> Thank you for this Bill,
> > >>
> > >> I must admit that I'm starting to get a feeling of "scope creep"
> here..
> > >> I understand that all efforts to "modularize" membership would require
> > >> some form of peripheral decoupling.
> > >>
> > >> BUT
> > >>
> > >> I'm starting to get concerned that we are hitting a rabbit hole
> > >> scenario. Maybe this is normal for an effort of this manner, BUT, I
> > >> would like to urge that we start discussing/proposing other
> > >> modulizations, like, Serialization and now TCP communications as real
> > >> modules with own proposals and with their own merit.
> > >>
> > >> YES, I understand this is minimal touch modulizations, but I'm
> concerned
> > >> that we are doing work under the incorrect banner. Just enough to
> > >> complete one "piece of it", but possibly a rework, of the module
> because
> > >> of the "good enough" approach we take.
> > >>
> > >> Maybe we are discovering that there is some pre-work that needed to
> have
> > >> been completed before the whole membership modularization effort was
> > >> started.. And maybe this is the time where we need to take a step
> back,
> > >> look at this from a higher perspective and decide if membership is
> > >> really still the priority here with serialization and transport
> > >> (TCPServer) being a side-effect.
> > >>
> > >> For this reason alone I vote: *-0 *on this matter.. (it is only a
> little
> > >> better than -1)
> > >>
> > >> --Udo
> > >>
> > >> On 11/18/19 1:48 PM, Bill Burcham wrote:
> > >>> Dear Geode,
> > >>>
> > >>> In support of the Membership modularization efforts
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/GEODE/Move+membership+code+to+a+separate+gradle+sub-project
> > >> ,
> > >>> we would like to move the types in the
> > >>> org.apache.geode.distributed.internal.tcpserver package (i.e. the
> > >> TcpServer
> > >>> class and related types) into a separate module in order to eliminate
> > >>> dependencies on geode-core.
> > >>>
> > >>> The membership subsystem is dependent on this group of types, which
> in
> > >> turn
> > >>> are (were) highly-dependent on geode-core. We have been
> systematically
> > >>> eliminating dependencies from these types to geode-core as part of
> > >>> https://issues.apache.org/jira/browse/GEODE-7343 _TcpServer should
> not
> > >>> depend on geode-core_ The final sub-task on that ticket puts
> TcpServer
> > >> and
> > >>> related types into its own separate module.
> > >>>
> > >>> The proposed module would be called "geode-tcp-server" and would
> > contain
> > >>> TcpServer and the other types in the
> > >>> org.apache.geode.distributed.internal.tcpserver package.
> > >>>
> > >>> Your feedback is welcomed and appreciated.
> > >>>
> > >>> Your Friend,
> > >>> Bill
> > >>>
> > >>
> >
> >
>
> --
> *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*
> <https://en.wikipedia.org/wiki/Hal_Abelson>
>

[DISCUSS] Move TcpServer and friends to new geode-tcp-server module

[Bill Burcham]

Dear Geode,

In support of the Membership modularization efforts
https://cwiki.apache.org/confluence/display/GEODE/Move+membership+code+to+a+separate+gradle+sub-project,
we would like to move the types in the
org.apache.geode.distributed.internal.tcpserver package (i.e. the TcpServer
class and related types) into a separate module in order to eliminate
dependencies on geode-core.

The membership subsystem is dependent on this group of types, which in turn
are (were) highly-dependent on geode-core. We have been systematically
eliminating dependencies from these types to geode-core as part of
https://issues.apache.org/jira/browse/GEODE-7343 _TcpServer should not
depend on geode-core_ The final sub-task on that ticket puts TcpServer and
related types into its own separate module.

The proposed module would be called "geode-tcp-server" and would contain
TcpServer and the other types in the
org.apache.geode.distributed.internal.tcpserver package.

Your feedback is welcomed and appreciated.

Your Friend,
Bill

Re: Deprecate SystemFailure Class

[Bill Burcham]

The SystemFailure class is in the org.apache.geode package which means it
is part of the public API.

My purpose in messaging the dev list was to discover any concerns or
objections to adding the deprecated flag to SystemFailure per GEODE-7369.

Related to that work, I intend to eliminate calls to SystemFailure methods
e.g. initiateFailure() from within the membership subsystem per
https://issues.apache.org/jira/browse/GEODE-7354

It's my opinion we should eventually eliminate all such calls throughout
Geode eventually, but I don't know of any imminent plan to do so.

On Tue, Oct 29, 2019 at 8:26 AM Bruce Schuchardt 
wrote:

> Bill, are you proposing to remove calls to
> SystemFailure.initiateFailure() and remove all of the emergency-class
> stuff or just add a Deprecated flag to SystemFailure?
>
>
> On 10/28/19 12:01 PM, Bill Burcham wrote:
> > The SystemFailure class is a clearing house for detecting and attempting
> to
> > mitigate SystemFailure exceptions e.g. VirtualMachineError and
> > OutOfMemoryError.
> >
> > This class should not exist. SystemFailure exceptions should be allowed
> to
> > percolate up and cause the JVM to terminate as soon as possible with an
> > informative message in the log, if possible.
> >
> > Here is what the JVM spec has to say [1]:
> >
> > "A Java Virtual Machine implementation throws an object that is an
> instance
> > of a subclass of the class VirtualMethodError (sic) when an internal
> error
> > or resource limitation prevents it from implementing the semantics
> > described in this chapter. This specification cannot predict where
> internal
> > errors or resource limitations may be encountered and does not mandate
> > precisely when they can be reported."
> >
> > There's a typo in the spec there: it says "VirtualMethodError" when it
> > means "VirtualMachineError". Anyhoo, the upshot is: the JVM spec does not
> > apply after you've encountered a VirtualMachineError. As a result, there
> is
> > no reason to believe that subsequent processing will make things better
> > (than exiting immediately).
> >
> > The SystemFailure class should be deprecated so no new dependencies to it
> > are added. Existing dependencies on it, should be eliminated over time.
> >
> > Do you have any objections to deprecating the class and beginning
> > elimination of usage of it within Geode?
> >
> > Regards,
> > Bill
> >
> > [1]
> https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-6.html#jvms-6.3
> > [2] oh and here's a ticket
> https://issues.apache.org/jira/browse/GEODE-7369
> >
>

Deprecate SystemFailure Class

[Bill Burcham]

The SystemFailure class is a clearing house for detecting and attempting to
mitigate SystemFailure exceptions e.g. VirtualMachineError and
OutOfMemoryError.

This class should not exist. SystemFailure exceptions should be allowed to
percolate up and cause the JVM to terminate as soon as possible with an
informative message in the log, if possible.

Here is what the JVM spec has to say [1]:

"A Java Virtual Machine implementation throws an object that is an instance
of a subclass of the class VirtualMethodError (sic) when an internal error
or resource limitation prevents it from implementing the semantics
described in this chapter. This specification cannot predict where internal
errors or resource limitations may be encountered and does not mandate
precisely when they can be reported."

There's a typo in the spec there: it says "VirtualMethodError" when it
means "VirtualMachineError". Anyhoo, the upshot is: the JVM spec does not
apply after you've encountered a VirtualMachineError. As a result, there is
no reason to believe that subsequent processing will make things better
(than exiting immediately).

The SystemFailure class should be deprecated so no new dependencies to it
are added. Existing dependencies on it, should be eliminated over time.

Do you have any objections to deprecating the class and beginning
elimination of usage of it within Geode?

Regards,
Bill

[1] https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-6.html#jvms-6.3
[2] oh and here's a ticket https://issues.apache.org/jira/browse/GEODE-7369

Re: resource manager requirements & recommendations

[Bill Burcham]

ah like CMSInitiatingOccupancyFraction for CMS

On Wed, Sep 18, 2019 at 8:02 AM Anthony Baker  wrote:

> I’m really interested to follow the ZGC engine and understand how well it
> works for geode.  The -XX:SoftMaxHeapSize option in JDK 13 (just released)
> *might* be the key for us.
>
> Anthony
>
>
> > On Sep 16, 2019, at 1:31 AM, Alberto Bustamante Reyes
>  wrote:
> >
> > Thanks for the answer and the links Anthony, the discussion is
> interesting.
> >
> > I think the differences between using CMS and G1 should be documented, I
> will to contribute in this topic. For example, we have found these comments
> in a GemFire support ticket (
> https://community.pivotal.io/s/question/0D50e5q9JT0CAM/please-refer-to-the-pivotal-ticket-210727
> ):
> >
> > "First, we are not completely compatible with G1GC yet in GemFire,
> meaning that some features, percentages, etc., in GemFire need to be
> rethought out if changing from CMS to G1. For example, if using eviction or
> critical thresholds, with CMS, these percentages would be a % of "Tenured"
> heap size. For G1GC, they would be a % of "Total" heap size, because as you
> may realize, G1GC doesn't have a max Eden space or max Tenured space."
> >
> >
> > 
> > De: Anthony Baker 
> > Enviado: miércoles, 11 de septiembre de 2019 18:58
> > Para: dev@geode.apache.org 
> > Asunto: Re: resource manager requirements & recommendations
> >
> > The challenge with designing a good approach for managing heap use in
> Java is that we *can’t* know how much of the current heap use is really
> garbage.  That means that it can be really easy to evict too much or too
> little data.
> >
> > With the CMS engine there are tuning parameters like occupancy fraction
> that you can set to match the eviction threshold.  This leads to a fairly
> predictable approach to managing heap memory.  With G!GC, the challenge is
> harder since the entire heap might fill up with garbage before any
> collections occur.
> >
> > Despite CMS being deprecated, I think it’s currently the best choice to
> control heap use in Geode.  As noted in JEP 291 [1] and subsequent
> discussion [2]:  "For some applications CMS is a very good fit and might
> always outperform G1”.  I also think we need to do more work in this area
> to make G1 perform as well as CMS.
> >
> > Anthony
> >
> > [1] http://openjdk.java.net/jeps/291
> > [2]
> http://mail.openjdk.java.net/pipermail/jdk9-dev/2017-April/thread.html#start
> >
> >> On Sep 11, 2019, at 9:14 AM, Alberto Bustamante Reyes
>  wrote:
> >>
> >> Hi all,
> >>
> >> Im interested on using the resource manager with G1 garbage collector.
> To check if it is possible, I have been reading documentation about heap
> memory management and I came up with some questions because there are some
> points in the documentation where it is not clear for me if they are
> describing requirements or recommendations.
> >>
> >> As far as I understood, the requirements for using the Resource Manager
> are only two:
> >>
> >> *   set the critical heap percentage
> >> *   configure your GC properly in order to work before the eviction
> procedure starts.
> >>
> >> Am I right? There are three points in the documentation that makes me
> question if I'm correct:
> >>
> >>
> >> 1.  The first chapter in
> https://geode.apache.org/docs/guide/19/managing/heap_use/heap_management.html
> states how to configure your GC for improving performance, but it only
> talks about CMS, there is no info about other GCs.
> >> 2.  In the steps of how to configure ResourceManager, when talking
> about tuning GC parameters, it talks again only about CMS.
> >> 3.  In the documentation of ResourceManager class,
> setCriticalHeapPercentage method, it is stated the following:
> >>
> >> Many virtual machine implementations have additional VM switches to
> control the behavior of the garbage collector. We suggest that you
> investigate tuning the garbage collector when using this type of eviction
> controller. A collector that frequently collects is needed to keep our heap
> usage up to date. In particular, on the Sun HotSpot VM, the
> -XX:+UseConcMarkSweepGC flag needs to be set, [...]
> >>
> >> So it seems that CMS is a requirement, but I have not found in the code
> any limitation about using only CMS.
> >>
> >> If my previous statement about the requirements is fine, then I suppose
> the documentation needs a review to distinguish between generic
> requirements and the CMS specific use case.
> >>
> >> Other question that come to my mind is about the lack of info about G1.
> As CMS is deprecated since Java 9, are there any plans to test and document
> G1 configuration?
> >>
> >> Thanks in advance for your comments!
> >>
> >> Alberto B.
> >>
> >>
> >>
> >>
> >>
> >>
> >
>
>

Re: Unnecessary uses of final on local variables

[Bill Burcham]

-1 to the proposal as I understand it:

Premise:

a. Because the "final" modifier on local variables serves no function other
than to inform compiler optimization and…

b. because the compiler can tell if a variable is "effectively final"
without an explicit "final" modifier in the source code

Conclusion:

• The "final" modifier is just "noise" in the source code, so…

Ramifications:

1. Do not use the "final" modifier on local variables.

2. By extension, it would be not just ok, but in fact a positive change, to
_remove_ the "final" keyword from local variables, because it is just noise.

3. I believe the proposal should be interpreted to apply to method
parameters as well—not just local variables. (Local variables includes loop
variables when using for-each syntax—those can potentially be declared
"final" just like any other local variable).


I am thumbs-down on the proposal, first, because I believe the premise of
the proposal is flawed. If the only value of the "final" modifier on
storage was compile-time optimization then my view might be different. The
fact is that the "final" modifier on a variable (or for-each loop variable)
or method parameter declaration expresses the developer's intention that
the value not be modified after initialization. Any violation of that
intention will be flagged and rejected by the compiler. As such, the
"final" modifier on variable and method parameter declarations is part of
our arsenal of Java language constructs, such as types (including generic
types), that let us express our intentions to the compiler.

Lurking in the proposal is a danger (ramification (2)): that some of us
will think it's not only ok, but an _improvement_ to actively _remove_
final modifiers from local variable declarations, parameter declarations
and for-each loop variable declarations. This lurking danger is a clear and
present threat which I feel requires urgent consensus.

I, for one, put "final" where I mean to put "final". I shouldn't have to
monitor every single PR to ensure my finals aren't removed as noise—any
more than I should have to monitor PRs to ensure folks aren't turning
List back into (pre-generic) List.

Can we at least agree to this:

• when we encounter a final local variable, method parameter, or for-each
loop variable declaration, we agree to _not_ remove the final modifier
unless we have to

And by "have to", I mean, we actually need to mutate the variable to make
the code work. Any aesthetic concern about "final" being "noise" would not
count here.

-Bill

Re: Unnecessary uses of final on local variables

[Bill Burcham]

, 2019, at 10:30 AM, Ernest Burghardt  >
> >>> wrote:
> >>>>
> >>>> +1 to auto-enforcement (if possible) post-consensus
> >>>>
> >>>> On Tue, Jun 18, 2019 at 8:33 AM Murtuza Boxwala 
> >>> wrote:
> >>>>
> >>>>> final in Java does not guarantee immutability.  It would be AWESOME
> if
> >>> it
> >>>>> did but all it guarantees is that the variable cannot be reassigned.
> In
> >>>>> most cases the variable points to an object’s location (memory
> >>> address), so
> >>>>> you can still call methods on it, e.g.
> >>>>>
> >>>>> final var = new Foo();
> >>>>> var.mutateState();
> >>>>>
> >>>>> final variables like these are in no way thread safe. To make objects
> >>>>> immutable, the objects themselves need to follow a pattern that
> >>> guarantees
> >>>>> that.  Something like the ValueObject <
> >>>>> https://martinfowler.com/bliki/ValueObject.html> pattern.
> >>>>>
> >>>>> Mutability may well be the enemy, but I don’t think this is the
> >>> construct
> >>>>> that gets us much/if any closer.
> >>>>>
> >>>>> In local variables and parameters final feels like noise to me, and
> in
> >>>>> fact may make things more difficult to reason about, if we start
> >>> assuming
> >>>>> variables with final are thread safe.
> >>>>>
> >>>>> But I may be missing something.  I am more curious to see how we
> come to
> >>>>> consensus on something like this, because the worst outcome from all
> >>> this
> >>>>> will be to have some folks actively adding final and some actively
> >>> removing
> >>>>> it, which will add noise to PRs and to the code.  And once we reach
> >>>>> consensus, how do we enforce somethings like this? ./gradlew spA?
> >>>>>
> >>>>>> On Jun 17, 2019, at 8:55 PM, Jacob Barrett 
> >>> wrote:
> >>>>>>
> >>>>>> I too am in camp final too. You could say `final boolean useFinal =
> >>>>> true`. For all the same reasons Bill stated below.
> >>>>>>
> >>>>>>> On Jun 17, 2019, at 5:33 PM, Bill Burcham 
> >>> wrote:
> >>>>>>>
> >>>>>>> The final keyword is not redundant—quite the opposite—it's
> extremely
> >>>>> valuable.
> >>>>>>>
> >>>>>>> Local variables are not, in general, final, unless you declare
> them as
> >>>>> such. That being the case, it is not redundant to declare local
> >>> variables
> >>>>> "final".
> >>>>>>>
> >>>>>>> What the compiler will do for you, is _if_ it can ensure that a
> local
> >>>>> variable (or method parameter) is never modified (after
> initialization)
> >>>>> then that variable is treated as "effectively final". Variables that
> are
> >>>>> explicitly declared final, or are determined to be "effectively
> final"
> >>> may
> >>>>> be referenced in lambdas. That's a nice thing.
> >>>>>>>
> >>>>>>> I would like to offer a counter-recommendation: final should be the
> >>>>> default everywhere for fields, for method parameters (on classes,
> not on
> >>>>> interfaces), and for local variables.
> >>>>>>>
> >>>>>>> Many benefits would accrue to us, should we adopt this default:
> >>>>>>>
> >>>>>>> 1. final fields must be initialized in a constructor and never
> mutated
> >>>>> again. This makes reasoning about those fields easier.
> >>>>>>> 2. classes that have all their fields final are immutable and hence
> >>>>> easier to reason about: they can be passed between threads, for
> >>> instance,
> >>>>> with no need to protect from races
> >>>>>>> 3. final method parameters can never be mutated, making them
> easier to
> >>>>> reason about
> >>>>>>> 4. final local variables can never be mutated, making them easier
> to
> >>>>> reason about
> >>>>>>>
> >>>>>>> When final is the rule, non-final is the exception. Another way of
> >>>>> saying that is that when final is the rule, mutability is the
> exception.
> >>>>> That is as it should be. Mutability is the enemy.
> >>>>>>>
> >>>>>>> I have turned on a couple IntelliJ settings that make this the
> default
> >>>>> for me. I encourage you to do the same:
> >>>>>>>
> >>>>>>> First there are these two "Code style issues" in the Java
> inspections:
> >>>>>>>
> >>>>>>> "Field may be 'final'"
> >>>>>>> "Local variable or parameter can be final"
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Then there is this setting will cause newly-defined variables
> created
> >>>>> via the "Extract variable" refactoring:
> >>>>>>>
> >>>>>>> If you select that check box (after selecting those inspections
> >>>>> settings above), it'll declare the newly-introduced variable "final"
> and
> >>>>> it'll remember the setting the next time you invoke "Extract
> variable"
> >>>>> refactoring
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>
> >> --
> >> Cheers
> >>
> >> Jinmei
> >
>
>

Re: Unnecessary uses of final on local variables

[Bill Burcham]

The final keyword is not redundant—quite the opposite—it's extremely
valuable.

Local variables are not, in general, final, unless you declare them as
such. That being the case, it is not redundant to declare local variables
"final".

What the compiler will do for you, is _if_ it can ensure that a local
variable (or method parameter) is never modified (after initialization)
then that variable is treated as "effectively final". Variables that are
explicitly declared final, or are determined to be "effectively final" may
be referenced in lambdas. That's a nice thing.

I would like to offer a counter-recommendation: *final should be the
default everywhere for fields, for method parameters (on classes, not on
interfaces), and for local variables*.

Many benefits would accrue to us, should we adopt this default:

1. final fields must be initialized in a constructor and never mutated
again. This makes reasoning about those fields easier.
2. classes that have all their fields final are immutable and hence easier
to reason about: they can be passed between threads, for instance, with no
need to protect from races
3. final method parameters can never be mutated, making them easier to
reason about
4. final local variables can never be mutated, making them easier to reason
about

When final is the rule, non-final is the exception. Another way of saying
that is that when final is the rule, mutability is the exception. That is
as it should be. *Mutability is the enemy*.

I have turned on a couple IntelliJ settings that make this the default for
me. I encourage you to do the same:

First there are these two "Code style issues" in the Java inspections:

"Field may be 'final'"
"Local variable or parameter can be final"

[image: image.png]

Then there is this setting will cause newly-defined variables created via
the "Extract variable" refactoring:

If you select that check box (after selecting those inspections settings
above), it'll declare the newly-introduced variable "final" and it'll
remember the setting the next time you invoke "Extract variable" refactoring

[image: image.png]

need edit access to ciwiki

[Bill Burcham]

I need edit access to https://cwiki.apache.org for:

bill.burc...@gmail.com

The immediate need is to update some UML here:

https://cwiki.apache.org/confluence/display/GEODE/HA+Client+Event+Queues

Thanks!

Re: [DISCUSS] Static analysis of statics

[Bill Burcham]

On Wed, Feb 13, 2019 at 9:03 AM Dan Smith  wrote:
…

> I can switch them to @MakeReferentImmutable if that makes more sense.
>
> -Dan


I think you understand my concerns. I trust you to decide what's best.

Re: [DISCUSS] Static analysis of statics

[Bill Burcham]

I think the @Immutable anno in *Java Concurrency and Practice* is a class
annotation—not a field one.

Looking at that PR, it looks like this @Immutable anno is usable both on a
type (class) and on a field.

Is that an oversight? If not, then what does it mean? Does @Immutable on a
field mean both:

• the field is final
• the object referenced by the field is immutable

?

Looks like in the current PR @MakeImmutable applies only to fields—not
classes. I imagine that's an oversight.

My quick thought is that these two annotations, in the spirit of *JCP*,
should be type/class-level and not field-level,

Immutable
MakeImmutable

and that perhaps we could have some field-level annos like:

MakeNonStatic
MakeFinal
MakeReferentImmutable - change the type referenced by this field to be an
immutable one

In this way your MakeImmutable field anno is teased apart into two:
MakeFinal, MakeReferentImmutable. The possible benefit is that the two
annos can be used either separately or together to cover 3 situations
instead of just one.

Re: [DISCUSS] Replace @TestingOnly with @VisibleForTesting

[Bill Burcham]

+1. Annotation beats comment in this case, and @VisibleForTesting is more
descriptive than our old anno.

On Mon, Dec 17, 2018 at 3:43 PM Jinmei Liao  wrote:

> +1. Yes, the original intention was exactly as @VisibleForTesting.
>
> On Mon, Dec 17, 2018 at 2:45 PM Galen O'Sullivan 
> wrote:
>
> > Sounds good to me.
> >
> > On Mon, Dec 17, 2018 at 1:04 PM Owen Nichols 
> wrote:
> >
> > > Sounds good, I would be happy to +1 a PR for this
> > >
> > > > On Dec 17, 2018, at 12:22 PM, Kirk Lund  wrote:
> > > >
> > > > We have a custom annotation in geode-common called @TestingOnly:
> > > >
> > > >
> > geode-common/src/main/java/org/apache/geode/annotations/TestingOnly.java
> > > >
> > > > This annotation was created while pairing with Michael Feathers and
> the
> > > > intention was to annotate non-private constructors or methods that
> > have a
> > > > widened visibility scope to facilitate testing.
> > > >
> > > > Some developers, however, have interpreted it as meaning that the
> > > > constructor or method cannot be used in the main src code and can
> only
> > be
> > > > used from test src code.
> > > >
> > > > I'd like to propose deleting @TestingOnly and change to
> > > > using @VisibleForTesting which is defined in Guava (which is already
> a
> > > > Geode dependency). The name of the Guava annotation is less ambiguous
> > and
> > > > it's already been adopted for use by additional projects including
> > > AssertJ
> > > > which we use extensively.
> > > >
> > > > The javadocs on VisibleForTesting explains its usage very clearly:
> > > >
> > > > /**
> > > > * Annotates a program element that exists, or is more widely visible
> > > > than otherwise necessary, only
> > > > * for use in test code.
> > > > *
> > > > * @author Johannes Henkel
> > > > */
> > > > @GwtCompatible
> > > > public @interface VisibleForTesting {
> > > > }
> > >
> > >
> >
>
>
> --
> Cheers
>
> Jinmei
>

Re: Questions about Poms and Publishing

[Bill Burcham]

@Patrick Rhomberg  I've never seen the
dependencyManagement element survive in a published POM before.

Since it sounds like you're asserting that you saw that element in a
published POM (published by Gradle), I decided to verify that. I ran this
from the Geode develop branch just now:

./gradlew build publishMavenPublicationToMavenLocal -x javadoc
-Dskip.tests=true

When I look
at 
~/.m2/repository/org/apache/geode/geode-core/1.9.0-SNAPSHOT/geode-core-1.9.0-SNAPSHOT.pom
I see no dependencyManagement section.

The absence of that element comports with my experience. My experience w/
the dependencyManagement element is that it is used when you're using Maven
to manage your build. It is wonderful for DRYing up what would otherwise be
unmanageable version information spread among lots of pom.xml (source) file.

"dependency constraints" in Gradle sounds like it'd be a big step forward
for similar reasons. I'd assume that "dependency constraints" don't result
in a dependencyManagement element in any published POM file though.


On Wed, Nov 7, 2018 at 10:00 AM Jacob Barrett  wrote:

> The dependency management element applies dependency constraints to first
> class dependencies and transitive dependencies. For example in dependency
> management of this say A:1 and B:2 it does not mean your module will
> necessarily depend on A:1 and B:2 but if the module or transitive module
> does that the versions will be nudged to match these constraints. So then
> if you module you have a dependency section that includes A it will become
> A:1 and if A:1 depends on B:1 then B:1 will be nudged to B:2.
>
> -Jake
>
>
> > On Nov 6, 2018, at 3:25 PM, Anthony Baker  wrote:
> >
> > I want reproducible builds.  If dependency locking [1] works I would be
> open to dynamic versions [2].
> >
> > Anthony
> >
> > [1] https://docs.gradle.org/current/userguide/dependency_locking.html
> > [2]
> https://docs.gradle.org/current/userguide/declaring_dependencies.html#sub:declaring_dependency_with_dynamic_version
> >
> >
> >
> >> On Nov 6, 2018, at 3:02 PM, Patrick Rhomberg 
> wrote:
> >>
> >> My current question surrounds the structure of POMs in specifying
> version
> >> information.  Gradle supports `dependency constraints` to unify library
> >> versioning.  This seems to me to be a clean, concise alternative to our
> >> current approach of cluttering the project property space with
> >> project.'library.version', with mixed adherence throughout our build
> files.
> >
>

Re: [PROPOSAL] Add getCache and getLocator to Launchers

[Bill Burcham]

+1 for exposing getCache() on CacheLauncher instances. Death to all
singletons!

I'm less certain about the wisdom of exposing a getCache() on
LocatorLauncher instances. Seems like it would be better to let clients
call getLocator() on LocatorLauncher instances, then they can do the
traversal to the cache via getCache() themselves. Would it make sense to
expose getLocator() on LocatorLauncher instances (instead of exposing
getCache() on those)?

If we did implement getCache() for LocatorLauncher instances, it might look
something like this:

// on LocatorLauncher
public Optional getCache() {
return locator == null ? Optional.empty() :
Optional.of(locator.getCache());
}

That traversal seems more appropriate for the caller to implement. That way
the caller knows why the cache is unavailable (when its unavailable) e.g.
because there is no locator vs. because there is a locator but that locator
has no cache.

On Wed, Oct 31, 2018 at 3:05 PM Dan Smith  wrote:

> Yay for APIs that don't require singletons!
>
> -Dan
>
> On Wed, Oct 31, 2018 at 2:54 PM Jinmei Liao  wrote:
>
> > +1. sounds like a good addition and since we already have package level
> > getters for them anyway.
> >
> > On Wed, Oct 31, 2018 at 2:48 PM Kirk Lund  wrote:
> >
> > > LocatorLauncher provides an API which can be used in-process to create
> a
> > > Locator. There is no public API on that class to get a reference to the
> > > Locator or its Cache.
> > >
> > > Similarly, ServerLauncher provides an API which can be used in-process
> to
> > > create a Server, but there is no public API in that class to get a
> > > reference to its Cache.
> > >
> > > The User of either Launcher would then have to resort to invoking
> > > singletons to get a reference to the Cache.
> > >
> > > There are existing package-private getter APIs on both Launchers but
> > > they're only used by tests in that same package.
> > >
> > > I propose adding public APIs for getCache to both LocatorLauncher and
> > > ServerLauncher as well as adding getLocator to LocatorLauncher. The
> > > signatures would look like:
> > >
> > > /**
> > >  * Gets a reference to the Cache that was created by this
> ServerLauncher.
> > >  *
> > >  * @return a reference to the Cache
> > >  */
> > > public org.apache.geode.cache.Cache getCache();
> > >
> > > /**
> > >  * Gets a reference to the Locator that was created by this
> > > LocatorLauncher.
> > >  *
> > >  * @return a reference to the Locator
> > >  */
> > > public org.apache.geode.distributed.Locator getLocator();
> > >
> > > Any thoughts? Yay or nay?
> > >
> > > Thanks,
> > > Kirk
> > >
> >
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>

Re: [Discuss] Transitive dependencies and internal .pom changes

[Bill Burcham]

So it sounds like (per Robert) we use the gradle-lint-plugin's
unused-dependency rule to warn us of unused dependencies. That handles one
side of the equation (only list *necessary* dependencies).

I haven't heard anyone mention tool support for the other side of the
equation: ensuring that we list the *sufficient* list of dependencies per
build.gradle.

Patrick, you just mentioned the java-library plugin's ability to
distinguish between api and implementation dependencies. Geode doesn't use
that plugin yet AFAIK, nor does the PR introduce that plugin right?

All this seems of a piece with the march toward modularity, exemplified by
Java 9 modules. Patrick, is your vision something like:

1. accept this PR
…
2. introduce the java-library plugin and segregate dependencies into api
vs. implementation ones
…
N. define Java modules


On Fri, Sep 28, 2018 at 10:39 AM Patrick Rhomberg 
wrote:

> Bill has the heart of it, yes.
>
> I should have also mentioned that this ties into java-library plugin
> configuration, notably that the `compile` configuration is deprecated.
> For dependences that we do not wish to leak, we will need to use
> `implementation`.  For dependencies which we intentionally elect to share
> with our consumers, we should use `api`.
>
> For modularity, it should not break one module's ability to build for
> another module to declare a `compile` configuration to be `implementation`,
> unless of course that dependency is an active and necessary component part
> of that module's consumption.  In that case, it should be declared part of
> the modules `api`.
> In this vein and as Bill pointed out, a module should have an accurate
> representation of its own build-time dependencies.
>
> In time, this will allow a much improved build experience, since
> `implementation` and `api` are positioned for better granularity of
> incremental building.
>
> On Fri, Sep 28, 2018 at 10:23 AM, Robert Houghton 
> wrote:
>
> > Hi Bill,
> >
> > We are using a Gradle plug-in to identify dependencies that are unused,
> or
> > are declared in the wrong module or scope.
> > This is called out by the Gradle documents on improving build [
> > https://guides.gradle.org/performance/#avoid_unnecessary_and_unused_
> > dependencies].
> > The plug-in documentation is here [
> > https://github.com/nebula-plugins/gradle-lint-plugin/
> > wiki/Unused-Dependency-Rule
> > ]
> >
> > Thank you,
> > -Robert Houghton
> >
> > On Fri, Sep 28, 2018 at 10:18 AM Bill Burcham 
> wrote:
> >
> > > From the PR, Anthony, it seems to me that Patrick is proposing that
> each
> > > build.gradle be explicit about mentioning the "things" it depends on.
> For
> > > example:
> > >
> > > [image: image.png]
> > >
> > > Look at how geode-connectors goes from mentioning a few dependencies to
> > > mentioning many more. The value of this is that it ostensibly shows us
> > all
> > > the things that geode-connectors actually depends on.
> > >
> > > The challenge I see is two-fold:
> > >
> > > • as with Java imports and "unused imports", there is the risk of
> listing
> > > more dependencies than we actually need
> > > • there's also the risk that we still don't have a complete list of
> > > dependencies
> > >
> > > Patrick: do you have tool support for this? Is there a tool that can
> > > identify or even remove unused dependencies? What process do you use
> for
> > > finding these heretofore hidden dependencies? Did you run gradle
> > > app:dependencies?
> > >
> >
>

Re: [Discuss] Transitive dependencies and internal .pom changes

[Bill Burcham]

>From the PR, Anthony, it seems to me that Patrick is proposing that each
build.gradle be explicit about mentioning the "things" it depends on. For
example:

[image: image.png]

Look at how geode-connectors goes from mentioning a few dependencies to
mentioning many more. The value of this is that it ostensibly shows us all
the things that geode-connectors actually depends on.

The challenge I see is two-fold:

• as with Java imports and "unused imports", there is the risk of listing
more dependencies than we actually need
• there's also the risk that we still don't have a complete list of
dependencies

Patrick: do you have tool support for this? Is there a tool that can
identify or even remove unused dependencies? What process do you use for
finding these heretofore hidden dependencies? Did you run gradle
app:dependencies?

please give me (bburcham) permission to assign himself Jira tickets

[Bill Burcham]

I made a ticket and would like to assign it to myself. Please let me.

-Bill