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

2018-11-26 Thread Udo Kohlmeyer
Geode-core provides suitable public alternatives, I'm sure Spring Data 
Geode would be able to adapt and use the certified public API's.


--Udo


On 11/26/18 15:48, Dan Smith wrote:

Our wire format and disk format generally does *not* depend on class names
or packages. Internal classes implement DataSerializableFixedID and are
sent using a fixed integer ID. So we should be able to move these classes
around without affecting the on the wire format.

I don't think we should shy away from changing *internal* packages. Worst
case is that we break someone's code and encourage them to ask for a public
API, which is a conversation we should be having anyway.

But to allay some concerns I took a look at what internal classes spring
data geode is using (on the master branch). I think all of these are in
geode-core. So if we leave the packages in geode-core alone, SDG will be
fine.

import org.apache.geode.cache.query.internal.ResultsBag;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.distributed.internal.InternalLocator;
import org.apache.geode.internal.DistributionLocator;
import org.apache.geode.internal.GemFireVersion;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.cache.PoolManagerImpl;
import org.apache.geode.internal.cache.UserSpecifiedRegionAttributes;
import org.apache.geode.internal.concurrent.ConcurrentHashSet;
import org.apache.geode.internal.datasource.ConfigProperty;
import org.apache.geode.internal.datasource.GemFireBasicDataSource;
import org.apache.geode.internal.jndi.JNDIInvoker;
import org.apache.geode.internal.security.shiro.GeodePermissionResolver;
import org.apache.geode.management.internal.cli.domain.RegionInformation;
import
org.apache.geode.management.internal.cli.functions.GetRegionsFunction;
import org.apache.geode.management.internal.security.ResourcePermissions;
import org.apache.geode.pdx.internal.PdxInstanceEnum;
import org.apache.geode.pdx.internal.PdxInstanceFactoryImpl;


On Mon, Nov 26, 2018 at 3:24 PM Kirk Lund  wrote:


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 that's probably a separate project in its own right.

Are you really convinced that we could move internal classes around without
breaking rolling upgrades, client-server backwards compatibility and
diskstore contents?

On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:


I think we can actually fix most of these issues without geode-2.0. Most

of

these are in internal packages, which means we can change the package of
these classes without breaking users. The only concerning one is
org.apache.geode.cache.util, which is a public package. However, the
AutoBalancer is actually still marked @Experimental, so we could
technically move that as well. Or maybe deprecate the old one and it's
associated jar, and create a new jar with a reasonable package. JDK11

users

could just avoid the old jar.

I have been wondering for a while if we should just fold geode-cq and
geode-wan back into geode-core. These are not really properly separated
out, they are very tangled with core. The above package overlap kinda

shows

that as well. But maybe going through the effort of renaming the packages
to make them java 11 modules would help improve the code anyway!

-Dan




On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols 

wrote:

To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
requirement that packages cannot be split across modules.

If we simply map existing Geode jars to Modules, we have about 10 split
packages (see table below).  Any restructuring would potentially have

to

wait until Geode 2.0.

A workaround would be to map existing packages into a small number of

new

modules (e.g. geode-api and geode-internal).  Existing jars would

remain

as-is.  Users making the transition to Java 11 /w Jigsaw already need

to

switch from CLASSPATH to MODULEPATH  so the inconvenience of a

different

naming scheme for Geode-as-modules might be acceptable (however, once
module naming and organization is established, we may be stuck with it

for

a long time).

Thoughts?

Is it even possible to rearrange all classes in each package below

into a

single jar?  Is doing so desirable enough to defer Java 11 support

until

a

yet-unplanned Geode 2.0, or perhaps to drive such a release?

Or, is it satisfactory to fatten Geode releases to include one set of

jars

for CLASSPATH use, plus a different set of jars for MODULEPATH use?


Package
Jar

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 (Geode Statistics archive file).

The StatSampler takes a sample of all statistics (every second by default)
and then invokes every registered SampleHandler. The interface defines 4
methods:

*1) sampled* -- invoked whenever there's a sample taken -- provides a List
of ResourceInstances with latest values
*2) allocatedResourceType* -- invoked when a new ResourceType has been
defined -- this may be caused by enabling a feature or adding a new
instance of a feature or the User may have used the Statistics API to
define a new Statistics ResourceType
*3) allocatedResourceInstance* -- invoked when a new ResourceInstance of a
ResourceType is created
*4) destroyedResourceInstance* -- invoked when a ResourceInstance is
destroyed

A ResourceType is basically a definition of a bucket of key/value stats.
Within Geode, each ResourceType is typically a class such as
DistributionStats. A ResourceInstance is an instance of that with actual
values. One ResourceType may have many ResourceInstances.

The format of a .gfs file closely follows those 4 methods. ResourceType
definition is written to the .gfs file before any sample can contain a
ResourceInstance of that type. A ResourceInstance allocated record is also
written to the .gfs file before any sample may contain any values for that
instance. A ResourceInstance destroyed record is written indicating the end
of lifecycle for a particular instance.

There are two main implementations of SampleHandler:

*A) StatArchiveHandler* -- mechanism that writes binary data to the .gfs
(Geode Statistics File)
*B) StatMonitorHandler* -- mechanism that notifies objects that are clients
to the statistics monitoring API (which is what updates the metrics
attributes on Geode mbeans

The purpose of this design is to allow a developer to build a new
implementation of SampleHandler that writes out to another format or
defines some sort of custom consumer of Statistics samples. For example,
you could define a MicrometerHandler which adapts Geode stats into
Micrometer, or you could define an entirely new custom file format of stats.

By default, the StatSampler samples the statistics every second and there
is only one StatSampler thread, so stat sampling is sensitive to the
performance of any implementation of SampleHandler. For example, you might
not want to define a handler that writes to a RDB over the network.

Performance problems in a SampleHandler could cause gaps in stats values,
delayed updating of Geode mbeans for monitoring tools, and it could cause
someone who is analyzing Geode artifacts to think that a stop-the-world GC
pause has occurred -- the time between stat samples is generally assumed to
be fairly consistent unless a serious GC pause occurs.


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

2018-11-26 Thread Dan Smith
Our wire format and disk format generally does *not* depend on class names
or packages. Internal classes implement DataSerializableFixedID and are
sent using a fixed integer ID. So we should be able to move these classes
around without affecting the on the wire format.

I don't think we should shy away from changing *internal* packages. Worst
case is that we break someone's code and encourage them to ask for a public
API, which is a conversation we should be having anyway.

But to allay some concerns I took a look at what internal classes spring
data geode is using (on the master branch). I think all of these are in
geode-core. So if we leave the packages in geode-core alone, SDG will be
fine.

import org.apache.geode.cache.query.internal.ResultsBag;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.distributed.internal.InternalLocator;
import org.apache.geode.internal.DistributionLocator;
import org.apache.geode.internal.GemFireVersion;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.cache.PoolManagerImpl;
import org.apache.geode.internal.cache.UserSpecifiedRegionAttributes;
import org.apache.geode.internal.concurrent.ConcurrentHashSet;
import org.apache.geode.internal.datasource.ConfigProperty;
import org.apache.geode.internal.datasource.GemFireBasicDataSource;
import org.apache.geode.internal.jndi.JNDIInvoker;
import org.apache.geode.internal.security.shiro.GeodePermissionResolver;
import org.apache.geode.management.internal.cli.domain.RegionInformation;
import
org.apache.geode.management.internal.cli.functions.GetRegionsFunction;
import org.apache.geode.management.internal.security.ResourcePermissions;
import org.apache.geode.pdx.internal.PdxInstanceEnum;
import org.apache.geode.pdx.internal.PdxInstanceFactoryImpl;


On Mon, Nov 26, 2018 at 3:24 PM Kirk Lund  wrote:

> 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 that's probably a separate project in its own right.
>
> Are you really convinced that we could move internal classes around without
> breaking rolling upgrades, client-server backwards compatibility and
> diskstore contents?
>
> On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:
>
> > I think we can actually fix most of these issues without geode-2.0. Most
> of
> > these are in internal packages, which means we can change the package of
> > these classes without breaking users. The only concerning one is
> > org.apache.geode.cache.util, which is a public package. However, the
> > AutoBalancer is actually still marked @Experimental, so we could
> > technically move that as well. Or maybe deprecate the old one and it's
> > associated jar, and create a new jar with a reasonable package. JDK11
> users
> > could just avoid the old jar.
> >
> > I have been wondering for a while if we should just fold geode-cq and
> > geode-wan back into geode-core. These are not really properly separated
> > out, they are very tangled with core. The above package overlap kinda
> shows
> > that as well. But maybe going through the effort of renaming the packages
> > to make them java 11 modules would help improve the code anyway!
> >
> > -Dan
> >
> >
> >
> >
> > On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols 
> wrote:
> >
> > > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > > requirement that packages cannot be split across modules.
> > >
> > > If we simply map existing Geode jars to Modules, we have about 10 split
> > > packages (see table below).  Any restructuring would potentially have
> to
> > > wait until Geode 2.0.
> > >
> > > A workaround would be to map existing packages into a small number of
> new
> > > modules (e.g. geode-api and geode-internal).  Existing jars would
> remain
> > > as-is.  Users making the transition to Java 11 /w Jigsaw already need
> to
> > > switch from CLASSPATH to MODULEPATH  so the inconvenience of a
> different
> > > naming scheme for Geode-as-modules might be acceptable (however, once
> > > module naming and organization is established, we may be stuck with it
> > for
> > > a long time).
> > >
> > > Thoughts?
> > >
> > > Is it even possible to rearrange all classes in each package below
> into a
> > > single jar?  Is doing so desirable enough to defer Java 11 support
> until
> > a
> > > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> > >
> > > Or, is it satisfactory to fatten Geode releases to include one set of
> > jars
> > > for CLASSPATH use, plus a different set of 

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 that's probably a separate project in its own right.

Are you really convinced that we could move internal classes around without
breaking rolling upgrades, client-server backwards compatibility and
diskstore contents?

On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:

> I think we can actually fix most of these issues without geode-2.0. Most of
> these are in internal packages, which means we can change the package of
> these classes without breaking users. The only concerning one is
> org.apache.geode.cache.util, which is a public package. However, the
> AutoBalancer is actually still marked @Experimental, so we could
> technically move that as well. Or maybe deprecate the old one and it's
> associated jar, and create a new jar with a reasonable package. JDK11 users
> could just avoid the old jar.
>
> I have been wondering for a while if we should just fold geode-cq and
> geode-wan back into geode-core. These are not really properly separated
> out, they are very tangled with core. The above package overlap kinda shows
> that as well. But maybe going through the effort of renaming the packages
> to make them java 11 modules would help improve the code anyway!
>
> -Dan
>
>
>
>
> On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols  wrote:
>
> > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > requirement that packages cannot be split across modules.
> >
> > If we simply map existing Geode jars to Modules, we have about 10 split
> > packages (see table below).  Any restructuring would potentially have to
> > wait until Geode 2.0.
> >
> > A workaround would be to map existing packages into a small number of new
> > modules (e.g. geode-api and geode-internal).  Existing jars would remain
> > as-is.  Users making the transition to Java 11 /w Jigsaw already need to
> > switch from CLASSPATH to MODULEPATH  so the inconvenience of a different
> > naming scheme for Geode-as-modules might be acceptable (however, once
> > module naming and organization is established, we may be stuck with it
> for
> > a long time).
> >
> > Thoughts?
> >
> > Is it even possible to rearrange all classes in each package below into a
> > single jar?  Is doing so desirable enough to defer Java 11 support until
> a
> > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> >
> > Or, is it satisfactory to fatten Geode releases to include one set of
> jars
> > for CLASSPATH use, plus a different set of jars for MODULEPATH use?
> >
> >
> > Package
> > Jar
> > org.apache.geode.cache.client.internal
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.client.internal.locator.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.query.internal.cq
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.cache.util
> > geode-core-1.8.0.jar
> > geode-rebalancer-1.8.0.jar
> > org.apache.geode.internal
> > geode-connectors-1.8.0.jar
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-lucene-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.tier.sockets.command
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.internal.cache.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.parallel
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.serial
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.protocol.protobuf.v1
> > geode-protobuf-1.8.0.jar
> > geode-protobuf-messages-1.8.0.jar
>


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

2018-11-26 Thread John Blum
@Galen - If a method is added to a Java interface (e.g. CacheListener),
then classes implementing the interface must define the method, or a
compile error will occur.  This is true in the case where 1) the class
implements CacheListener directly rather than extending CacheListenerAdapter
and 2) where the added method on CacheListener is not a default method.

A few things/thoughts:

1) First, CLASSPATH does not go away, AFAIK.  [1]
2) I am not intimately knowledgeable on (all things) JPMS yet, but a good
interim solution might be to opt for filename-based, "automatic modules" in
Geode.  This perhaps does not solve the split package problem.  But, as I
previously mentioned, I think having "internal" APIs is a problem that
needs to be solved before modularizing.
3) What percentage of users truly really care about JPMS yet? Java 9/10/11
deployments still pale in comparison to the number of Java 8 deployments
and will for the foreseeable future, especially with Oracle's new release
cadence and LTS policies, which many devs don't like.

Food for thought,
-j



[1] https://blog.joda.org/2018/09/from-java-8-to-java-11.html


On Mon, Nov 26, 2018 at 1:14 PM Jacob Barrett  wrote:

> Before we get to far off the intention of this discussion, let's refocus.
> The purpose of the this discussion isn’t to find the “best” modules
> solution but rather to find one that is “good enough” to allow Geode 1.x to
> work in a Java 11 world with a reasonable user experience. While we can run
> the server effectively in Java 11 we cannot consume modules nor be consumed
> as modules by a modularized Java 11 application. Whatever steps we take in
> Geode 1.x will likely be thrown away for a solid solution for 2.x.
>
> So the question is:
> 1) Is refactoring internal to avoid the split packaging issue have a
> significant negative impact on downstream projects that currently use
> internal classes due to deficiencies in our public APIs? If so, what are
> the impacts and can we minimize them in other ways?
>
> 2) Would an uber module be a safer and less negatively impactful solution
> than option 1?
>
> 3) Is there some other interim solution nobody has thrown out yet?
>
> -Jake
>
>

-- 
-John
john.blum10101 (skype)


Re: Using SystemOutRule or SystemErrRule in Geode tests

2018-11-26 Thread Kirk Lund
The following unit test will pass repeatedly in both your IDE (ex: run
until failure) or in stressNewTest:

public class SystemOutRuleTest {

  @Rule
  public SystemOutRule systemOutRule = new SystemOutRule().enableLog();

  @Test
  public void passesRepeatedly() {
System.out.println("hello");

assertThat(systemOutRule.getLog()).contains("hello");
  }
}

But the following test will fail after the first time the test is executed
(ex: run until failure in IntelliJ) :

public class SystemOutRuleTest {

  @Rule
  public SystemOutRule systemOutRule = new SystemOutRule().enableLog();

  @Test
  public void passesFirstTime() {
LogManager.getLogger().info("hello");

assertThat(systemOutRule.getLog()).contains("hello");
  }
}

If you need to verify that a class is logging a specific message in some
circumstances then you can extract that test from the Unit Test and move it
to an Integration Test. You do *not* have to annotate SystemOutRule
with @ClassRule but I recommend doing that especially if you're using other
Rules doing some complicated setUp involving Geode classes:

public class CacheLoggingIntegrationTest {

  private InternalCache cache;

  @Rule
  public SystemOutRule systemOutRule = new SystemOutRule().enableLog();

  @Before
  public void setUp() {
cache = (InternalCache) new CacheFactory().set(LOCATORS, "").create();
  }

  @After
  public void tearDown() {
cache.close();
  }

  @Test
  public void passesFirstTime() {
cache.getLogger().info("hello");

assertThat(systemOutRule.getLog()).contains("hello");
  }
}

If you change the SystemOutRule in CacheLoggingIntegrationTest to a
static @ClassRule:

  @ClassRule
  public static SystemOutRule systemOutRule = new
SystemOutRule().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 26, 2018 at 1:46 PM Kirk Lund  wrote:

> 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 general.
>
> Here's what that means for Geode tests...
>
> If you're writing a Unit Test, then that test will be executed in a
> long-lived JVM in which other Unit Tests are also going to be executed. The
> SystemOutRule from system-rules will work with code that uses System.out
> directly but will not work repeatedly with loggers.
>
> If you're writing an Integration Test or Distributed Test, then that test
> will be executed in a fresh JVM (fork every one is the config), so as long
> as you can get the SystemOutRule before to invoke before logging, the test
> will behave. In general that means you'll want to annotate it
> with @BeforeClass.
>
> Here are a couple examples in which log4j2 captures System.out to
> illustrate why SystemOutRule can't intercept the output after invoking
> System.setOut(PrintStream):
>
> log4j-core/src/main/java/org/apache/logging/log4j/core/config/status/StatusConfiguration.java:43:
>   private static final PrintStream DEFAULT_STREAM = System.out;
>
> log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java:87:
>   ps = System.out;
>
> If SystemOutRule executes its before -- System.setOut(PrintStream) --
> after the above code, logging will be printing to a different PrintStream
> than the PrintStream that SystemOutRule inserted into System.
>


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

Here's what that means for Geode tests...

If you're writing a Unit Test, then that test will be executed in a
long-lived JVM in which other Unit Tests are also going to be executed. The
SystemOutRule from system-rules will work with code that uses System.out
directly but will not work repeatedly with loggers.

If you're writing an Integration Test or Distributed Test, then that test
will be executed in a fresh JVM (fork every one is the config), so as long
as you can get the SystemOutRule before to invoke before logging, the test
will behave. In general that means you'll want to annotate it
with @BeforeClass.

Here are a couple examples in which log4j2 captures System.out to
illustrate why SystemOutRule can't intercept the output after invoking
System.setOut(PrintStream):

log4j-core/src/main/java/org/apache/logging/log4j/core/config/status/StatusConfiguration.java:43:
  private static final PrintStream DEFAULT_STREAM = System.out;

log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java:87:
  ps = System.out;

If SystemOutRule executes its before -- System.setOut(PrintStream) -- after
the above code, logging will be printing to a different PrintStream than
the PrintStream that SystemOutRule inserted into System.


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

2018-11-26 Thread Jacob Barrett
Before we get to far off the intention of this discussion, let's refocus. The 
purpose of the this discussion isn’t to find the “best” modules solution but 
rather to find one that is “good enough” to allow Geode 1.x to work in a Java 
11 world with a reasonable user experience. While we can run the server 
effectively in Java 11 we cannot consume modules nor be consumed as modules by 
a modularized Java 11 application. Whatever steps we take in Geode 1.x will 
likely be thrown away for a solid solution for 2.x.

So the question is:
1) Is refactoring internal to avoid the split packaging issue have a 
significant negative impact on downstream projects that currently use internal 
classes due to deficiencies in our public APIs? If so, what are the impacts and 
can we minimize them in other ways?

2) Would an uber module be a safer and less negatively impactful solution than 
option 1?

3) Is there some other interim solution nobody has thrown out yet?

-Jake



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

2018-11-26 Thread Galen O'Sullivan
>
> If you want to
> maintain an API as internal, then you should properly protect that API with
> the appropriate *Java* access modifiers (private, package-private and
> protected).
>

Hopefully modules will allow us to do this better, by restricting which
*classes* are visible outside the module. However, Java doesn't have a
concept of submodules, which means there's no way to make
org.apache.geode.cache.tier.sockets visible to org.apache.cache or
org.apache.cache.tier without also being visible to everything. We can't
make InternalCache (for example) only visible to our own code without
declaring it package-private and putting (effectively) all of Geode in the
same package.

Adding a method to
> an interface, is an API breaking change.
>
I don't agree with this. If a new method is added, old API works just fine,
and unless I'm mistaken (and I may be), any code that used the old
interface should be able to use the new interface without issues.

Galen

On Mon, Nov 26, 2018 at 12:13 PM John Blum  wrote:

> *> Most of these are in internal packages, which means we can change the
> package of these classes without breaking users.*
>
> I don't agree with this.
>
> Some functionality in Geode required by an application, or framework, is
> only available in an "internal" package, unfortunately.  This has to do
> with the fact that many interfaces in Geode were not well designed.
>
> In general, and as a general rule of thumb, anytime you change a method
> name or method signature, it is an API breaking change.  Adding a method to
> an interface, is an API breaking change.  Removing methods from an
> interface is (potentially) an API breaking change.  Changing a type is an
> API breaking change.  There are several other examples.  Any class/method
> (dare I say field of a class) that is not "properly protected" is part of
> your API, regardless if we deem it be "internal" only or not, and
> (possibly) will affect users.
>
> I think any level of modularity (and maturing) is pointless without 1)
> first, having well defined contracts/interfaces between functional areas
> (based on concerns) and 2) using appropriate access modifiers and cleanly
> delineating APIs/SPIs from implementations/providers.
>
> -j
>
>
>
> On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:
>
> > I think we can actually fix most of these issues without geode-2.0. Most
> of
> > these are in internal packages, which means we can change the package of
> > these classes without breaking users. The only concerning one is
> > org.apache.geode.cache.util, which is a public package. However, the
> > AutoBalancer is actually still marked @Experimental, so we could
> > technically move that as well. Or maybe deprecate the old one and it's
> > associated jar, and create a new jar with a reasonable package. JDK11
> users
> > could just avoid the old jar.
> >
> > I have been wondering for a while if we should just fold geode-cq and
> > geode-wan back into geode-core. These are not really properly separated
> > out, they are very tangled with core. The above package overlap kinda
> shows
> > that as well. But maybe going through the effort of renaming the packages
> > to make them java 11 modules would help improve the code anyway!
> >
> > -Dan
> >
> >
> >
> >
> > On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols 
> wrote:
> >
> > > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > > requirement that packages cannot be split across modules.
> > >
> > > If we simply map existing Geode jars to Modules, we have about 10 split
> > > packages (see table below).  Any restructuring would potentially have
> to
> > > wait until Geode 2.0.
> > >
> > > A workaround would be to map existing packages into a small number of
> new
> > > modules (e.g. geode-api and geode-internal).  Existing jars would
> remain
> > > as-is.  Users making the transition to Java 11 /w Jigsaw already need
> to
> > > switch from CLASSPATH to MODULEPATH  so the inconvenience of a
> different
> > > naming scheme for Geode-as-modules might be acceptable (however, once
> > > module naming and organization is established, we may be stuck with it
> > for
> > > a long time).
> > >
> > > Thoughts?
> > >
> > > Is it even possible to rearrange all classes in each package below
> into a
> > > single jar?  Is doing so desirable enough to defer Java 11 support
> until
> > a
> > > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> > >
> > > Or, is it satisfactory to fatten Geode releases to include one set of
> > jars
> > > for CLASSPATH use, plus a different set of jars for MODULEPATH use?
> > >
> > >
> > > Package
> > > Jar
> > > org.apache.geode.cache.client.internal
> > > geode-core-1.8.0.jar
> > > geode-cq-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.cache.client.internal.locator.wan
> > > geode-core-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.cache.query.internal.cq
> > > geode-core-1.8.0.jar
> > > geode-cq-1.8.0.jar
> > > 

Re: Adding index for Like operartor

2018-11-26 Thread Jason Huynh
Key indexes tell the query engine that the field is the same as the key
that was used to put into the underlying region.  So let's say your field
is "ID".  Creating a key index tells the query engine that a "select * from
/region theRegion where ID = 1" is the same as doing : theRegion.get(1).

In this case I think you are looking for a functional index.  Those are
sorted and will be used for like queries.



On Mon, Nov 26, 2018 at 12:52 PM Alexander Murmann 
wrote:

> Hi,
>
> Have you thought about using the built-in Lucene functionality for this?
> Fuzzy text search is exactly what it's made for.
>
> On Mon, Nov 26, 2018 at 12:15 PM anjana_nair 
> wrote:
>
> >
> > Hi,
> >
> > I would like to add an index to key (keyindex) and the query executed is
> > going to be like
> >
> > "constantstring%"  where constant string is  going to be  the constant
> > prefix in a lile query. In this case  can we just go ahead with key index
> > or
> > should we use a function index ?  key indexes are not sorted ?
> >
> >
> >
> > --
> > Sent from:
> > http://apache-geode-incubating-developers-forum.70738.x6.nabble.com/
> >
>


Re: Adding index for Like operartor

2018-11-26 Thread Alexander Murmann
Hi,

Have you thought about using the built-in Lucene functionality for this?
Fuzzy text search is exactly what it's made for.

On Mon, Nov 26, 2018 at 12:15 PM anjana_nair 
wrote:

>
> Hi,
>
> I would like to add an index to key (keyindex) and the query executed is
> going to be like
>
> "constantstring%"  where constant string is  going to be  the constant
> prefix in a lile query. In this case  can we just go ahead with key index
> or
> should we use a function index ?  key indexes are not sorted ?
>
>
>
> --
> Sent from:
> http://apache-geode-incubating-developers-forum.70738.x6.nabble.com/
>


Re: AcceptanceTestOpenJDK11 precheckin fails to compile

2018-11-26 Thread Patrick Rhomberg
I believe that the   unable to access file: corrupted zip file   indicates
some transient failure in GCP?  I remember seeing it in the past, but I
don't think we ever got to the bottom of it.

To respond to your other email, yeah, I think bumping your PR to re-fire
might be the best way to proceed?  As it stands, since the build failed,
none of the JDK11 tests would've run, which mildly worries me, even if the
build failure is understood.

We could look into separating the build and test tasks in Concourse, so
that the build step would retry on these failures.  I'm not sure if it
would end up caching things in a way that would cause us to not actually
ameliorate the issue, though.

tl;dr: I think you just rolled snake-eyes.

Imagination is Change.
~Patrick



On Mon, Nov 26, 2018 at 12:20 PM Kirk Lund  wrote:

> 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 java.lang.reflect.Constructor;
> ^
>   bad class file:
>
> /usr/lib/jvm/java-8-openjdk-amd64/lib/ct.sym(META-INF/sym/rt.jar/java/lang/reflect/Constructor.class)
> unable to access file: corrupted zip file
> Please remove or make sure it appears in the correct subdirectory of
> the classpath.
>
> /home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java:18:
> error: cannot find symbol
> import static
>
> org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ENTRY_EVENT_NEW_VALUE;
> ^
>   symbol:   static ENTRY_EVENT_NEW_VALUE
>   location: class
>
> /home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java:17:
> error: cannot find symbol
> import static org.apache.geode.internal.lang.SystemUtils.getLineSeparator;
> ^
>   symbol:   static getLineSeparator
>   location: class
>


Re: AcceptanceTestOpenJDK11 precheckin fails to compile

2018-11-26 Thread Jacob Barrett
That’s really odd because that jar is baked into the build image. If it was 
truly corrupt then all builds should be failing. 

Hmmm


> On Nov 26, 2018, at 12:19 PM, Kirk Lund  wrote:
> 
> 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 java.lang.reflect.Constructor;
>^
>  bad class file:
> /usr/lib/jvm/java-8-openjdk-amd64/lib/ct.sym(META-INF/sym/rt.jar/java/lang/reflect/Constructor.class)
>unable to access file: corrupted zip file
>Please remove or make sure it appears in the correct subdirectory of
> the classpath.
> /home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java:18:
> error: cannot find symbol
> import static
> org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ENTRY_EVENT_NEW_VALUE;
> ^
>  symbol:   static ENTRY_EVENT_NEW_VALUE
>  location: class
> /home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java:17:
> error: cannot find symbol
> import static org.apache.geode.internal.lang.SystemUtils.getLineSeparator;
> ^
>  symbol:   static getLineSeparator
>  location: class


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
integrationTest and distributedTest targets still need the
geode-core/src/main/resources/log4j2.xml earliest on the classpath.

The best solution is probably to hide
geode-core/src/main/resources/log4j2.xml from unit test if that's possible.

Second best solution might be to have a resources src set containing a
simplified log4j2.xml which is added to the classpath for unit test but not
for the other testing targets.

Anyone with gradle knowledge interested in helping with this?


Re: New Geode PMC Chair: Karen Miller

2018-11-26 Thread Dave Barnes
Thanks, Mark - Welcome Karen!

On Mon, Nov 26, 2018 at 12:17 PM Dan Smith  wrote:

> Congratulations Karen! And thanks for all of your hard work, Mark!
>
> -Dan
>
> On Mon, Nov 26, 2018 at 11:56 AM Mark Bretl  wrote:
>
> > Hello Geode Community,
> >
> > After two years, the Project Management Committee (PMC) agreed to
> > transition the role of PMC chair from me to another PMC member. I would
> > like to announce, at last week's Apache Board Meeting, the Board approved
> > the Geode PMC's recommendation of Karen Miller to the office of V.P.,
> > Apache Geode.
> >
> > Please welcome our new PMC Chair, Karen Miller!
> >
> > Best regards,
> >
> > --Mark
> >
>


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 java.lang.reflect.Constructor;
^
  bad class file:
/usr/lib/jvm/java-8-openjdk-amd64/lib/ct.sym(META-INF/sym/rt.jar/java/lang/reflect/Constructor.class)
unable to access file: corrupted zip file
Please remove or make sure it appears in the correct subdirectory of
the classpath.
/home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java:18:
error: cannot find symbol
import static
org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ENTRY_EVENT_NEW_VALUE;
^
  symbol:   static ENTRY_EVENT_NEW_VALUE
  location: class
/home/geode/geode/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java:17:
error: cannot find symbol
import static org.apache.geode.internal.lang.SystemUtils.getLineSeparator;
^
  symbol:   static getLineSeparator
  location: class


Re: New Geode PMC Chair: Karen Miller

2018-11-26 Thread Dan Smith
Congratulations Karen! And thanks for all of your hard work, Mark!

-Dan

On Mon, Nov 26, 2018 at 11:56 AM Mark Bretl  wrote:

> Hello Geode Community,
>
> After two years, the Project Management Committee (PMC) agreed to
> transition the role of PMC chair from me to another PMC member. I would
> like to announce, at last week's Apache Board Meeting, the Board approved
> the Geode PMC's recommendation of Karen Miller to the office of V.P.,
> Apache Geode.
>
> Please welcome our new PMC Chair, Karen Miller!
>
> Best regards,
>
> --Mark
>


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

2018-11-26 Thread John Blum
*> Most of these are in internal packages, which means we can change the
package of these classes without breaking users.*

I don't agree with this.

Some functionality in Geode required by an application, or framework, is
only available in an "internal" package, unfortunately.  This has to do
with the fact that many interfaces in Geode were not well designed.

In general, and as a general rule of thumb, anytime you change a method
name or method signature, it is an API breaking change.  Adding a method to
an interface, is an API breaking change.  Removing methods from an
interface is (potentially) an API breaking change.  Changing a type is an
API breaking change.  There are several other examples.  Any class/method
(dare I say field of a class) that is not "properly protected" is part of
your API, regardless if we deem it be "internal" only or not, and
(possibly) will affect users.

I think any level of modularity (and maturing) is pointless without 1)
first, having well defined contracts/interfaces between functional areas
(based on concerns) and 2) using appropriate access modifiers and cleanly
delineating APIs/SPIs from implementations/providers.

-j



On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:

> I think we can actually fix most of these issues without geode-2.0. Most of
> these are in internal packages, which means we can change the package of
> these classes without breaking users. The only concerning one is
> org.apache.geode.cache.util, which is a public package. However, the
> AutoBalancer is actually still marked @Experimental, so we could
> technically move that as well. Or maybe deprecate the old one and it's
> associated jar, and create a new jar with a reasonable package. JDK11 users
> could just avoid the old jar.
>
> I have been wondering for a while if we should just fold geode-cq and
> geode-wan back into geode-core. These are not really properly separated
> out, they are very tangled with core. The above package overlap kinda shows
> that as well. But maybe going through the effort of renaming the packages
> to make them java 11 modules would help improve the code anyway!
>
> -Dan
>
>
>
>
> On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols  wrote:
>
> > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > requirement that packages cannot be split across modules.
> >
> > If we simply map existing Geode jars to Modules, we have about 10 split
> > packages (see table below).  Any restructuring would potentially have to
> > wait until Geode 2.0.
> >
> > A workaround would be to map existing packages into a small number of new
> > modules (e.g. geode-api and geode-internal).  Existing jars would remain
> > as-is.  Users making the transition to Java 11 /w Jigsaw already need to
> > switch from CLASSPATH to MODULEPATH  so the inconvenience of a different
> > naming scheme for Geode-as-modules might be acceptable (however, once
> > module naming and organization is established, we may be stuck with it
> for
> > a long time).
> >
> > Thoughts?
> >
> > Is it even possible to rearrange all classes in each package below into a
> > single jar?  Is doing so desirable enough to defer Java 11 support until
> a
> > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> >
> > Or, is it satisfactory to fatten Geode releases to include one set of
> jars
> > for CLASSPATH use, plus a different set of jars for MODULEPATH use?
> >
> >
> > Package
> > Jar
> > org.apache.geode.cache.client.internal
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.client.internal.locator.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.query.internal.cq
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.cache.util
> > geode-core-1.8.0.jar
> > geode-rebalancer-1.8.0.jar
> > org.apache.geode.internal
> > geode-connectors-1.8.0.jar
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-lucene-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.tier.sockets.command
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.internal.cache.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.parallel
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.serial
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.protocol.protobuf.v1
> > geode-protobuf-1.8.0.jar
> > geode-protobuf-messages-1.8.0.jar
>


-- 
-John
john.blum10101 (skype)


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

2018-11-26 Thread John Blum
I need more time to think about this clearly (currently juggling multiple
things), but I would say this...

I don't think there should be "internal" APIs, as in o.a.g.*.internal.
There are only APIs/SPIs and implementations, period.  If you want to
maintain an API as internal, then you should properly protect that API with
the appropriate *Java* access modifiers (private, package-private and
protected).

-j


On Mon, Nov 26, 2018 at 11:29 AM Jacob Barrett  wrote:

> On Mon, Nov 26, 2018 at 11:23 AM Udo Kohlmeyer 
> wrote:
>
> > Imo, no module should export "internal". Maybe a stretch goal, but
> > something we should keep in mind.
> >
>
> Yes, agreed, but outside the scope of this current discussion.
>


-- 
-John
john.blum10101 (skype)


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 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 do not care
> about ordering of invocation, so you do *not* need to use RuleChain
> (unless you're using CleanupDUnitVMsRule). There are two caveats though:
>
> 1) if you want or need to specify the number of VMs to be something other
> than the default of 4, then you'll need to specify the number of VMs for
> every one of these dunit rules (see example below)
>
> 2) these dunit rules do not currently work with bouncing of vms
>
> If you need to use CleanupDUnitVMsRule along with other dunit rules, then
> you will need to use. If you need to use VM bouncing within a test, then
> you'll need to wait until I can merge my
> (I have a branch on which I've made some changes to make this work)
>
> So it's actually #2 that's causing your problem. But as I mentioned, I do
> have a branch on which it does now work with bouncing of VMs but I'm not
> quite ready to merge it.
>
> Here's an example for a test that wants to limit the number of dunit VMs
> to 2. RuleChain is not needed, but you do have to specify the # of dunit
> VMs on all 3 dunit rules:
>
>   @Rule
>   public DistributedRule distributedRule = new DistributedRule(2);
>
>   @Rule
>   public SharedCountersRule sharedCountersRule = new SharedCountersRule(2);
>
>   @Rule
>   public SharedErrorCollector errorCollector = new SharedErrorCollector(2);
>
> Please don't use ManagementTestRule at all. I'm trying to modify all tests
> that use it to use DistributedRule with Geode User APIs so that I can
> delete it.
>
> All of those other dunit rules in your list extend AbstractDistributedRule
> which is why rule ordering does not matter (unless you use
> CleanupDUnitVMsRule because it bounces VMs).
>
> Thanks,
> Kirk
>
> On Wed, Nov 21, 2018 at 10:34 AM Patrick Rhomberg 
> wrote:
>
>> tl;dr: Use JUnit RuleChain.
>> 
>>
>> Hello all!
>>
>> Several [1] of our test @Rule classes make use of the fact that our DUnit
>> VMs Host is statically accessible to affect every test VM.  For instance,
>> the SharedCountersRule initializes a counter in every VM, and the
>> CleanupDUnitVMsRule bounces VMs before and after each test.
>>
>> Problematically, JUnit rules applied in an unpredictable / JVM-dependent
>> ordering. [2]  As a result, some flakiness we find in our tests may be the
>> result of unexpected interaction and ordering of our test rules. [3]
>>
>> Fortunately, a solution to this problem already exists.  Rule ordering can
>> be imposed by JUnit's RuleChain. [4]
>>
>> In early exploration with this rule, some tests failed due to the
>> RuleChain
>> not being serializable.  However, as it should only apply to the test VM,
>> and given that it can be composed of (unannotated) rules that remain
>> accessible and serializable, it should be a simple matter of declaring the
>> offending field transient, as it will only be necessary in the test VM.
>>
>> So, you dear reader: while you're out there making Geode the best it can
>> be, if you find yourself in a test class that uses more than one rule
>> listed in [1], or if you notice some other rule not listed below that
>> reaches out to VMs as part of its @Before or @After, please update that
>> test to use the RuleChain to apply the rules in a predictable order.
>>
>> Imagination is Change.
>> ~Patrick
>>
>> [1] A probably-incomplete list of invasive rules can be found via
>> $> git grep -il inEveryVM | grep Rule.java
>>
>> geode-core/src/distributedTest/java/org/apache/geode/management/ManagementTestRule.java
>> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/CacheRule.java
>>
>> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClientCacheRule.java
>>
>> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedDiskDirRule.java
>>
>> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedRule.java
>>
>> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedUseJacksonForJsonPathRule.java
>>
>> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/SharedCountersRule.java
>>
>> [2] See the documentation for rules here:
>> https://junit.org/junit4/javadoc/4.12/org/junit/Rule.html ; notably,
>> "However,
>> if there are multiple [Rule] fields (or methods) they will be applied in
>> an
>> order that depends on your JVM's implementation of the reflection API,
>> which is undefined, in general."
>>
>> [3] For what it's worth, this was discovered after looking into why the
>> DistributedRule check 

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 do not care
about ordering of invocation, so you do *not* need to use RuleChain (unless
you're using CleanupDUnitVMsRule). There are two caveats though:

1) if you want or need to specify the number of VMs to be something other
than the default of 4, then you'll need to specify the number of VMs for
every one of these dunit rules (see example below)

2) these dunit rules do not currently work with bouncing of vms

If you need to use CleanupDUnitVMsRule along with other dunit rules, then
you will need to use. If you need to use VM bouncing within a test, then
you'll need to wait until I can merge my
(I have a branch on which I've made some changes to make this work)

So it's actually #2 that's causing your problem. But as I mentioned, I do
have a branch on which it does now work with bouncing of VMs but I'm not
quite ready to merge it.

Here's an example for a test that wants to limit the number of dunit VMs to
2. RuleChain is not needed, but you do have to specify the # of dunit VMs
on all 3 dunit rules:

  @Rule
  public DistributedRule distributedRule = new DistributedRule(2);

  @Rule
  public SharedCountersRule sharedCountersRule = new SharedCountersRule(2);

  @Rule
  public SharedErrorCollector errorCollector = new SharedErrorCollector(2);

Please don't use ManagementTestRule at all. I'm trying to modify all tests
that use it to use DistributedRule with Geode User APIs so that I can
delete it.

All of those other dunit rules in your list extend AbstractDistributedRule
which is why rule ordering does not matter (unless you use
CleanupDUnitVMsRule because it bounces VMs).

Thanks,
Kirk

On Wed, Nov 21, 2018 at 10:34 AM Patrick Rhomberg 
wrote:

> tl;dr: Use JUnit RuleChain.
> 
>
> Hello all!
>
> Several [1] of our test @Rule classes make use of the fact that our DUnit
> VMs Host is statically accessible to affect every test VM.  For instance,
> the SharedCountersRule initializes a counter in every VM, and the
> CleanupDUnitVMsRule bounces VMs before and after each test.
>
> Problematically, JUnit rules applied in an unpredictable / JVM-dependent
> ordering. [2]  As a result, some flakiness we find in our tests may be the
> result of unexpected interaction and ordering of our test rules. [3]
>
> Fortunately, a solution to this problem already exists.  Rule ordering can
> be imposed by JUnit's RuleChain. [4]
>
> In early exploration with this rule, some tests failed due to the RuleChain
> not being serializable.  However, as it should only apply to the test VM,
> and given that it can be composed of (unannotated) rules that remain
> accessible and serializable, it should be a simple matter of declaring the
> offending field transient, as it will only be necessary in the test VM.
>
> So, you dear reader: while you're out there making Geode the best it can
> be, if you find yourself in a test class that uses more than one rule
> listed in [1], or if you notice some other rule not listed below that
> reaches out to VMs as part of its @Before or @After, please update that
> test to use the RuleChain to apply the rules in a predictable order.
>
> Imagination is Change.
> ~Patrick
>
> [1] A probably-incomplete list of invasive rules can be found via
> $> git grep -il inEveryVM | grep Rule.java
>
> geode-core/src/distributedTest/java/org/apache/geode/management/ManagementTestRule.java
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/CacheRule.java
>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClientCacheRule.java
>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedDiskDirRule.java
>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedRule.java
>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedUseJacksonForJsonPathRule.java
>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/SharedCountersRule.java
>
> [2] See the documentation for rules here:
> https://junit.org/junit4/javadoc/4.12/org/junit/Rule.html ; notably,
> "However,
> if there are multiple [Rule] fields (or methods) they will be applied in an
> order that depends on your JVM's implementation of the reflection API,
> which is undefined, in general."
>
> [3] For what it's worth, this was discovered after looking into why the
> DistributedRule check fo suspicious strings caused the test *after* the
> test that emitted the strings to fail.  It's only tangentially related, but
> got me looking into when and how the @After was getting applied.
>
> [4] https://junit.org/junit4/javadoc/4.12/org/junit/rules/RuleChain.html
>


New Geode PMC Chair: Karen Miller

2018-11-26 Thread Mark Bretl
Hello Geode Community,

After two years, the Project Management Committee (PMC) agreed to
transition the role of PMC chair from me to another PMC member. I would
like to announce, at last week's Apache Board Meeting, the Board approved
the Geode PMC's recommendation of Karen Miller to the office of V.P.,
Apache Geode.

Please welcome our new PMC Chair, Karen Miller!

Best regards,

--Mark


Adding index for Like operartor

2018-11-26 Thread anjana_nair


Hi,

I would like to add an index to key (keyindex) and the query executed is
going to be like

"constantstring%"  where constant string is  going to be  the constant
prefix in a lile query. In this case  can we just go ahead with key index or
should we use a function index ?  key indexes are not sorted ?



--
Sent from: http://apache-geode-incubating-developers-forum.70738.x6.nabble.com/


Re: Avoiding Unpredictability in Our DUnit Testing Rules

2018-11-26 Thread Mark Hanson
Thanks for the correction Bruce!

> On Nov 26, 2018, at 10:04 AM, Bruce Schuchardt  wrote:
> 
> While it's common to use VM.getVM(int) to get a VM you still must use 
> Host.getHost() to get a VM running an older version of Geode. Until someone 
> adds a VM.getVM(version, vmNumber) we can't frown upon 
> Host.getHost(0).getVM(startingVersion, 0).
> 
> On 11/21/18 4:09 PM, Mark Hanson wrote:
>> It is frowned upon.  VM.getVM(0) is now the accepted way to get VM 0.
>> 
>> Thanks,
>> Mark
>> 
>> 
>>> On Nov 21, 2018, at 10:41 AM, Nabarun Nag  wrote:
>>> 
>>> Will doing this  in the test,
>>> 
>>> final Host host = Host.getHost(0);
>>> VM server1 = host.getVM(startingVersion, 0);
>>> 
>>> be frowned upon, if I use the above over using @Rule.
>>> 
>>> Regards
>>> Nabarun Nag
>>> 
 On Nov 21, 2018, at 10:36 AM, Robert Houghton  wrote:
 
 Great find, Patrick. I hope this shakes out some of the test bugs!
 
 On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg >>> 
> tl;dr: Use JUnit RuleChain.
> 
> 
> Hello all!
> 
> Several [1] of our test @Rule classes make use of the fact that our DUnit
> VMs Host is statically accessible to affect every test VM.  For instance,
> the SharedCountersRule initializes a counter in every VM, and the
> CleanupDUnitVMsRule bounces VMs before and after each test.
> 
> Problematically, JUnit rules applied in an unpredictable / JVM-dependent
> ordering. [2]  As a result, some flakiness we find in our tests may be the
> result of unexpected interaction and ordering of our test rules. [3]
> 
> Fortunately, a solution to this problem already exists.  Rule ordering can
> be imposed by JUnit's RuleChain. [4]
> 
> In early exploration with this rule, some tests failed due to the 
> RuleChain
> not being serializable.  However, as it should only apply to the test VM,
> and given that it can be composed of (unannotated) rules that remain
> accessible and serializable, it should be a simple matter of declaring the
> offending field transient, as it will only be necessary in the test VM.
> 
> So, you dear reader: while you're out there making Geode the best it can
> be, if you find yourself in a test class that uses more than one rule
> listed in [1], or if you notice some other rule not listed below that
> reaches out to VMs as part of its @Before or @After, please update that
> test to use the RuleChain to apply the rules in a predictable order.
> 
> Imagination is Change.
> ~Patrick
> 
> [1] A probably-incomplete list of invasive rules can be found via
> $> git grep -il inEveryVM | grep Rule.java
> 
> geode-core/src/distributedTest/java/org/apache/geode/management/ManagementTestRule.java
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/CacheRule.java
> 
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClientCacheRule.java
> 
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedDiskDirRule.java
> 
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedRule.java
> 
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedUseJacksonForJsonPathRule.java
> 
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/SharedCountersRule.java
> 
> [2] See the documentation for rules here:
> https://junit.org/junit4/javadoc/4.12/org/junit/Rule.html ; notably,
> "However,
> if there are multiple [Rule] fields (or methods) they will be applied in 
> an
> order that depends on your JVM's implementation of the reflection API,
> which is undefined, in general."
> 
> [3] For what it's worth, this was discovered after looking into why the
> DistributedRule check fo suspicious strings caused the test *after* the
> test that emitted the strings to fail.  It's only tangentially related, 
> but
> got me looking into when and how the @After was getting applied.
> 
> [4] https://junit.org/junit4/javadoc/4.12/org/junit/rules/RuleChain.html
> 



Re: JIRA write access?

2018-11-26 Thread Dan Smith
Done!

-Dan

On Mon, Nov 26, 2018 at 11:15 AM Sean Goller  wrote:

> Hi, In order to start dealing with some JIRA tickets could I get assign and
> resolve access?
> My apache username is smgoller.
>
> Thanks!
>
> -Sean.
>


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

2018-11-26 Thread Jacob Barrett
On Mon, Nov 26, 2018 at 11:23 AM Udo Kohlmeyer 
wrote:

> Imo, no module should export "internal". Maybe a stretch goal, but
> something we should keep in mind.
>

Yes, agreed, but outside the scope of this current discussion.


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

2018-11-26 Thread Udo Kohlmeyer
Imo, no module should export "internal". Maybe a stretch goal, but 
something we should keep in mind.



On 11/26/18 10:50, Jacob Barrett wrote:

One lingering question I have around jigsaw is the split package issue
recursive? In that I mean if a module exports "org.apache.geode.internal"
and another module exports "org.apache.geode.internal.foo" is this legal?

On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:


I think we can actually fix most of these issues without geode-2.0. Most of
these are in internal packages, which means we can change the package of
these classes without breaking users. The only concerning one is
org.apache.geode.cache.util, which is a public package. However, the
AutoBalancer is actually still marked @Experimental, so we could
technically move that as well. Or maybe deprecate the old one and it's
associated jar, and create a new jar with a reasonable package. JDK11 users
could just avoid the old jar.

I have been wondering for a while if we should just fold geode-cq and
geode-wan back into geode-core. These are not really properly separated
out, they are very tangled with core. The above package overlap kinda shows
that as well. But maybe going through the effort of renaming the packages
to make them java 11 modules would help improve the code anyway!

-Dan




On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols  wrote:


To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
requirement that packages cannot be split across modules.

If we simply map existing Geode jars to Modules, we have about 10 split
packages (see table below).  Any restructuring would potentially have to
wait until Geode 2.0.

A workaround would be to map existing packages into a small number of new
modules (e.g. geode-api and geode-internal).  Existing jars would remain
as-is.  Users making the transition to Java 11 /w Jigsaw already need to
switch from CLASSPATH to MODULEPATH  so the inconvenience of a different
naming scheme for Geode-as-modules might be acceptable (however, once
module naming and organization is established, we may be stuck with it

for

a long time).

Thoughts?

Is it even possible to rearrange all classes in each package below into a
single jar?  Is doing so desirable enough to defer Java 11 support until

a

yet-unplanned Geode 2.0, or perhaps to drive such a release?

Or, is it satisfactory to fatten Geode releases to include one set of

jars

for CLASSPATH use, plus a different set of jars for MODULEPATH use?


Package
Jar
org.apache.geode.cache.client.internal
geode-core-1.8.0.jar
geode-cq-1.8.0.jar
geode-wan-1.8.0.jar
org.apache.geode.cache.client.internal.locator.wan
geode-core-1.8.0.jar
geode-wan-1.8.0.jar
org.apache.geode.cache.query.internal.cq
geode-core-1.8.0.jar
geode-cq-1.8.0.jar
org.apache.geode.cache.util
geode-core-1.8.0.jar
geode-rebalancer-1.8.0.jar
org.apache.geode.internal
geode-connectors-1.8.0.jar
geode-core-1.8.0.jar
geode-cq-1.8.0.jar
geode-lucene-1.8.0.jar
geode-wan-1.8.0.jar
org.apache.geode.internal.cache.tier.sockets.command
geode-core-1.8.0.jar
geode-cq-1.8.0.jar
org.apache.geode.internal.cache.wan
geode-core-1.8.0.jar
geode-wan-1.8.0.jar
org.apache.geode.internal.cache.wan.parallel
geode-core-1.8.0.jar
geode-wan-1.8.0.jar
org.apache.geode.internal.cache.wan.serial
geode-core-1.8.0.jar
geode-wan-1.8.0.jar
org.apache.geode.internal.protocol.protobuf.v1
geode-protobuf-1.8.0.jar
geode-protobuf-messages-1.8.0.jar




JIRA write access?

2018-11-26 Thread Sean Goller
Hi, In order to start dealing with some JIRA tickets could I get assign and
resolve access?
My apache username is smgoller.

Thanks!

-Sean.


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

2018-11-26 Thread Jacob Barrett
If Dan's assertions are correct, and we can in fact refactor all these
packages without API changes, then I am all for giving it a shot. Worst
case we have done a little of the work we will have to do for a 2.0 api
change anyway and have to revert to the idea of an uber jar.

On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:

> I think we can actually fix most of these issues without geode-2.0. Most of
> these are in internal packages, which means we can change the package of
> these classes without breaking users. The only concerning one is
> org.apache.geode.cache.util, which is a public package. However, the
> AutoBalancer is actually still marked @Experimental, so we could
> technically move that as well. Or maybe deprecate the old one and it's
> associated jar, and create a new jar with a reasonable package. JDK11 users
> could just avoid the old jar.
>
> I have been wondering for a while if we should just fold geode-cq and
> geode-wan back into geode-core. These are not really properly separated
> out, they are very tangled with core. The above package overlap kinda shows
> that as well. But maybe going through the effort of renaming the packages
> to make them java 11 modules would help improve the code anyway!
>
> -Dan
>
>
>
>
> On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols  wrote:
>
> > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > requirement that packages cannot be split across modules.
> >
> > If we simply map existing Geode jars to Modules, we have about 10 split
> > packages (see table below).  Any restructuring would potentially have to
> > wait until Geode 2.0.
> >
> > A workaround would be to map existing packages into a small number of new
> > modules (e.g. geode-api and geode-internal).  Existing jars would remain
> > as-is.  Users making the transition to Java 11 /w Jigsaw already need to
> > switch from CLASSPATH to MODULEPATH  so the inconvenience of a different
> > naming scheme for Geode-as-modules might be acceptable (however, once
> > module naming and organization is established, we may be stuck with it
> for
> > a long time).
> >
> > Thoughts?
> >
> > Is it even possible to rearrange all classes in each package below into a
> > single jar?  Is doing so desirable enough to defer Java 11 support until
> a
> > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> >
> > Or, is it satisfactory to fatten Geode releases to include one set of
> jars
> > for CLASSPATH use, plus a different set of jars for MODULEPATH use?
> >
> >
> > Package
> > Jar
> > org.apache.geode.cache.client.internal
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.client.internal.locator.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.query.internal.cq
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.cache.util
> > geode-core-1.8.0.jar
> > geode-rebalancer-1.8.0.jar
> > org.apache.geode.internal
> > geode-connectors-1.8.0.jar
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-lucene-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.tier.sockets.command
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.internal.cache.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.parallel
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.serial
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.protocol.protobuf.v1
> > geode-protobuf-1.8.0.jar
> > geode-protobuf-messages-1.8.0.jar
>


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

2018-11-26 Thread Jacob Barrett
One lingering question I have around jigsaw is the split package issue
recursive? In that I mean if a module exports "org.apache.geode.internal"
and another module exports "org.apache.geode.internal.foo" is this legal?

On Mon, Nov 19, 2018 at 5:21 PM Dan Smith  wrote:

> I think we can actually fix most of these issues without geode-2.0. Most of
> these are in internal packages, which means we can change the package of
> these classes without breaking users. The only concerning one is
> org.apache.geode.cache.util, which is a public package. However, the
> AutoBalancer is actually still marked @Experimental, so we could
> technically move that as well. Or maybe deprecate the old one and it's
> associated jar, and create a new jar with a reasonable package. JDK11 users
> could just avoid the old jar.
>
> I have been wondering for a while if we should just fold geode-cq and
> geode-wan back into geode-core. These are not really properly separated
> out, they are very tangled with core. The above package overlap kinda shows
> that as well. But maybe going through the effort of renaming the packages
> to make them java 11 modules would help improve the code anyway!
>
> -Dan
>
>
>
>
> On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols  wrote:
>
> > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > requirement that packages cannot be split across modules.
> >
> > If we simply map existing Geode jars to Modules, we have about 10 split
> > packages (see table below).  Any restructuring would potentially have to
> > wait until Geode 2.0.
> >
> > A workaround would be to map existing packages into a small number of new
> > modules (e.g. geode-api and geode-internal).  Existing jars would remain
> > as-is.  Users making the transition to Java 11 /w Jigsaw already need to
> > switch from CLASSPATH to MODULEPATH  so the inconvenience of a different
> > naming scheme for Geode-as-modules might be acceptable (however, once
> > module naming and organization is established, we may be stuck with it
> for
> > a long time).
> >
> > Thoughts?
> >
> > Is it even possible to rearrange all classes in each package below into a
> > single jar?  Is doing so desirable enough to defer Java 11 support until
> a
> > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> >
> > Or, is it satisfactory to fatten Geode releases to include one set of
> jars
> > for CLASSPATH use, plus a different set of jars for MODULEPATH use?
> >
> >
> > Package
> > Jar
> > org.apache.geode.cache.client.internal
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.client.internal.locator.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.cache.query.internal.cq
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.cache.util
> > geode-core-1.8.0.jar
> > geode-rebalancer-1.8.0.jar
> > org.apache.geode.internal
> > geode-connectors-1.8.0.jar
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > geode-lucene-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.tier.sockets.command
> > geode-core-1.8.0.jar
> > geode-cq-1.8.0.jar
> > org.apache.geode.internal.cache.wan
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.parallel
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.cache.wan.serial
> > geode-core-1.8.0.jar
> > geode-wan-1.8.0.jar
> > org.apache.geode.internal.protocol.protobuf.v1
> > geode-protobuf-1.8.0.jar
> > geode-protobuf-messages-1.8.0.jar
>


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

2018-11-26 Thread Helena Bales
For test failures that do not have a ticket for that particular stack
trace, you should re-trigger your pre-checkin. If the test fails again,
your change probably caused it to start failing, even if it doesn't seem
related (like the unit test ordering issue we had Friday before last), and
you would be expected to fix it before committing.
If the test passes your next run of tests the process is less established.
Ideally we would never encounter this case because it means we have checked
in flaky code at some point before your branch off of develop, which
StressNewTest should catch. We will probably find a few of these because
the StressNewTest job was silently failing for a little while. It is
working again, but we may have checked in some flaky tests during the time
it was down.

I propose that we follow the CIO process with these. The process would be
as follows:
1. Create a Jira ticket for the failure with links to the relevant
resources from the failing CI run. Include any evidence that you have
towards it being a flaky test that exists on develop and not just in your
branch, and evidence that it was not made flaky by your change.
2. See if that test file was changed recently. If it was, talk to the
person that changed it.
3. If it wasn't changed recently, post a link to gemfire-green-ci
4. Comment on your pull request with an update on the status of the failing
test in your run.

That is just an idea based on what we are doing for the develop pipeline.
It seems like we are having more failures in PR pipelines than we were
seeing a couple of weeks ago, and some tests that seem to only fail in the
PR pipelines, so it might be time to start tracking some of this stuff. My
main concern with this method is that it might pollute our backlog with
tickets that no one is ever going to look at.

On Mon, Nov 26, 2018 at 10:19 AM Kirk Lund  wrote:

> 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:
> org.apache.geode.internal.cache.SizingFlagDUnitTest >
> testPRHeapLRUDeltaPutOnPrimary FAILED
> org.apache.geode.test.dunit.RMIException: While invoking
> org.apache.geode.internal.cache.SizingFlagDUnitTest$12.run in VM 0 running
> on Host eb7aca4f2587 with 4 VMs
> at org.apache.geode.test.dunit.VM.invoke(VM.java:433)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:402)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:361)
> at
>
> org.apache.geode.internal.cache.SizingFlagDUnitTest.assertValueType(SizingFlagDUnitTest.java:793)
> at
>
> org.apache.geode.internal.cache.SizingFlagDUnitTest.doPRDeltaTestLRU(SizingFlagDUnitTest.java:312)
> at
>
> org.apache.geode.internal.cache.SizingFlagDUnitTest.testPRHeapLRUDeltaPutOnPrimary(SizingFlagDUnitTest.java:220)
>
> Caused by:
> org.apache.geode.cache.EntryNotFoundException: Entry not found for
> key 0
> at
>
> org.apache.geode.internal.cache.LocalRegion.checkEntryNotFound(LocalRegion.java:2760)
> at
>
> org.apache.geode.internal.cache.LocalRegion.nonTXbasicGetValueInVM(LocalRegion.java:3448)
> at
>
> org.apache.geode.internal.cache.LocalRegionDataView.getValueInVM(LocalRegionDataView.java:105)
> at
>
> org.apache.geode.internal.cache.LocalRegion.basicGetValueInVM(LocalRegion.java:3436)
> at
>
> org.apache.geode.internal.cache.LocalRegion.getValueInVM(LocalRegion.java:3424)
> at
>
> org.apache.geode.internal.cache.PartitionedRegionDataStore.getLocalValueInVM(PartitionedRegionDataStore.java:2775)
> at
>
> org.apache.geode.internal.cache.PartitionedRegion.getValueInVM(PartitionedRegion.java:8786)
> at
>
> org.apache.geode.internal.cache.SizingFlagDUnitTest$12.run(SizingFlagDUnitTest.java:797)
>


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:
org.apache.geode.internal.cache.SizingFlagDUnitTest >
testPRHeapLRUDeltaPutOnPrimary FAILED
org.apache.geode.test.dunit.RMIException: While invoking
org.apache.geode.internal.cache.SizingFlagDUnitTest$12.run in VM 0 running
on Host eb7aca4f2587 with 4 VMs
at org.apache.geode.test.dunit.VM.invoke(VM.java:433)
at org.apache.geode.test.dunit.VM.invoke(VM.java:402)
at org.apache.geode.test.dunit.VM.invoke(VM.java:361)
at
org.apache.geode.internal.cache.SizingFlagDUnitTest.assertValueType(SizingFlagDUnitTest.java:793)
at
org.apache.geode.internal.cache.SizingFlagDUnitTest.doPRDeltaTestLRU(SizingFlagDUnitTest.java:312)
at
org.apache.geode.internal.cache.SizingFlagDUnitTest.testPRHeapLRUDeltaPutOnPrimary(SizingFlagDUnitTest.java:220)

Caused by:
org.apache.geode.cache.EntryNotFoundException: Entry not found for
key 0
at
org.apache.geode.internal.cache.LocalRegion.checkEntryNotFound(LocalRegion.java:2760)
at
org.apache.geode.internal.cache.LocalRegion.nonTXbasicGetValueInVM(LocalRegion.java:3448)
at
org.apache.geode.internal.cache.LocalRegionDataView.getValueInVM(LocalRegionDataView.java:105)
at
org.apache.geode.internal.cache.LocalRegion.basicGetValueInVM(LocalRegion.java:3436)
at
org.apache.geode.internal.cache.LocalRegion.getValueInVM(LocalRegion.java:3424)
at
org.apache.geode.internal.cache.PartitionedRegionDataStore.getLocalValueInVM(PartitionedRegionDataStore.java:2775)
at
org.apache.geode.internal.cache.PartitionedRegion.getValueInVM(PartitionedRegion.java:8786)
at
org.apache.geode.internal.cache.SizingFlagDUnitTest$12.run(SizingFlagDUnitTest.java:797)


Re: Avoiding Unpredictability in Our DUnit Testing Rules

2018-11-26 Thread Bruce Schuchardt
While it's common to use VM.getVM(int) to get a VM you still must use 
Host.getHost() to get a VM running an older version of Geode. Until 
someone adds a VM.getVM(version, vmNumber) we can't frown upon 
Host.getHost(0).getVM(startingVersion, 0).


On 11/21/18 4:09 PM, Mark Hanson wrote:

It is frowned upon.  VM.getVM(0) is now the accepted way to get VM 0.

Thanks,
Mark



On Nov 21, 2018, at 10:41 AM, Nabarun Nag  wrote:

Will doing this  in the test,

final Host host = Host.getHost(0);
VM server1 = host.getVM(startingVersion, 0);

be frowned upon, if I use the above over using @Rule.

Regards
Nabarun Nag


On Nov 21, 2018, at 10:36 AM, Robert Houghton  wrote:

Great find, Patrick. I hope this shakes out some of the test bugs!

On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg 
tl;dr: Use JUnit RuleChain.


Hello all!

Several [1] of our test @Rule classes make use of the fact that our DUnit
VMs Host is statically accessible to affect every test VM.  For instance,
the SharedCountersRule initializes a counter in every VM, and the
CleanupDUnitVMsRule bounces VMs before and after each test.

Problematically, JUnit rules applied in an unpredictable / JVM-dependent
ordering. [2]  As a result, some flakiness we find in our tests may be the
result of unexpected interaction and ordering of our test rules. [3]

Fortunately, a solution to this problem already exists.  Rule ordering can
be imposed by JUnit's RuleChain. [4]

In early exploration with this rule, some tests failed due to the RuleChain
not being serializable.  However, as it should only apply to the test VM,
and given that it can be composed of (unannotated) rules that remain
accessible and serializable, it should be a simple matter of declaring the
offending field transient, as it will only be necessary in the test VM.

So, you dear reader: while you're out there making Geode the best it can
be, if you find yourself in a test class that uses more than one rule
listed in [1], or if you notice some other rule not listed below that
reaches out to VMs as part of its @Before or @After, please update that
test to use the RuleChain to apply the rules in a predictable order.

Imagination is Change.
~Patrick

[1] A probably-incomplete list of invasive rules can be found via
$> git grep -il inEveryVM | grep Rule.java

geode-core/src/distributedTest/java/org/apache/geode/management/ManagementTestRule.java
geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/CacheRule.java

geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClientCacheRule.java

geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedDiskDirRule.java

geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedRule.java

geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedUseJacksonForJsonPathRule.java

geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/SharedCountersRule.java

[2] See the documentation for rules here:
https://junit.org/junit4/javadoc/4.12/org/junit/Rule.html ; notably,
"However,
if there are multiple [Rule] fields (or methods) they will be applied in an
order that depends on your JVM's implementation of the reflection API,
which is undefined, in general."

[3] For what it's worth, this was discovered after looking into why the
DistributedRule check fo suspicious strings caused the test *after* the
test that emitted the strings to fail.  It's only tangentially related, but
got me looking into when and how the @After was getting applied.

[4] https://junit.org/junit4/javadoc/4.12/org/junit/rules/RuleChain.html



Re: Avoiding Unpredictability in Our DUnit Testing Rules

2018-11-26 Thread Helena Bales
Is there a preferred ordering of the above list of rules? Or is the
preferred ordering whichever passes?

On Wed, Nov 21, 2018 at 4:36 PM Mark Hanson  wrote:

> It is frowned upon.  VM.getVM(0) is now the accepted way to get VM 0.
>
> Thanks,
> Mark
>
>
> > On Nov 21, 2018, at 10:41 AM, Nabarun Nag  wrote:
> >
> > Will doing this  in the test,
> >
> > final Host host = Host.getHost(0);
> > VM server1 = host.getVM(startingVersion, 0);
> >
> > be frowned upon, if I use the above over using @Rule.
> >
> > Regards
> > Nabarun Nag
> >
> >> On Nov 21, 2018, at 10:36 AM, Robert Houghton 
> wrote:
> >>
> >> Great find, Patrick. I hope this shakes out some of the test bugs!
> >>
> >> On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg  wrote:
> >>
> >>> tl;dr: Use JUnit RuleChain.
> >>> 
> >>>
> >>> Hello all!
> >>>
> >>> Several [1] of our test @Rule classes make use of the fact that our
> DUnit
> >>> VMs Host is statically accessible to affect every test VM.  For
> instance,
> >>> the SharedCountersRule initializes a counter in every VM, and the
> >>> CleanupDUnitVMsRule bounces VMs before and after each test.
> >>>
> >>> Problematically, JUnit rules applied in an unpredictable /
> JVM-dependent
> >>> ordering. [2]  As a result, some flakiness we find in our tests may be
> the
> >>> result of unexpected interaction and ordering of our test rules. [3]
> >>>
> >>> Fortunately, a solution to this problem already exists.  Rule ordering
> can
> >>> be imposed by JUnit's RuleChain. [4]
> >>>
> >>> In early exploration with this rule, some tests failed due to the
> RuleChain
> >>> not being serializable.  However, as it should only apply to the test
> VM,
> >>> and given that it can be composed of (unannotated) rules that remain
> >>> accessible and serializable, it should be a simple matter of declaring
> the
> >>> offending field transient, as it will only be necessary in the test VM.
> >>>
> >>> So, you dear reader: while you're out there making Geode the best it
> can
> >>> be, if you find yourself in a test class that uses more than one rule
> >>> listed in [1], or if you notice some other rule not listed below that
> >>> reaches out to VMs as part of its @Before or @After, please update that
> >>> test to use the RuleChain to apply the rules in a predictable order.
> >>>
> >>> Imagination is Change.
> >>> ~Patrick
> >>>
> >>> [1] A probably-incomplete list of invasive rules can be found via
> >>> $> git grep -il inEveryVM | grep Rule.java
> >>>
> >>>
> geode-core/src/distributedTest/java/org/apache/geode/management/ManagementTestRule.java
> >>>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/CacheRule.java
> >>>
> >>>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClientCacheRule.java
> >>>
> >>>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedDiskDirRule.java
> >>>
> >>>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedRule.java
> >>>
> >>>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedUseJacksonForJsonPathRule.java
> >>>
> >>>
> geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/SharedCountersRule.java
> >>>
> >>> [2] See the documentation for rules here:
> >>> https://junit.org/junit4/javadoc/4.12/org/junit/Rule.html ; notably,
> >>> "However,
> >>> if there are multiple [Rule] fields (or methods) they will be applied
> in an
> >>> order that depends on your JVM's implementation of the reflection API,
> >>> which is undefined, in general."
> >>>
> >>> [3] For what it's worth, this was discovered after looking into why the
> >>> DistributedRule check fo suspicious strings caused the test *after* the
> >>> test that emitted the strings to fail.  It's only tangentially
> related, but
> >>> got me looking into when and how the @After was getting applied.
> >>>
> >>> [4]
> https://junit.org/junit4/javadoc/4.12/org/junit/rules/RuleChain.html
> >>>
> >
>
>