Re: Unnecessary uses of final on local variables

2019-06-19 Thread Kirk Lund
Thanks for the very thoughtful and well-worded reply, Bill. I would certainly welcome further discussion on how and where to consistently use the final keyword (if that's desirable), especially within the context of reviewing pull requests. On Wed, Jun 19, 2019 at 11:23 AM Bill Burcham wrote: >

Unnecessary uses of final on local variables

2019-06-13 Thread Kirk Lund
According to Effective Java 3rd Edition, all local variables are implicitly made final by the JVM (and thus receiving the same JVM optimizations as a final variable) without requiring use of the keyword as long as you only set the value once. So this becomes an unnecessary use of the keyword final

Re: [PROPOSAL] Instrumenting Geode Code

2019-06-11 Thread Kirk Lund
+1 Looks good On Mon, Jun 10, 2019 at 5:13 PM Aaron Lindsey wrote: > +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 >

Please review PR #3650

2019-06-06 Thread Kirk Lund
Reminder: Please review PR #3650 https://github.com/apache/geode/pull/3650. Cleanup commits are separated from the fix commits to help facilitate review. GEODE-6183: Fix isAttachAPIFound and launcher tests Thanks, Kirk

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

2019-06-04 Thread Kirk Lund
+1 for doing whatever facilitates reviewing and is best for the PR (which may vary by PR or even reviewers) -1 to disallowing or even strongly discouraging force pushing in a PR (go ahead and merge or rebase as each person prefers but don't force that preference on others) I prefer to separate

Re: [DISCUSS] require reviews before merging a PR

2019-06-04 Thread Kirk Lund
I'm -1 for requiring N reviews before merging a commit. Overall, I support Lazy Consensus. If I post a PR that fixes the flakiness in a test, the precheckin jobs prove it, and it sits there for 2 weeks without reviews, then I favor merging it in at that point without any reviews. I'm not going to

Re: [DISCUSS] Remove exception.getMessage() error handling

2019-05-28 Thread Kirk Lund
I favor providing a detailed log message that includes the Throwable as an argument. I have seen code like the following in Geode: } catch (Exception exception) { logger.error(exception.getMessage()); } ...but I consider that to be a naive anti-pattern for informing the user that a problem

Re: Remove mavenLocal from Geode gradle files

2019-05-08 Thread Kirk Lund
nings. See https://docs.gradle.org/5.4/userguide/command_line_interface.html#sec:command_line_warnings *BUILD FAILED* in 57s 389 actionable tasks: 46 executed, 86 from cache, 257 up-to-date On Wed, May 8, 2019 at 1:29 PM Kirk Lund wrote: > I'll assign the bug to you so you can deci

Re: Remove mavenLocal from Geode gradle files

2019-05-08 Thread Kirk Lund
I'll assign the bug to you so you can decide if you want to close it or try to fix it... On Wed, May 8, 2019 at 1:26 PM Kirk Lund wrote: > Other people including Barry have run into it as well. > > No, I'm not using --offline, but I will try --refresh-dependencies. The > only

Re: Remove mavenLocal from Geode gradle files

2019-05-08 Thread Kirk Lund
es geode commands, right? > > > > On Wed, May 8, 2019, 12:12 Jacob Barrett wrote: > > > > > Maven local is necessary for some of our other build processes like > > > benchmarks. > > > > > > Is there no other way to correct this issue. I have ne

Remove mavenLocal from Geode gradle files

2019-05-08 Thread Kirk Lund
I'd like like to remove mavelLocal the Geode gradle files. GEODE-6753: Use of mavenLocal in gradle may cause build to fail with missing tests dependencies https://issues.apache.org/jira/browse/GEODE-6753 Thanks, Kirk

How to run PulseAuthorizationTest

2019-04-22 Thread Kirk Lund
Is there a way to run a geode-pulse uiTest on the command-line without it failing due to "webdriver.chrome.driver system property"? I would expect the gradle build to be setting this property. $ ./gradlew geode-pulse:uiTest --tests PulseAuthorizationTest ... > Task :geode-pulse:uiTest

Re: Request for bulk transition permission in Geode Jira

2019-04-22 Thread Kirk Lund
I think I gave you permissions for this. Let me know if it doesn't work for you. Thanks! On Mon, Apr 22, 2019 at 7:16 AM Owen Nichols wrote: > As per Geode release manager instructions, I will need to bulk-transition > resolved issues to completed (once we have voted to release a 1.9.0 RC). >

Re: Fixing Awaitility await().untilAsserted(new WaitCriterion

2019-04-11 Thread Kirk Lund
ion extends ThrowingRunnable. So this pattern should still work > as WaitCriterion did before. But just using an assertion inside of a lambda > is the better option. > > -Dan > > On Thu, Apr 11, 2019 at 12:02 PM Kirk Lund wrote: > > > Just a quick heads up... I'm seei

Fixing Awaitility await().untilAsserted(new WaitCriterion

2019-04-11 Thread Kirk Lund
Just a quick heads up... I'm seeing an Awaitility usage pattern that is broken and does nothing. Specifically, it's any uses of dunit WaitCriterion with untilAsserted: GeodeAwaitility.*await().untilAsserted(new WaitCriterion*() { @Override public boolean done() {

Re: Jira Permission Request

2019-04-10 Thread Kirk Lund
Done! You should have permissions now. Thanks! On Wed, Apr 10, 2019 at 3:18 PM Aaron Lindsey wrote: > 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

Re: Permission request

2019-04-10 Thread Kirk Lund
Done! You should have edit permissions now. Thanks! On Wed, Apr 10, 2019 at 3:15 PM Aaron Lindsey wrote: > 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 >

Re: Jira & wiki permissions

2019-04-10 Thread Kirk Lund
Done! You should have permissions to edit both. Thanks! On Wed, Apr 10, 2019 at 3:13 PM Alberto Bustamante Reyes wrote: > Hi Geode community, > > > I would like to have permissions to edit the wiki and assign me tickets. > Could someone help me with this? > > My username is

Re: Request wiki edit permission

2019-04-10 Thread Kirk Lund
Done! If there's anything you find that doesn't work for you, let me know. Thanks! On Wed, Apr 10, 2019 at 11:35 AM Owen Nichols wrote: > Hi, my Apache LDAP username is onichols. > > I would like to request permission to edit > https://cwiki.apache.org/confluence/display/GEODE < >

Re: [DISCUSS] Move or remove org.apache.geode.admin

2019-04-05 Thread Kirk Lund
them to continue to limp along on deprecated, unmaintained > and > >>> untested APIs because we missed an arbitrary window to remove the > symbols. > >>> > >>> If however the community agrees with you interpretation then we must > make > >>> sure a

Re: [DISCUSS] Move or remove org.apache.geode.admin

2019-04-04 Thread Kirk Lund
ow to remove the > symbols. > >> > >> If however the community agrees with you interpretation then we must > make > >> sure a ticket is created for the next major with a list of deprecated > items > >> and updated when new items are deprecated so that the task is perfor

Re: [DISCUSS] Move or remove org.apache.geode.admin

2019-04-03 Thread Kirk Lund
1) +1 YES. If we continue to *not* have at least one major release per year, then I 100% support the removal of deprecated APIs and features in a minor release such as 1.10. If we decide to have at least one major release per year, then I'd be willing to revisit this and consider not allowing

Re: Weird intermittent build error caused by spotlessJava

2019-03-26 Thread Kirk Lund
arguments --no-daemon and --no-parallel respectively, if you > don't want that behavior to be the default as specified in your local > ~/.gradle/gradle.properties. > > On Tue, Mar 26, 2019 at 11:46 AM Kirk Lund wrote: > > > I actually have two lines in my gradle.properties. The com

Re: Weird intermittent build error caused by spotlessJava

2019-03-26 Thread Kirk Lund
n a Mac using gradlew in geode (I don't have my own > version of gradle installed) and oracle jdk1.8.0_202. > > On Tue, Mar 26, 2019 at 10:48 AM Kirk Lund wrote: > >> Few more details that might help others... >> >> Each Geode module fails in the same way but obviously o

Re: Weird intermittent build error caused by spotlessJava

2019-03-26 Thread Kirk Lund
own version of gradle installed) and oracle jdk1.8.0_202. On Tue, Mar 26, 2019 at 10:48 AM Kirk Lund wrote: > Few more details that might help others... > > Each Geode module fails in the same way but obviously on a different .java > file, and I have not altered these .java file

Re: Weird intermittent build error caused by spotlessJava

2019-03-26 Thread Kirk Lund
ion On Tue, Mar 26, 2019 at 10:43 AM Kirk Lund wrote: > Intermittent build error caused by spotlessJava. My checkout of Geode > intermittently gets into a state that then fails to build. I've been seeing > this come and go for the last month or two. I'm not sure what puts it into

Weird intermittent build error caused by spotlessJava

2019-03-26 Thread Kirk Lund
Intermittent build error caused by spotlessJava. My checkout of Geode intermittently gets into a state that then fails to build. I've been seeing this come and go for the last month or two. I'm not sure what puts it into this state but executing: $ ./gradlew clean build -x test ...will

Review of GEODE-6295: Add Micrometer-based metrics system #3277

2019-03-11 Thread Kirk Lund
Please review GEODE-6295: Add Micrometer-based metrics system #3277 if you have time: https://github.com/apache/geode/pull/3277 Thanks, Kirk

Re: Review request for GEODE-6295: Add Micrometer-based metrics system #3277

2019-03-11 Thread Kirk Lund
Please review our PR. Thanks! On Fri, Mar 8, 2019 at 4:09 PM Kirk Lund wrote: > Please review GEODE-6295: Add Micrometer-based metrics system #3277 and > provide any feedback you have in the PR or feel free to reply to this email > thread. > > https://github.com/apache/

Review request for GEODE-6295: Add Micrometer-based metrics system #3277

2019-03-08 Thread Kirk Lund
Please review GEODE-6295: Add Micrometer-based metrics system #3277 and provide any feedback you have in the PR or feel free to reply to this email thread. https://github.com/apache/geode/pull/3277 Thanks, Kirk

Request PR review for #3263

2019-03-06 Thread Kirk Lund
Anyone have time to review a PR for me? It fixes up another DUnit rule so it works with adding and bouncing VMs. https://github.com/apache/geode/pull/3263 Thanks, Kirk

Re: Bug Numbers and Trac Numbers in comments

2019-02-21 Thread Kirk Lund
rry about aligning versions of > what's in the code with potential versions of the outside resource. So my > vote is for augmenting the comment. Maybe we can do even better and make > the code or test self-explanatory enough to need even fewer comments? > > On Wed, Feb 20, 201

Re: Bug Numbers and Trac Numbers in comments

2019-02-20 Thread Kirk Lund
Well, the problem is that different people disagree on what's "meaningful" in this context. For example: See PersistentPartitionHangsDuringRestartRegressionTest.java * /*** * * RegressionTest for bug 42226. * * * 1. Member A has the bucket * * * 2. Member B starts creating the bucket. It

StopServerWithSecurityAcceptanceTest fails in precheckin

2019-02-14 Thread Kirk Lund
org.apache.geode.management.internal.cli.commands.StopServerWithSecurityAcceptanceTest > cannotStopServerAsClusterReaderOverHttp FAILED org.junit.ComparisonFailure: expected:<[0]> but was:<[1]> at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at

Precheckin flakiness

2019-02-13 Thread Kirk Lund
Despite our attempts to make precheckin free of flaky failures, I'm still seeing at least one flaky failure (that does not reproduce and does not correlate to my PR changes) in every single precheckin launched for my PRs. For example, the latest one (

Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Kirk Lund
Quite a few Geode stats are currently defined as IntCounters which very easily wrap past Integer.MAX_VALUE. Some users wanted these stats to not wrap to negative MAX_VALUE, so my team defined the following two tickets and changed them to LongCounters on the develop branch: GEODE-6345:

Re: geode-junit and geode-dunit classpath problems

2019-02-08 Thread Kirk Lund
, 2019 at 2:01 PM Kirk Lund wrote: > We have a src/main class called UseJacksonForJsonPathRule which uses > classes from this dependency in geode-junit/build.gradle: > > compile('com.jayway.jsonpath:json-path') > > I'm trying to write a unit test in src/test called > UseJacks

geode-junit and geode-dunit classpath problems

2019-02-08 Thread Kirk Lund
We have a src/main class called UseJacksonForJsonPathRule which uses classes from this dependency in geode-junit/build.gradle: compile('com.jayway.jsonpath:json-path') I'm trying to write a unit test in src/test called UseJacksonForJsonPathRuleTest but it fails with the following

Re: Very red CI -> Hold merges, please

2019-02-08 Thread Kirk Lund
> a doubt go back to our usual flow of merging to develop. > > > > > > Thoughts? > > > > > > On Thu, Feb 7, 2019 at 2:37 PM Kirk Lund wrote: > > > > > >> Hmm, and that was another false search hit in Jira! Searching for > > >>

Re: Adding integrationTest src set to geode-dunit

2019-02-08 Thread Kirk Lund
Yep, I'll ping you later this afternoon to see if you're free. Thanks! On Thu, Feb 7, 2019 at 4:11 PM Robert Houghton wrote: > Can I work with you on this tomorrow? > > On Thu, Feb 7, 2019, 15:09 Kirk Lund wrote: > > > The usual geode src sets like integrationTest don't alr

Adding integrationTest src set to geode-dunit

2019-02-07 Thread Kirk Lund
The usual geode src sets like integrationTest don't already exist in some modules such as geode-dunit. I'm trying to write a new IntegrationTest but simply creating the directories and placing a new .java file in it doesn't seem to work.

Re: Very red CI -> Hold merges, please

2019-02-07 Thread Kirk Lund
Kirk Lund wrote: > The UpgradeTest failures on your latest commit for this PR are > WANRollingUpgradeNewSenderProcessOldEvent which seems to be a reoccurrence > of [GEODE-3967](https://issues.apache.org/jira/browse/GEODE-3967). I > recommend having Gester take a look at that these failure

Re: Very red CI -> Hold merges, please

2019-02-07 Thread Kirk Lund
://issues.apache.org/jira/browse/GEODE-3967) as resolved on Jan 9th. On Thu, Feb 7, 2019 at 12:37 PM Jens Deppe wrote: > No worries. I think I have a better fix now. At least the builds are moving > again. > > On Thu, Feb 7, 2019 at 12:11 PM Kirk Lund wrote: > > > Sorry

Re: Very red CI -> Hold merges, please

2019-02-07 Thread Kirk Lund
Sorry, go ahead and revert the commit and reopen the PR. On Thu, Feb 7, 2019 at 11:36 AM Jens Deppe wrote: > I was still working on a fix... > > On Thu, Feb 7, 2019 at 11:31 AM Kirk Lund wrote: > > > I merged it in. > > > > On Thu, Feb 7, 2019 at 11:28 AM Kirk Lun

Re: Very red CI -> Hold merges, please

2019-02-07 Thread Kirk Lund
I merged it in. On Thu, Feb 7, 2019 at 11:28 AM Kirk Lund wrote: > I think we should go ahead and merge in > https://github.com/apache/geode/pull/3172 since it resolves the > GfshConsoleModeUnitTest UnitTest failures. > > On Thu, Feb 7, 2019 at 9:57 AM Nabarun Nag wrote: > &

Re: Very red CI -> Hold merges, please

2019-02-07 Thread Kirk Lund
I think we should go ahead and merge in https://github.com/apache/geode/pull/3172 since it resolves the GfshConsoleModeUnitTest UnitTest failures. On Thu, Feb 7, 2019 at 9:57 AM Nabarun Nag wrote: > FYI, I have just merged a ci timeout fix to increase the timeout for > geode-benchmarks to 4h.

GfshConsoleModeUnitTest is failing in UnitTest

2019-02-06 Thread Kirk Lund
Did someone break unit tests? I have 3 PRs with unit tests failing in GfshConsoleModeUnitTest: org.apache.geode.management.internal.cli.shell.GfshConsoleModeUnitTest > consoleModeShouldRedirectOnlyJDKLoggers FAILED java.lang.AssertionError: Expecting:

Re: ExecutorServiceRuleDumpThreads fails intermittently

2019-02-05 Thread Kirk Lund
. On Tue, Feb 5, 2019 at 11:55 AM Kirk Lund wrote: > ExecutorServiceRuleDumpThreads.showsThreadBlockedByOtherThread fails > intermittently in precheckin. > > UnitTestOpenJDK11 failure: [ > https://concourse.apachegeode-ci.info/builds/35684|https://concourse.apachegeode-ci.in

ExecutorServiceRuleDumpThreads fails intermittently

2019-02-05 Thread Kirk Lund
ExecutorServiceRuleDumpThreads.showsThreadBlockedByOtherThread fails intermittently in precheckin. UnitTestOpenJDK11 failure: [ https://concourse.apachegeode-ci.info/builds/35684|https://concourse.apachegeode-ci.info/builds/35684 ] *This is one I recently created, so I'll fix it. *Not sure why

LocatorUDPSecurityDUnitTest fails intermittently

2019-02-05 Thread Kirk Lund
LocatorUDPSecurityDUnitTest failed in one of my PR precheckins. It does not reproduce on my branch and I cannot find any open Jira tickets for it. DistributedTestOpenJDK11 failure: https://concourse.apachegeode-ci.info/builds/35457 I'm not sure who to assign this to. Any suggestions or

Review request for PR #3123

2019-02-04 Thread Kirk Lund
Does anyone have time to review PR #3123 for me? GEODE-6301: Use ThreadInfo.toString in ExecutorServiceRule.dumpThreads https://github.com/apache/geode/pull/3123 This PR changes ExecutorServiceRule.dumpThreads to use java.lang.management.ThreadInfo.toString() which includes lock monitors and

Re: [Proposal] Adding Micrometer to Apache Geode

2019-02-04 Thread Kirk Lund
+1 to add Micrometer in the described way and I'll add my approval to the PR as well On Fri, Feb 1, 2019 at 4:16 PM Dale Emery wrote: > Hello all, > > I've created a PR to add Micrometer to Geode: > https://github.com/apache/geode/pull/3153 > > I invite your review. > > The Micrometer system

ImportClusterConfigTest flaky failure

2019-01-30 Thread Kirk Lund
I just filed https://issues. apache.org/jira/browse/GEODE-6343 against ImportClusterConfigTest.importWouldNotShutDownServer because it failed in a precheckin. I'm not sure who to assign this failure to. Any suggestions or volunteers? Failure: https://concourse.apachegeode-ci.info/builds/34064

PR review request

2019-01-24 Thread Kirk Lund
Anyone want to review my PR, please? GEODE-6313: Make ControllableProcess fail if status is only whitespace #3112 https://github.com/apache/geode/pull/3112 Thanks, Kirk

Re: dunit hang in ClusterConfigLocatorRestartDUnitTest

2019-01-23 Thread Kirk Lund
le synchronizers: - <0xd0884428> (a java.util.concurrent.ThreadPoolExecutor$Worker) On Wed, Jan 23, 2019 at 10:02 AM Kirk Lund wrote: > I hit a dunit hang in one of my precheckin runs. > > The only test mentioned in callstacks/dunit-hangs.txt is > ClusterConfig

dunit hang in ClusterConfigLocatorRestartDUnitTest

2019-01-23 Thread Kirk Lund
I hit a dunit hang in one of my precheckin runs. The only test mentioned in callstacks/dunit-hangs.txt is ClusterConfigLocatorRestartDUnitTest. I see some Pooled Message Processor threads that might be hung waiting for the same

Re: Gradle build broken?

2019-01-23 Thread Kirk Lund
I saw the same problem a few weeks ago. I ended up deleting the directories in my .m2 repository and rebuilding. That seemed to fix it. The cause seems to have something to do with that log4j core tests jar, but I'm not sure why our build would be looking for the corresponding sources jar. If

Re: Creating the default disk store after persistent region

2019-01-18 Thread Kirk Lund
ava.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748) On Fri, Jan 18, 2019 at 2:24 PM Kirk Lund wrote: > And my description doesn't match the threads again... Just ignore the > message. > > Look at the threads if you care. >

Re: Creating the default disk store after persistent region

2019-01-18 Thread Kirk Lund
And my description doesn't match the threads again... Just ignore the message. Look at the threads if you care. On Fri, Jan 18, 2019 at 2:18 PM Kirk Lund wrote: > Have there been any changes within the last year involving the following? > Is anyone familiar with getOrCreateDefaultDis

Creating the default disk store after persistent region

2019-01-18 Thread Kirk Lund
Have there been any changes within the last year involving the following? Is anyone familiar with getOrCreateDefaultDiskStore and when it's invoked? When Region creation for a persistent Region does not specify a disk store, the code call getOrCreateDefaultDiskStore. It then proceeds to create

Re: Loner changes its membership port when starting an acceptor

2019-01-16 Thread Kirk Lund
ll > use the toString of the member ID. I think that's what's happening to > you. The getName() method in InternalDistributedMember doesn't return a > string like you've described. It returns whatever was configured as the > "name" in the distribution configuration properties

Re: Loner changes its membership port when starting an acceptor

2019-01-16 Thread Kirk Lund
of an object > to never change. Let's add another method that returns a Bean > identifier and is documented to never change. > > On 1/15/19 9:45 AM, Kirk Lund wrote: > > Sorry about the confusion. I meant that the change of membership port > > results in DistributedMember

Re: Loner changes its membership port when starting an acceptor

2019-01-15 Thread Kirk Lund
ever there is also a "unique ID" in > > LonerDistributionManager that was supposed to address this problem but > > apparently didn't. > > > > On 1/14/19 4:45 PM, Kirk Lund wrote: > >> So I was stepping through some WAN tests in IJ debugger (on develop >

Loner changes its membership port when starting an acceptor

2019-01-14 Thread Kirk Lund
So I was stepping through some WAN tests in IJ debugger (on develop with no changes) and discovered that any MXBeans that are created before starting a server port (either using CacheServer or GatewayReceiver) are broken and fail to be updated after that -- the ObjectNames include the

StatSampler no longer logs fatal message for JDK-8207200

2019-01-08 Thread Kirk Lund
I just committed a change to develop which catches the IllegalStateException caused by JDK-8207200 when the StatSampler samples JVM memory usage using MemoryMXBean. commit 1143af997292df0f6d480084ffd2103fa2c17bbf (origin/develop, origin/HEAD) Author: Kirk Lund Date: Mon Jan 7 16:43:34 2019

Re: [DISCUSS] Disable merge for failing pull requests

2018-12-26 Thread Kirk Lund
the merge button, then my PR could potentially be blocked indefinitely. After we get it more consistently GREEN, I would be willing to change my vote to +1. On Fri, Dec 21, 2018 at 10:36 AM Kirk Lund wrote: > I was responding to Udo's comment: > > "Could one not configure the button

PR review requests

2018-12-26 Thread Kirk Lund
If anyone is free to review a couple PRs, please take a look at mine: GEODE-3205: Cleanup and reenable DiskSpaceLimitIntegrationTest https://github.com/apache/geode/pull/3035 - basically, the FILE_SIZE_LIMIT_BYTES was too small, resulting in rolling too many - added lots more info in assertion

Re: Re: [PROPOSAL] use default value for validate-serializable-objects in dunit

2018-12-21 Thread Kirk Lund
asn't > whitelisted. The message would be rejected and the thread issuing the > function would hang. > > > > On 12/21/18 2:42 PM, Kirk Lund wrote: > > > > > > I filed GEODE-6202: DUnit should not enable VALIDATE_SERIALIZABLE_OBJECTS > > by default > > >

Re: [PROPOSAL] use default value for validate-serializable-objects in dunit

2018-12-21 Thread Kirk Lund
ded class. > > We also have non-default values for cluster configuration in dunit runs. > > > On 3/13/18 10:03 AM, Kirk Lund wrote: > > I want to propose using the default value for > validate-serializable-object > > in dunit tests instead of forcing it on for all dunit t

Reminder: do NOT @Ignore tests that flicker

2018-12-21 Thread Kirk Lund
Just a reminder: please do not @Ignore tests that flicker. I was pairing on some statistics changes this week and ran across a disabled test in DiskSpaceLimitIntegrationTest: @Test @Ignore("Fails frequently in docker container.") public void aboveZeroDeletesPreviousFiles() throws Exception

Re: [DISCUSS] Disable merge for failing pull requests

2018-12-21 Thread Kirk Lund
e: > Kirk, this change would not require you to get someone to merge it. It > would just require that your PR pass CI before it can be merged. > > On Thu, Dec 20, 2018 at 2:38 PM Kirk Lund wrote: > > > I have enough trouble just getting other developers to review my PR. I > > d

Re: Javadoc errors in org/apache/geode/cache/configuration/RegionAttributesType.java

2018-12-20 Thread Kirk Lund
ke we forgot to clean > > up the javadocs. Will make a PR to fix ASAP. > > > > -Aditya > > > > On Thu, Dec 20, 2018 at 1:57 PM Kirk Lund wrote: > > > >> Just a reminder that any classes not behind internal packages need to > have > >> functio

Re: [DISCUSS] Disable merge for failing pull requests

2018-12-20 Thread Kirk Lund
I have enough trouble just getting other developers to review my PR. I don't want to have to struggle to find someone to merge it for me, too. On Mon, Nov 19, 2018 at 4:09 PM Udo Kohlmeyer wrote: > I don't believe "name and shame" is a hammer we should wield, but if we > have use it... use it

Javadoc errors in org/apache/geode/cache/configuration/RegionAttributesType.java

2018-12-20 Thread Kirk Lund
Just a reminder that any classes not behind internal packages need to have functional Javadocs. We have Javadoc errors involving org/apache/geode/cache/configuration/RegionAttributesType.java referring to RegionTimeToLive which I can't find when I grep. > Task :geode-assembly:docs

'io.spring.dependency-management' not found.

2018-12-19 Thread Kirk Lund
My other IJ project is now broken after letting IJ update. Anyone know anything about geode-dependency-management.gradle or why it would cause: Caused by: org.gradle.api.plugins.UnknownPluginException: Plugin with id 'io.spring.dependency-management' not found. I tried refreshing from Gradle

Re: Two copies of ExtendsFunctionAdapter.java

2018-12-17 Thread Kirk Lund
ing built at all since they're in the > resources section rather than java. I don't see compiled versions of these > classes in my geode directory. Perhaps it's an IntelliJ configuration > issue? > > Galen > > > On Mon, Dec 17, 2018 at 11:23 AM Kirk Lund wrote: > > >

[DISCUSS] Replace @TestingOnly with @VisibleForTesting

2018-12-17 Thread Kirk Lund
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

Two copies of ExtendsFunctionAdapter.java

2018-12-17 Thread Kirk Lund
IntelliJ just started failing to compile because we have two copies of ExtendsFunctionAdapter.java. Apparently, IJ was happy enough to ignore these duplicates for a month or so, but it's now fed up and will no longer tolerate the duplication so it's failing with: Error:(21, 8) java: duplicate

Working on Geode with IntelliJ 2018.3

2018-12-13 Thread Kirk Lund
IntelliJ 2018.3.1 downloads sources jars for the jars of old geode versions that the geode-old-versions module refers to. The Javadoc {@link class} editor support in IJ 2018.3.1 then seems to randomly reference some of the Geode classes in those old geode jars instead of local .java files in the

Re: geode-core dependencies for maven pom

2018-12-06 Thread Kirk Lund
ett wrote: > It should be in the POM as optional. > > > On Dec 6, 2018, at 10:17 AM, Kirk Lund wrote: > > > > Is the presence of "ext.optional = true" for spring-shell in > > geode-core/build.gradle the reason it's missing from the geode-core maven > > pom?

Re: geode-core dependencies for maven pom

2018-12-06 Thread Kirk Lund
dule: 'asm' exclude module: 'cglib' exclude module: 'guava' exclude module: 'spring-aop' exclude module: 'spring-context-support' exclude module: 'spring-core' *ext.optional = true* } On Thu, Dec 6, 2018 at 10:12 AM Kirk Lund wrote: > The geode-core maven pom is missing

Re: geode-core dependencies for maven pom

2018-12-06 Thread Kirk Lund
ontents are as expected. The POM > is generated from dependencies and transitive dependencies specified in the > various Gradle files. > > > On Dec 6, 2018, at 9:55 AM, Kirk Lund wrote: > > > > Is this the only file controlling and testing the dependencies for the >

Re: geode-core dependencies for maven pom

2018-12-06 Thread Kirk Lund
Is this the only file controlling and testing the dependencies for the geode-core maven pom that we publish? geode-core/src/test/resources/expected-pom.xml On Thu, Dec 6, 2018 at 9:52 AM Kirk Lund wrote: > Can someone please point me at the right place to review and alter > the depend

geode-core dependencies for maven pom

2018-12-06 Thread Kirk Lund
Can someone please point me at the right place to review and alter the dependencies for geode-core that are being published for its maven pom? Also, are there any tests involving the dependencies of the geode-core maven pom?

Re: PowerMock and mock ClassLoader

2018-12-04 Thread Kirk Lund
ng usages. > > > > Ryan > > > > On Tue, Dec 4, 2018 at 3:08 PM Kirk Lund wrote: > > > > > I filed GEODE-6143: PowerMock should not be used in Geode tests. > > > > > > We need everyone to stop using PowerMock in new tests. If anyone sees a >

Re: Using gradlew --tests runs test class twice

2018-12-04 Thread Kirk Lund
Oops, you're right. On Tue, Dec 4, 2018 at 11:03 AM Jacob Barrett wrote: > I don’t see how your output indicates the test is running twice. Is there > more output missing from the clip? Try added ‘—info’ to your Gradle command > to validate. > > > > On Dec 4, 2018, at 10:53

Using gradlew --tests runs test class twice

2018-12-04 Thread Kirk Lund
So, based on the output that I see from gradle when I execute this: $ ./gradlew geode-core:integrationTest --tests AnalyzeSerializablesJUnitTest ...gradle apparently runs the test class twice. I say this because this is my output: 94% EXECUTING [1m 21s] geode-core:integrationTest > 3 tests

PowerMock and mock ClassLoader

2018-12-04 Thread Kirk Lund
Anyone have any ideas which unit test is using PowerMock and is injecting a mock ClassLoader? This keeps failing in my precheckin runs. I think we need to a) remove all uses of PowerMock and b) forbid its use going forward. 2018-12-04 18:11:36,258 Distributed system shutdown hook ERROR Could not

Re: Third Iteration of Java Modules Support

2018-12-03 Thread Kirk Lund
Moving internal.logging to geode-logging would involve moving some internal.logging classes to org.apache.geode.logging. I think that would help with the logging portion of what you're doing. On Mon, Dec 3, 2018 at 4:30 PM Kirk Lund wrote: > I'm iteratively cleaning up geode's internal logg

Re: Third Iteration of Java Modules Support

2018-12-03 Thread Kirk Lund
I'm iteratively cleaning up geode's internal logging (specifically our dependency on log4j core) so that log4j core can be optional and a user could switch it out for logback or do further customization of our logging. Even without the modules work you're doing, moving our logging internals to a

SampleHandler interface and handling of statistics samples

2018-11-26 Thread Kirk Lund
SampleHandler is in the org.apache.geode.internal.statistics package. The SampleHandler interface was originally introduced to allow multiple handlers to receive notification of a statistics sample. Before this interface, the StatSampler used a StatArchiveWriter directly to write to the .gfs

Re: [DISCUSS] Geode packages mustn't span Jigsaw modules

2018-11-26 Thread Kirk Lund
One problem about moving around internal classes is that Geode uses (proprietary and Java-based) serialization on the wire instead of defining a wire-format that isn't dependent on class names/structures/packages. I for one would love to move to a real wire-format with a well-defined protocol but

Re: Using SystemOutRule or SystemErrRule in Geode tests

2018-11-26 Thread Kirk Lund
stemOutRule().enableLog(); ...then it will repeatedly pass if you run-until-failure in IntelliJ but only because IJ ends up running it as one test class. It will still fail when run repeatedly by stressNewTest or if a previous test class caused Log4J2 to startup within the same JVM. On Mon, Nov 2

Using SystemOutRule or SystemErrRule in Geode tests

2018-11-26 Thread Kirk Lund
Log4J and Logback both capture a reference to System.out and/or System.err and then use that reference thereafter. This means that any manipulation of System.out or System.err -- using System.setOut(PrintStream) or System.setErr(PrintStream) -- probably isn't going to play nicely with logging in

Unit test target loads up full Geode logging

2018-11-26 Thread Kirk Lund
The unit test target has geode-core/src/main/resources in the classpath which means that Log4J finds our log4j2.xml file and creates/registers our custom appenders. We might want to have a unit test log4j2.xml that gets placed earlier on the classpath but only for the unit test target. The

AcceptanceTestOpenJDK11 precheckin fails to compile

2018-11-26 Thread Kirk Lund
Just the first few compilation failures are listed below. Is the image for AcceptanceTestOpenJDK11 precheckin broken in some way? /home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java:20: error: cannot access Constructor import

Re: Avoiding Unpredictability in Our DUnit Testing Rules

2018-11-26 Thread Kirk Lund
Typo fix: If you need to use CleanupDUnitVMsRule along with other dunit rules, then you will need to* use RuleChain*. If you don't need to use CleanupDUnitVMsRule then you don't need to use RuleChain. On Mon, Nov 26, 2018 at 11:57 AM Kirk Lund wrote: > Actually the only prob

Re: Avoiding Unpredictability in Our DUnit Testing Rules

2018-11-26 Thread Kirk Lund
Actually the only problem with this is specific to bouncing of dunit VMs. I filed "GEODE-6033: DistributedTest rules should support VM bounce" earlier this month and I have a branch with preliminary changes that seem to be working fine. Aside from bouncing of dunit VMs, the dunit rules you listed

Handling precheckin jobs with concourse errors

2018-11-26 Thread Kirk Lund
The DistributedTestOpenJDK11 precheckin job for PR #2878 failed with what I think is a Concourse error: https://github.com/apache/geode/pull/2878 I believe the expected process from me is to just poke my PR to repeat precheckin. If that's wrong, please let me know. Thanks!

What is the new process for flaky test failures in precheckin?

2018-11-26 Thread Kirk Lund
I just saw SizingFlagDUnitTest fail in a precheckin but it passes on my branch when I run directly. I cannot find a Jira ticket for it. What's the new process for handling these flickering tests? See: https://concourse.apachegeode-ci.info/builds/17745 Test failure stack:

<    1   2   3   4   5   6   7   8   9   10   >