Permissions to apache-geode jira dashboard

[Murtuza Boxwala]

Can I please have permissions to edit the Apache-Geode JIRA dashboard.  My 
username is mboxwala.  I would like to be able to add new tickets, edit ticket, 
assign tickets to myself…

Thanks,
Murtuza

Re: Unnecessary uses of final on local variables

[Murtuza Boxwala]

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

Re: Unnecessary uses of final on local variables

[Murtuza Boxwala]

What does "rough consensus"[1] look like on this thread? How do we make a 
decision and close it out?

Kirk suggested an idea, there’s been a couple days of feedback, so we can:

1) reject the proposal and commit to using final ‘everywhere'
2) accept the proposal and use final very sparingly
3) continue the discussion over e-mail
4) take other steps to come to a consensus like
have a meeting with a few folks who are passionate about the topic
Try both approaches.  Pick one module and use final everywhere, and 
pick another and use it sparingly and see which approach we like


Personally, I don’t see any “fundamental flaws"[1] with using final or removing 
it.


Also, I might just be rushing the natural flow of conversation, so in that 
case, sorry for being impatient.

1-https://doist.com/blog/decision-making-flat-organization/ 
<https://doist.com/blog/decision-making-flat-organization/>

> On Jun 18, 2019, at 2:48 PM, Jinmei Liao  wrote:
> 
> I agree with Murtuza, most finals on local variables and method parameters
> are just noise to me. I only use "final" on these two situations:
> 1. to declare public static constants of immutable types (e.g. String,
> Integer)
> 2. to prevent children from overriding a method.
> 
> But thought I can't offer an example, I don't want to put out a blank
> statement saying that "final" on local variables are entirely unnecessary.
> There must be a case that it could be useful, so even if we come to a
> consensus that local variables should not have final on it, I don't think
> using a static analysis tool to get rid of all of them is a good idea.
> 
> On Tue, Jun 18, 2019 at 11:14 AM Anthony Baker  wrote:
> 
>> I’ll offer this alternative:  perhaps shorter method bodies obviate the
>> need for explicit final vars.
>> 
>> Anthony
>> 
>> 
>>> On Jun 18, 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 "effectivel

Confluence access

[Murtuza Boxwala]

Can I get access to the confluence wiki, please?

Email: mboxw...@pivotal.io 
Username: mboxwala

Re: [DISCUSS] Add a test dependency to geode-core - ArchUnit

[Murtuza Boxwala]

I think that’s a really clever way to increment toward splitting geode-core 
into more modules. I am excited to see what it looks like 👍

> On Jun 20, 2019, at 7:45 PM, Jacob Barrett  wrote:
> 
> Gotcha! Sounds good.
> 
>> On Jun 20, 2019, at 4:35 PM, Dan Smith  wrote:
>> 
>> We don't have a membership gradle module, just a package. We're adding this
>> to geode-core.
>> 
>> For a little more context - we are thinking about refactoring membership
>> (and/or maybe some other pieces) into separate gradle modules - proposal
>> forthcoming! However, as a first step we need to untangle those pieces of
>> code from the rest of geode-core. Rather than creating some long lived
>> branch we can incrementally untangle the code a piece at a time, on
>> develop. Having a way to track progress and enforce the direction of
>> dependencies on the way to a separate gradle module will help with that.
>> 
>> -Dan
>> 
>>> On Thu, Jun 20, 2019 at 4:23 PM Jacob Barrett  wrote:
>>> 
>>> Are you adding this dependency to just the membership module? I am cool
>>> with that.
>>> 
 On Jun 20, 2019, at 2:39 PM, Dan Smith  wrote:
 
 Hi all,
 
 Bill, Ernie, and I would like to add a new (apache licensed) test
 dependency to geode-core - https://github.com/TNG/ArchUnit. This is a
>>> tool
 that lets you write tests that make assertions about the
>>> interdependencies
 of your code - for example enforcing that package A does not depend on
 package B.
 
 Initially we intend to add some tests about what parts of the system the
 org.apache.geode.distributed.internal.membership package depends on, with
 an eye towards making that code more independently testable (proposal on
 that coming soon!).
 
 Does anyone have an issue with adding this test dependency?
 
 -Dan
>>> 
>>> 

Re: [DISCUSS] Add a test dependency to geode-core - ArchUnit

[Murtuza Boxwala]

Two things come to mind:

1) uni-directional dependency
Packages can be dependent on each other, because the classes inside of them can 
use each other. e.g.let’s say package A has class A1 and class A2 and 
package B has class B1 and B2.  A1 can depend on B1, and B2 can depend on A2. 
Hence, the packages are dependent on each other.

Modules can only have uni-directional dependency. If Module A depends on Module 
B, then no class in Module B can reference a class in Module A.  This prevents 
tangling, i.e. spaghetti

2) Incremental compilation
This lack of tangling helps not only developers, but the compiler too.  In the 
packages example above, if I change any of the classes, all the code has to get 
recompiled because the dependency lines can go in any direction, and the 
compiler won’t attempt to optimize.  In the modules case, if Module A changes, 
Module B will not recompile, because the dependency guarantees that nothing 
about Module B could have been affected.

> On Jun 21, 2019, at 2:14 PM, Udo Kohlmeyer  wrote:
> 
> I know that I'm missing the benefit of physically moving the code from the 
> package into its own module.
> 
> Could you possibly explain to me what it is?
> 
> On 6/21/19 07:37, Murtuza Boxwala wrote:
>> I think that’s a really clever way to increment toward splitting geode-core 
>> into more modules. I am excited to see what it looks like 👍
>> 
>>> On Jun 20, 2019, at 7:45 PM, Jacob Barrett  wrote:
>>> 
>>> Gotcha! Sounds good.
>>> 
>>>> On Jun 20, 2019, at 4:35 PM, Dan Smith  wrote:
>>>> 
>>>> We don't have a membership gradle module, just a package. We're adding this
>>>> to geode-core.
>>>> 
>>>> For a little more context - we are thinking about refactoring membership
>>>> (and/or maybe some other pieces) into separate gradle modules - proposal
>>>> forthcoming! However, as a first step we need to untangle those pieces of
>>>> code from the rest of geode-core. Rather than creating some long lived
>>>> branch we can incrementally untangle the code a piece at a time, on
>>>> develop. Having a way to track progress and enforce the direction of
>>>> dependencies on the way to a separate gradle module will help with that.
>>>> 
>>>> -Dan
>>>> 
>>>>> On Thu, Jun 20, 2019 at 4:23 PM Jacob Barrett  wrote:
>>>>> 
>>>>> Are you adding this dependency to just the membership module? I am cool
>>>>> with that.
>>>>> 
>>>>>> On Jun 20, 2019, at 2:39 PM, Dan Smith  wrote:
>>>>>> 
>>>>>> Hi all,
>>>>>> 
>>>>>> Bill, Ernie, and I would like to add a new (apache licensed) test
>>>>>> dependency to geode-core - https://github.com/TNG/ArchUnit. This is a
>>>>> tool
>>>>>> that lets you write tests that make assertions about the
>>>>> interdependencies
>>>>>> of your code - for example enforcing that package A does not depend on
>>>>>> package B.
>>>>>> 
>>>>>> Initially we intend to add some tests about what parts of the system the
>>>>>> org.apache.geode.distributed.internal.membership package depends on, with
>>>>>> an eye towards making that code more independently testable (proposal on
>>>>>> that coming soon!).
>>>>>> 
>>>>>> Does anyone have an issue with adding this test dependency?
>>>>>> 
>>>>>> -Dan
>>>>> 

GEODE-6637

[Murtuza Boxwala]

Problem:

Every time we make a put the code in BucketRegion.searchAndLock and 
BucketRegion.removeAndNotifyKeys locks (synchronized) on a hash map, the 
allKeysMap, to checkout the key to be modified and prevent anyone else from 
updating at the same time. This area of the code gets a lot of contention, 
especially if the same keys are getting checked out.

Deeper Problem:

In trying to fix this, I would like to use a different data structure, like a 
ConcurrentHashMap but am running across a problem.  It seems that when we want 
to put a single item, we end up calling the same code that putAll calls, and we 
just wrap the single key in an array. It further seems that the entire 
allKeysMap is synchronized because we want to lock multiple keys at the same 
time and do the putAll ‘atomically.’

Question

Is there any reason that putAll needs to lock all the keys and insert at the 
same-ish time. Could putAll be turned into something that looked like: 

public void putAll(entries)
for(Entry entry: entries)
put(entry.key, entry.value)

Or is it important that they all happen together?  Does putAll guarantee some 
kind of transactional properties?

An analogy

Imagine a restaurant (our hash map) that has a bunch of tables built for 1 
person (think classroom chairs with a pull out desk).  PutAll is like coming to 
this restaurant and asking for a table for four people.  Now someone has to go, 
find four tables, pull them together, etc…you know how it goes.


Any help is appreciated.

Thanks,
Murtuza

https://issues.apache.org/jira/browse/GEODE-6637

Re: IntelliJ setup for develop

[Murtuza Boxwala]

In my ideal world, I compile and run tests with IntelliJ.  IntelliJ is 
constantly recompiling, so when you hit run, it’s usually just ready to run the 
tests and the feedback loop feels much faster.

But, the “output path is not specified for modules” does seem to reappear 
randomly, and apparently it will disappear on its own after a few restarts 
maybe…not sure.  A re-import will fix the issue as well, but the unreliability 
is definitely annoying.  Switching to gradle makes things more predictable at 
the cost of speed.  I currently have both runners set as "Default (IntelliJ)” 
and haven’t had issues for a couple weeks now.

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

Pipeline permission

[Murtuza Boxwala]

I am trying to deploy a "local" meta pipeline, but ran into a permissions issue 
when trying to set or pause pipelines in apache geode. Can someone please add 
me to the right group(s)?

Re: Unit tests are hanging?

[Murtuza Boxwala]

How down know these warnings are related to the builds hanging.  When I was on 
CIO duty a couple days ago, I remember seeing this warning in a failing build, 
but then I looked back at passing builds on saw this as well.

> On Aug 7, 2019, at 12:40 PM, Kirk Lund  wrote:
> 
> The build is broken in CI right now (for main CI and PRs). The UnitTest
> jobs are timing out so I assume there's a hang of some sort.
> 
> The WARNINGs appear to be related to PowerMock and begins with "An illegal
> reflective access operation" in geode-assembly:test.
> 
> I'm running unit tests locally and haven't hit this yet, but maybe we
> should consider disabling all PowerMock tests and increase priority on
> re-enabling them without PowerMock?
> 
>> Task :geode-assembly:test
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by
> org.mockito.internal.util.reflection.AccessibilityChanger
> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
> to field java.io.File.path
> WARNING: Please consider reporting this to the maintainers of
> org.mockito.internal.util.reflection.AccessibilityChanger
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by
> org.mockito.internal.util.reflection.AccessibilityChanger
> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
> to field java.io.File.path
> WARNING: Please consider reporting this to the maintainers of
> org.mockito.internal.util.reflection.AccessibilityChanger
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> 
>> Task :geode-core:test
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by
> org.apache.geode.distributed.internal.deadlock.UnsafeThreadLocal
> (file:/home/geode/geode/geode-core/build/classes/java/main/) to method
> java.lang.ThreadLocal.getMap(java.lang.Thread)
> WARNING: Please consider reporting this to the maintainers of
> org.apache.geode.distributed.internal.deadlock.UnsafeThreadLocal
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by
> org.mockito.internal.util.reflection.AccessibilityChanger
> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
> to field java.util.Date.fastTime
> WARNING: Please consider reporting this to the maintainers of
> org.mockito.internal.util.reflection.AccessibilityChanger
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by
> org.powermock.reflect.internal.WhiteboxImpl
> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.powermock/powermock-reflect/2.0.0-beta.5/4ea415348f15620783a1f26343d6732adfa86bc8/powermock-reflect-2.0.0-beta.5.jar)
> to method java.lang.Object.finalize()
> WARNING: Please consider reporting this to the maintainers of
> org.powermock.reflect.internal.WhiteboxImpl
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by
> org.junit.contrib.java.lang.system.EnvironmentVariables
> (file:/home/geode/.gradle/caches/modules-2/files-2.1/com.github.stefanbirkner/system-rules/1.19.0/d541c9a1cff0dda32e2436c74562e2e4aa6c88cd/system-rules-1.19.0.jar)
> to field java.util.Collections$UnmodifiableMap.m
> WARNING: Please consider reporting this to the maintainers of
> org.junit.contrib.java.lang.system.EnvironmentVariables
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> 
> timeout exceeded

Re: Unit tests are hanging?

[Murtuza Boxwala]

Yea.  I think it might be a red herring, because I am seeing those errors in 
every run, passing ones two…just double checked on 
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UnitTestOpenJDK11/builds/948
 
<https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UnitTestOpenJDK11/builds/948>

If we don’t have any other ideas, though, it might be worth removing them.

It is odd that you don’t see them locally. Let me see if I can get them to show 
up…it might have something to do with the logging level.

> On Aug 7, 2019, at 12:45 PM, Kirk Lund  wrote:
> 
> I don't know if the PowerMock warnings are related but that's the only
> thing interesting in the output before "timeout exceeded".
> 
> On Wed, Aug 7, 2019 at 9:43 AM Murtuza Boxwala  wrote:
> 
>> How down know these warnings are related to the builds hanging.  When I
>> was on CIO duty a couple days ago, I remember seeing this warning in a
>> failing build, but then I looked back at passing builds on saw this as well.
>> 
>>> On Aug 7, 2019, at 12:40 PM, Kirk Lund  wrote:
>>> 
>>> The build is broken in CI right now (for main CI and PRs). The UnitTest
>>> jobs are timing out so I assume there's a hang of some sort.
>>> 
>>> The WARNINGs appear to be related to PowerMock and begins with "An
>> illegal
>>> reflective access operation" in geode-assembly:test.
>>> 
>>> I'm running unit tests locally and haven't hit this yet, but maybe we
>>> should consider disabling all PowerMock tests and increase priority on
>>> re-enabling them without PowerMock?
>>> 
>>>> Task :geode-assembly:test
>>> WARNING: An illegal reflective access operation has occurred
>>> WARNING: Illegal reflective access by
>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>> 
>> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
>>> to field java.io.File.path
>>> WARNING: Please consider reporting this to the maintainers of
>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>> WARNING: Use --illegal-access=warn to enable warnings of further illegal
>>> reflective access operations
>>> WARNING: All illegal access operations will be denied in a future release
>>> WARNING: An illegal reflective access operation has occurred
>>> WARNING: Illegal reflective access by
>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>> 
>> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
>>> to field java.io.File.path
>>> WARNING: Please consider reporting this to the maintainers of
>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>> WARNING: Use --illegal-access=warn to enable warnings of further illegal
>>> reflective access operations
>>> WARNING: All illegal access operations will be denied in a future release
>>> 
>>>> Task :geode-core:test
>>> WARNING: An illegal reflective access operation has occurred
>>> WARNING: Illegal reflective access by
>>> org.apache.geode.distributed.internal.deadlock.UnsafeThreadLocal
>>> (file:/home/geode/geode/geode-core/build/classes/java/main/) to method
>>> java.lang.ThreadLocal.getMap(java.lang.Thread)
>>> WARNING: Please consider reporting this to the maintainers of
>>> org.apache.geode.distributed.internal.deadlock.UnsafeThreadLocal
>>> WARNING: Use --illegal-access=warn to enable warnings of further illegal
>>> reflective access operations
>>> WARNING: All illegal access operations will be denied in a future release
>>> WARNING: An illegal reflective access operation has occurred
>>> WARNING: Illegal reflective access by
>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>> 
>> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
>>> to field java.util.Date.fastTime
>>> WARNING: Please consider reporting this to the maintainers of
>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>> WARNING: Use --illegal-access=warn to enable warnings of further illegal
>>> reflective access operations
>>> WARNING: All illegal access operations will be denied in a future release
>>> WARNING: An illegal reflective access operation

Re: Unit tests are hanging?

[Murtuza Boxwala]

That makes sense…I’m running 11 and I see them

> On Aug 7, 2019, at 1:11 PM, Helena Bales  wrote:
> 
> Kirk, as Lynn said in slack, these warnings are showing up with JDK11. So
> you'd probably see them locally if you used 11 instead of 8.
> 
> On Wed, Aug 7, 2019, 10:05 AM Kirk Lund  wrote:
> 
>> We previously decided to stop using PowerMock in any Geode tests and remove
>> it. Ryan and I created https://issues.apache.org/jira/browse/GEODE-6143
>> but
>> it's languishing because it takes a lot of time to strip out PowerMock from
>> a test.
>> 
>> I don't get any PowerMock warnings locally when I run unit tests.
>> 
>> On Wed, Aug 7, 2019 at 9:50 AM Murtuza Boxwala 
>> wrote:
>> 
>>> Yea.  I think it might be a red herring, because I am seeing those errors
>>> in every run, passing ones two…just double checked on
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UnitTestOpenJDK11/builds/948
>>> <
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UnitTestOpenJDK11/builds/948
>>>> 
>>> 
>>> If we don’t have any other ideas, though, it might be worth removing
>> them.
>>> 
>>> It is odd that you don’t see them locally. Let me see if I can get them
>> to
>>> show up…it might have something to do with the logging level.
>>> 
>>>> On Aug 7, 2019, at 12:45 PM, Kirk Lund  wrote:
>>>> 
>>>> I don't know if the PowerMock warnings are related but that's the only
>>>> thing interesting in the output before "timeout exceeded".
>>>> 
>>>> On Wed, Aug 7, 2019 at 9:43 AM Murtuza Boxwala 
>>> wrote:
>>>> 
>>>>> How down know these warnings are related to the builds hanging.  When
>> I
>>>>> was on CIO duty a couple days ago, I remember seeing this warning in a
>>>>> failing build, but then I looked back at passing builds on saw this as
>>> well.
>>>>> 
>>>>>> On Aug 7, 2019, at 12:40 PM, Kirk Lund  wrote:
>>>>>> 
>>>>>> The build is broken in CI right now (for main CI and PRs). The
>> UnitTest
>>>>>> jobs are timing out so I assume there's a hang of some sort.
>>>>>> 
>>>>>> The WARNINGs appear to be related to PowerMock and begins with "An
>>>>> illegal
>>>>>> reflective access operation" in geode-assembly:test.
>>>>>> 
>>>>>> I'm running unit tests locally and haven't hit this yet, but maybe we
>>>>>> should consider disabling all PowerMock tests and increase priority
>> on
>>>>>> re-enabling them without PowerMock?
>>>>>> 
>>>>>>> Task :geode-assembly:test
>>>>>> WARNING: An illegal reflective access operation has occurred
>>>>>> WARNING: Illegal reflective access by
>>>>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>>>>> 
>>>>> 
>>> 
>> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
>>>>>> to field java.io.File.path
>>>>>> WARNING: Please consider reporting this to the maintainers of
>>>>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>>>>> WARNING: Use --illegal-access=warn to enable warnings of further
>>> illegal
>>>>>> reflective access operations
>>>>>> WARNING: All illegal access operations will be denied in a future
>>> release
>>>>>> WARNING: An illegal reflective access operation has occurred
>>>>>> WARNING: Illegal reflective access by
>>>>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>>>>> 
>>>>> 
>>> 
>> (file:/home/geode/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-core/2.23.0/497ddb32fd5d01f9dbe99a2ec790aeb931dff1b1/mockito-core-2.23.0.jar)
>>>>>> to field java.io.File.path
>>>>>> WARNING: Please consider reporting this to the maintainers of
>>>>>> org.mockito.internal.util.reflection.AccessibilityChanger
>>>>>> WARNING: Use --illegal-access=warn to enable warnings of further
>>> illegal
>>>>>> reflective access operations
>>&g

Re: ./gradlew test no longer runs/reruns tests?

[Murtuza Boxwala]

To re-run tests it looks like the property is “forceTest”, so ./gradlew test 
-PforceTest should do the trick.

For diagnosing the javadocs, this might  come in handy:
A neat gradle trick I like to use to see which dependent tasks will get run is 
the “-m” flag.
So something like "./gradlew -m build" will show all the tasks that will run 
without actually running them.

> On Aug 7, 2019, at 1:19 PM, Kirk Lund  wrote:
> 
> I have another question about an apparent change in "build". It looks like
> javadoc is no longer running if I do a "./gradlew build". Even "./gradlew
> clean build" doesn't generate all javadocs.
> 
> *"./gradlew clean build" (or just "build") results in:*
> 
> /Users/klund/dev/gemfire/geode [640]$ find . -name
> "javadocs"
> ./geode-assembly/build/javadocs
> /Users/klund/dev/gemfire/geode [641]$ find . -name
> "javadoc"
> ./geode-old-versions/1.9.0/build/apache-geode-1.9.0/javadoc
> ./geode-old-versions/1.3.0/build/apache-geode-1.3.0/javadoc
> ./geode-old-versions/1.7.0/build/apache-geode-1.7.0/javadoc
> ./geode-old-versions/1.4.0/build/apache-geode-1.4.0/javadoc
> ./geode-old-versions/1.5.0/build/apache-geode-1.5.0/javadoc
> ./geode-old-versions/1.2.0/build/apache-geode-1.2.0/javadoc
> ./geode-old-versions/1.8.0/build/apache-geode-1.8.0/javadoc
> ./geode-old-versions/1.6.0/build/apache-geode-1.6.0/javadoc
> ./geode-assembly/build/install/apache-geode/javadoc
> 
> *"./gradlew javadoc" results in:*
> 
> /Users/klund/dev/geode3 [576]$ find . -name
> "javadocs"
> ./geode-assembly/build/javadocs
> /Users/klund/dev/geode3 [577]$ find . -name
> "javadoc"
> ./geode-pulse/geode-pulse-test/build/javadoc
> ./geode-pulse/geode-pulse-test/build/tmp/javadoc
> ./geode-junit/build/javadoc
> ./geode-junit/build/tmp/javadoc
> ./geode-concurrency-test/build/javadoc
> ./geode-concurrency-test/build/tmp/javadoc
> ./geode-lucene/geode-lucene-test/build/javadoc
> ./geode-lucene/geode-lucene-test/build/tmp/javadoc
> ./geode-lucene/build/javadoc
> ./geode-lucene/build/tmp/javadoc
> ./geode-redis/build/javadoc
> ./geode-redis/build/tmp/javadoc
> ./geode-json/build/javadoc
> ./geode-json/build/tmp/javadoc
> ./geode-old-versions/1.9.0/build/apache-geode-1.9.0/javadoc
> ./geode-old-versions/1.3.0/build/apache-geode-1.3.0/javadoc
> ./geode-old-versions/1.7.0/build/apache-geode-1.7.0/javadoc
> ./geode-old-versions/1.4.0/build/apache-geode-1.4.0/javadoc
> ./geode-old-versions/1.5.0/build/apache-geode-1.5.0/javadoc
> ./geode-old-versions/1.2.0/build/apache-geode-1.2.0/javadoc
> ./geode-old-versions/1.8.0/build/apache-geode-1.8.0/javadoc
> ./geode-old-versions/1.6.0/build/apache-geode-1.6.0/javadoc
> ./geode-memcached/build/javadoc
> ./geode-memcached/build/tmp/javadoc
> ./geode-assembly/build/install/apache-geode/javadoc
> ./geode-assembly/geode-assembly-test/build/javadoc
> ./geode-assembly/geode-assembly-test/build/tmp/javadoc
> ./extensions/geode-modules-session/build/javadoc
> ./extensions/geode-modules-session/build/tmp/javadoc
> ./extensions/session-testing-war/build/javadoc
> ./extensions/session-testing-war/build/tmp/javadoc
> ./extensions/geode-modules-test/build/javadoc
> ./extensions/geode-modules-test/build/tmp/javadoc
> ./extensions/geode-modules-tomcat8/build/javadoc
> ./extensions/geode-modules-tomcat8/build/tmp/javadoc
> ./extensions/geode-modules-tomcat7/build/javadoc
> ./extensions/geode-modules-tomcat7/build/tmp/javadoc
> ./extensions/geode-modules-tomcat9/build/javadoc
> ./extensions/geode-modules-tomcat9/build/tmp/javadoc
> ./extensions/geode-modules/build/javadoc
> ./extensions/geode-modules/build/tmp/javadoc
> ./geode-management/build/javadoc
> ./geode-management/build/tmp/javadoc
> ./geode-rebalancer/build/javadoc
> ./geode-rebalancer/build/tmp/javadoc
> ./geode-dunit/build/javadoc
> ./geode-dunit/build/tmp/javadoc
> ./geode-core/build/javadoc
> ./geode-core/build/tmp/javadoc
> ./static-analysis/pmd-rules/build/javadoc
> ./static-analysis/pmd-rules/build/tmp/javadoc
> ./geode-protobuf/build/javadoc
> ./geode-protobuf/build/tmp/javadoc
> ./geode-common/build/javadoc
> ./geode-common/build/tmp/javadoc
> ./geode-experimental-driver/build/javadoc
> ./geode-experimental-driver/build/tmp/javadoc
> ./geode-connectors/build/javadoc
> ./geode-connectors/build/tmp/javadoc
> ./geode-old-client-support/build/javadoc
> ./geode-old-client-support/build/tmp/javadoc
> 
> On Wed, Aug 7, 2019 at 10:13 AM Robert Houghton 
> wrote:
> 
>> We added a property to force test reruns specifically, but I can't remember
>> it off hand. More generally, is a gradle option to rerun all tasks (but
>> this takes longer) '--rerun-tasks'.
>> 
>> On Wed, Aug 7, 2019, 10:09 Kirk Lund  wrote:
>> 
>>> I used to use "./gradlew test" to run or rerun unit tests but it no
>> longer
>>> works. It seems to just early out... maybe gradle thinks that the unit
>>> tests were already run so it doesn't need to rerun them.
>>> 
>>> Is this intentional? I assume some gradle changes were made that changed
>>> this behavior.
>>> 
>> 

Re: Proposal to Include GEODE-7079 in 1.10.0

[Murtuza Boxwala]

In this specific case, how long has this issue been in the product?  When did 
we first see it? That would give me a lot more context in gauging the 
“criticality” of this.  Juan, can you share that information?

To Udo’s point, with every change we check in, we add some risk of instability 
or at least unknown behavior. 

I’m a 0 (unable to make a decision), without some more info.

> On Aug 15, 2019, at 2:08 PM, Anthony Baker  wrote:
> 
> While we can’t fix *all known bugs*, I think where we do have a fix for an 
> important issue we should think hard about the cost of not including that in 
> a release.
> 
> IMO, the fixed time approach to releases means that we *start* the release 
> effort (including stabilization and bug fixing if needed) on a known date and 
> we *finish* when new believe the quality of the release branch is sufficient. 
>  Given the number of important fixes being requested, I’m not sure we are 
> there yet.
> 
> I think the release branch concept has merit because it allows us to isolate 
> ongoing work from the changes needed for a release.
> 
> +1 for including GEODE-7079.
> 
> Anthony
> 
> 
>> On Aug 15, 2019, at 10:51 AM, Udo Kohlmeyer  wrote:
>> 
>> Seems everyone is in favor or including a /*non-critical*/ fix to an already 
>> cut branch of the a potential release...
>> 
>> Am I missing something?
>> 
>> Why cut a release at all... just have a perpetual cycle of fixes added to 
>> develop and users can chose what nightly snapshot build they would want to 
>> use..
>> 
>> I'm voting -1 on a non-critical issue, which is existing and worst effect is 
>> to fill logs will NPE logs... (yes, not something we want).
>> 
>> I believed that we (as a Geode community) agreed that once a release has 
>> been cut, only critical issue fixes will be included. If we continue just 
>> continually adding to the ALREADY CUT 1.10 release, where do we stop and 
>> when do we release...
>> 
>> --Udo
>> 
>> On 8/15/19 10:19 AM, Nabarun Nag wrote:
>>> +1
>>> 
>>> On Thu, Aug 15, 2019 at 10:15 AM Alexander Murmann 
>>> wrote:
>>> 
 +1
 
 Agreed to fixing this. It's impossible for a user to discover they hit an
 edge case that we fail to support till they are in prod and restart.
 
 On Thu, Aug 15, 2019 at 10:09 AM Juan José Ramos 
 wrote:
 
> Hello Udo,
> 
> Even if it is an existing issue I'd still consider it critical for those
> cases on which there are unprocessed events on the persistent queue
 after a
> restart and the region takes long to recover... you can actually see
> millions of *NPEs* flooding the member's logs.
> My two cents anyway, it's up to the community to make the final decision.
> Cheers.
> 
> 
> On Thu, Aug 15, 2019 at 5:58 PM Udo Kohlmeyer  wrote:
> 
>> Juan,
>> 
>> From your explanation, it seems this issue is existing and not
>> critical. Could we possibly hold this for 1.11?
>> 
>> --Udo
>> 
>> On 8/15/19 5:29 AM, Ju@N wrote:
>>> Hello team,
>>> 
>>> I'd like to propose including the *fix [1]* for *GEODE-7079 [2]* in
>> release
>>> 1.10.0.
>>> Long story short: a *NullPointerException* can be continuously thrown
>>> and flood the member's logs if a serial event processor (either
>>> *async-event-queue* or *gateway-sender*) starts processing events
 from
> a
>>> recovered persistent queue before the actual region to which it was
>>> attached is fully operational.
>>> Note: *no events are lost (even without the fix)* but, if the region
>> takes
>>> a while to recover, the logs  for the member can grow pretty quickly
> due
>> to
>>> the continuously thrown *NPEs.*
>>> Best regards.
>>> 
>>> [1]:
>>> 
 https://github.com/apache/geode/commit/6f4bbbd96bcecdb82cf7753ce1dae9fa6baebf9b
>>> [2]: https://issues.apache.org/jira/browse/GEODE-7079
>>> 
> 
> --
> Juan José Ramos Cassella
> Senior Software Engineer
> Email: jra...@pivotal.io
> 
> 

Re: No more hardcoded region separators!

[Murtuza Boxwala]

Is there any way to enforce that with some kind of LGTM or spotless rule?

On 5/28/20, 12:46 PM, "Donal Evans"  wrote:

I'm happy to say that as of about 5 minutes ago, there are no uses of 
hardcoded "/" in region paths/names in the geode codebase, as all of them have 
been replaced by the Region.SEPARATOR constant (with the exception of a few 
occurrences in the geode-management module, which while not having an explicit 
dependency on geode-core has implicit dependencies on some things like the 
region separator, index types etc). I'd like to request that going forward, we 
maintain this convention of only using Region.SEPARATOR and avoid introduction 
of any new hardcoded "/" characters in code pertaining to region paths or 
names, both in our own commits and in commits we review from other developers.