Review Request 56242: GEODE-2206: Add junit-quickcheck to geode-core; add a test that uses it.

2017-02-02 Thread Galen O'Sullivan
-quickcheck and looked into the data it generates: integral numbers seem pretty reasonable. Strings tend to be short-ish (length up to hundreds with hundreds of iterations, thousands with thousands), but are made up of random codepoints, which is nice. Thanks, Galen O'Sullivan

Re: Review Request 56242: GEODE-2206: Add junit-quickcheck to geode-core; add a test that uses it.

2017-02-02 Thread Galen O'Sullivan
(length up to hundreds with hundreds of iterations, thousands with thousands), but are made up of random codepoints, which is nice. Thanks, Galen O'Sullivan

Re: Review Request 56129: GEODE-2368 Need to fix log message in DirectChannel

2017-01-31 Thread Galen O'Sullivan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56129/#review163680 --- Ship it! Ship It! - Galen O'Sullivan On Jan. 31, 2017, 5:03

Re: Review Request 55736: GEODE-2328: Refactor add Field methods to reduce the duplicates

2017-02-01 Thread Galen O'Sullivan
t; > (Updated Jan. 20, 2017, 4:50 p.m.) > > > Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Hitesh > Khamesra. > > > Repository: geode > > > Description > --- > > the add fields have the same behavior and identical co

Re: Review Request 55736: GEODE-2328: Refactor add Field methods to reduce the duplicates

2017-02-01 Thread Galen O'Sullivan
--- > > (Updated Jan. 20, 2017, 4:50 p.m.) > > > Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Hitesh > Khamesra. > > > Repository: geode > > > Description > --- > > the add fields have the same behavior and identica

Re: Review Request 55738: GEODE-2329: Refactor JSONFormatter.fromJSON method to reduce duplication

2017-01-31 Thread Galen O'Sullivan
a (line 725) <https://reviews.apache.org/r/55738/#comment235239> Don't forget to run spotless so that the build passes, even though it will ruin your formatting. - Galen O'Sullivan On Jan. 20, 2017, 4:45 p.m., U

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-24 Thread Galen O'Sullivan
/cache/tier/sockets/AcceptorImplDUnitTest.java PRE-CREATION geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplJUnitTest.java 7aa11b7ca Diff: https://reviews.apache.org/r/55742/diff/ Testing --- precheckin in progress. Thanks, Galen O'Sullivan

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-24 Thread Galen O'Sullivan
ion. - Galen --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55742/#review162521 --- On Jan. 20, 2017, 11:20 p.m., Galen O'Sullivan wrot

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-24 Thread Galen O'Sullivan
nsider a thread that pauses on the `synchronized` block while another is inside shutting down.) - Galen O'Sullivan On Jan. 24, 2017, 9:53 p.m., Galen O'Sullivan wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-30 Thread Galen O'Sullivan
/internal/cache/tier/sockets/AcceptorImpl.java 060683de6 geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImplDUnitTest.java PRE-CREATION Diff: https://reviews.apache.org/r/55742/diff/ Testing --- precheckin started. Thanks, Galen O'Sullivan

Re: Review Request 56093: GEODE-2368 Need to fix log message in DirectChannel

2017-01-30 Thread Galen O'Sullivan
/TCPConduit.java (line 927) <https://reviews.apache.org/r/56093/#comment235029> The `getLocalAddr` / `getLocalAddress` / (and now) `getAddress` mix here is confusing. Do you understand the code well enough to refactor these into two or preferably one method? - Galen O'Sullivan On Jan. 30, 2

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-30 Thread Galen O'Sullivan
, Galen O'Sullivan

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-20 Thread Galen O'Sullivan
investigation. Diffs (updated) - geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java 060683de6 Diff: https://reviews.apache.org/r/55742/diff/ Testing --- precheckin in progress. Thanks, Galen O'Sullivan

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-20 Thread Galen O'Sullivan
/cache/tier/sockets/AcceptorImpl.java 060683de6 Diff: https://reviews.apache.org/r/55742/diff/ Testing (updated) --- precheckin in progress. Thanks, Galen O'Sullivan

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-24 Thread Galen O'Sullivan
ll throw an InterrruptedException. - Galen --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55742/#review162479 --- On J

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-24 Thread Galen O'Sullivan
). Thanks, Galen O'Sullivan

Re: Review Request 55742: [GEODE-2324] and what look like a few other bugs in the same method.

2017-01-24 Thread Galen O'Sullivan
il. To reply, visit: https://reviews.apache.org/r/55742/#review162512 ------- On Jan. 24, 2017, 10 p.m., Galen O'Sullivan wrote: > > --- > This is an automat

Re: Review Request 56986: GEODE-2497 surprise members are never timed out during startu

2017-02-23 Thread Galen O'Sullivan
/membership/gms/mgr/GMSMembershipManager.java (line 980) <https://reviews.apache.org/r/56986/#comment238540> Is it worth checking for dcReceiver after this? Do you know what the difference is? - Galen O'Sullivan On Feb. 23, 2017, 4:17 p.m., Bruce Schuchardt

Re: Review Request 56999: GEODE-2534 concurrently started locators fail to create a unified system

2017-02-24 Thread Galen O'Sullivan
code is confusing. We're waiting at the end of `findServices`, not trying to update services, and doing a dumb wait? Or do we get interrupted when the services are updated? - Galen O'Sullivan On Feb. 23, 2017, 9:50 p.m., Bruc

Re: Review Request 56682: GEODE-2444: move Redis out of geode-core.

2017-02-15 Thread Galen O'Sullivan
, Galen O'Sullivan

Review Request 54542: [GEODE-2191] Add a new randomized string serialization test.

2016-12-08 Thread Galen O'Sullivan
/pholser/junit-quickcheck) for these types of tests. Thanks, Galen O'Sullivan

Re: Review Request 54542: [GEODE-2191] Add a new randomized string serialization test.

2016-12-09 Thread Galen O'Sullivan
something like [junit-quickcheck](https://github.com/pholser/junit-quickcheck) for these types of tests. Thanks, Galen O'Sullivan

Review Request 58155: GEODE-2653: fix a flaky test.

2017-04-03 Thread Galen O'Sullivan
/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java 05ab6f7e5 Diff: https://reviews.apache.org/r/58155/diff/1/ Testing --- Running precheckin. Ran the test suite in IntelliJ a few hundred times on my machine. Thanks, Galen O'Sullivan

Re: Review Request 58544: GEODE-2632: refactor code to use InternalCache instead of GemFireCacheImpl

2017-04-20 Thread Galen O'Sullivan
casting everything; deleting old comments and commented code; making things more private. I don't know CQs or Lucene but it doesn't look like you've changed anything in how they work. Excellent! - Galen O'Sullivan On April 19, 2017, 11:09 p.m., Kirk Lund wrote

Re: Review Request 57482: Sometime client doesn't ping server in ping-interval

2017-03-10 Thread Galen O'Sullivan
to the setting and make the other side more resilient to single missed pings. What is this pinging? - Galen O'Sullivan On March 9, 2017, 10:38 p.m., Hitesh Khamesra wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 58155: GEODE-2653: fix a flaky test.

2017-04-03 Thread Galen O'Sullivan
enerated e-mail. To reply, visit: https://reviews.apache.org/r/58155/#review170927 --- On April 3, 2017, 6:31 p.m., Galen O'Sullivan wrote: > > --- > This is an automati

Re: Review Request 58187: GEODE-2732 after auto-reconnect a server is restarted on the default port of 40404

2017-04-05 Thread Galen O'Sullivan
/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java Lines 93 (patched) <https://reviews.apache.org/r/58187/#comment243953> So Cache is a singleton except when it gets replaced by reconnection? Whoa. - Galen O'Sullivan On April 4, 201

Re: Review Request 58155: GEODE-2653: fix a flaky test.

2017-04-05 Thread Galen O'Sullivan
/ Changes: https://reviews.apache.org/r/58155/diff/1-2/ Testing (updated) --- Precheckin passed. Ran the test suite in IntelliJ a few hundred times on my machine. Thanks, Galen O'Sullivan

Re: Review Request 60718: GEODE-2997: New flow getAll/putAll

2017-07-12 Thread Galen O'Sullivan
he mock here. Would a spy be easier to understand? geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/PutAllRequestOperationHandlerJUnitTest.java Lines 68 (patched) <https://reviews.apache.org/r/60718/#comment255537> The serialization service is stateless. Would a spy

Re: No nulls

2017-07-14 Thread Galen O'Sullivan
What's the general opinion of @NotNull annotations? Would they be useful? On Fri, Jul 14, 2017 at 9:23 AM, John Blum wrote: > +1 as well. > > However, I will caution this... use Java 8's new java.util.Optional class > in your codebase judiciously. Using it everywhere,

Re: Review Request 60834: GEODE-3051: Remove unreachable exception handling in AcceptorImpl.accept

2017-07-14 Thread Galen O'Sullivan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60834/#review180546 --- Ship it! Ship It! - Galen O'Sullivan On July 13, 2017, 12

Re: Avoid mutating standard Java System Properties (even in tests)

2017-07-20 Thread Galen O'Sullivan
I have had a lot of tests fail because mcast-port is nonzero. So now I set that system property in a lot of new tests. Maybe the tests that set it to something other than the default should be passing a Properties object to CacheFactory rather than messing with system properties. I think the

Review Request 60550: GEODE-3154: add geode-protobuf to expected_jars.txt

2017-06-29 Thread Galen O'Sullivan
keep builds from failing. Diffs - geode-assembly/src/test/resources/expected_jars.txt 62601677f Diff: https://reviews.apache.org/r/60550/diff/1/ Testing --- Built on my machine, verified that the test passes. Thanks, Galen O'Sullivan

Re: Review Request 60570: GEODE-3153 Client receives duplicate events during rolling upgrade

2017-06-30 Thread Galen O'Sullivan
bility tests, but do we want to be testing agains current version and testing version or current and 9.0? Will backwards compatibility set `testVersion` to 9.0 among other versions? - Galen O'Sullivan On June 30, 2017, 4:33 p.m., Bruce Schu

Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Galen O'Sullivan
the base commit? geode-assembly/build.gradle Lines 143 (patched) <https://reviews.apache.org/r/60312/#comment253398> It's a nit, but it would be nice to remove trailing whitespace. - Galen O'Sullivan On June 21, 2017, 9:35 p.m., Hitesh Khamesra

Re: Review Request 60312: GEODE-2804 Cache InetAddress if configure host as ip string.

2017-06-27 Thread Galen O'Sullivan
> On June 27, 2017, 6:19 p.m., Galen O'Sullivan wrote: > > The diff doesn't apply cleanly on develop on my machine. What's the base > > commit? Or better, can you merge or rebase on develop please? - Galen --- This is a

Re: GMSJoinLeaveJUnitTest failing on develop

2017-07-28 Thread Galen O'Sullivan
I just filed GEODE- for GMSJoinLeaveJUnitTest. testViewNotSentWhenShuttingDown . I've gotten a failure from the command line, but not when I run it from within IntelliJ (even repeatedly). On Fri, Jul 28, 2017 at 4:12 PM, Bruce Schuchardt wrote: > (GEODE-3155)

Re: Review Request 59925: GEODE-3052 Restarting 2 locators within 1s of each other causes potential locator split brain

2017-06-08 Thread Galen O'Sullivan
> On June 8, 2017, 11:45 p.m., Galen O'Sullivan wrote: > > This makes sense to me: we remove the locators if we can't connect to them. > > > > I wonder what happens if the two locators can't talk to each other (at > > first, anyways) but can talk to the res

Re: Review Request 59850: GEODE-3023: TcpServer thread can be blocked in processRequest

2017-06-08 Thread Galen O'Sullivan
- > > (Updated June 8, 2017, 12:20 a.m.) > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen > O'Sullivan, Hitesh Khamesra, and Brian Rowe. > > > Repository: geode > > > Description > ---

Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Galen O'Sullivan
reviews.apache.org/r/60157/diff/2/?file=1752476#file1752476line58> > > > > surely we don't need this here... How do we handle the case if we have > > another client protocol handler... this would fail. We can test this at the factory level. I hadn't been thinking of thi

Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Galen O'Sullivan
o reply, visit: https://reviews.apache.org/r/60157/#review178279 --- On June 20, 2017, 12:53 a.m., Galen O'Sullivan wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-19 Thread Galen O'Sullivan
equivalent. If you want me to run again, I will.) Thanks, Galen O'Sullivan

Re: Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

2017-06-20 Thread Galen O'Sullivan
ws.apache.org/r/60217/#comment252365> Do you know what this ignore does? geode-protobuf/src/test/java/org/apache/geode/serialization/registry/CodecRegistryJUnitTest.java Lines 58 (patched) <https://reviews.apache.org/r/60217/#comment252367> This test can have an expected except

Re: Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

2017-06-21 Thread Galen O'Sullivan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60217/#review178541 --- Ship it! Ship It! - Galen O'Sullivan On June 21, 2017, 7:01

Review Request 60394: GEODE-3075 and GEODE-2995: merg

2017-06-23 Thread Galen O'Sullivan
in `excludeClasses.txt` instead of `sanctionedSerializables.txt`. Thanks, Galen O'Sullivan

Re: Review Request 60394: GEODE-3075 and GEODE-2995: merge of new protobuf protocol work.

2017-06-23 Thread Galen O'Sullivan
settings.gradle c0fdb6e4f Diff: https://reviews.apache.org/r/60394/diff/2/ Changes: https://reviews.apache.org/r/60394/diff/1-2/ Testing --- Precheckin passed on v1 (with the serializable in `excludeClasses.txt` instead of `sanctionedSerializables.txt`. Thanks, Galen O'Sullivan

Re: Review Request 60394: GEODE-3075 and GEODE-2995: merge of new protobuf protocol work.

2017-06-23 Thread Galen O'Sullivan
gradle/rat.gradle 1bea5843b settings.gradle c0fdb6e4f Diff: https://reviews.apache.org/r/60394/diff/2/ Testing --- Precheckin passed on v1 (with the serializable in `excludeClasses.txt` instead of `sanctionedSerializables.txt`). Thanks, Galen O'Sullivan

Re: Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-20 Thread Galen O'Sullivan
, but I'd consider it pretty much equivalent. If you want me to run again, I will.) Thanks, Galen O'Sullivan

Re: Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

2017-06-20 Thread Galen O'Sullivan
, though. settings.gradle Line 39 (original), 39 (patched) <https://reviews.apache.org/r/60217/#comment252339> `geode-protobuf` will also need to be added to `geode-assembly` before this is functional, but we can do that in the work that combines this with the `ServerConnection` wor

Re: Review Request 60217: GEODE-2995: adding in protocol handler for protobuf messages

2017-06-21 Thread Galen O'Sullivan
> This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60217/ > --- > > (Updated June 21, 2017, 12:02 a.m.) > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen &g

Re: Review Request 60446: Events do not get removed from the client queue for 1.0 clients

2017-06-27 Thread Galen O'Sullivan
/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java Lines 40 (patched) <https://reviews.apache.org/r/60446/#comment253364> Is it possible for a Membership ID to be less than 17 bytes and this to be negative? - Galen O'Sullivan On June 26, 2017, 10:24 p.m., Bruce Schu

Re: Review Request 60442: GEODE-3130: Refactoring AcceptorImpl communication mode switch

2017-06-27 Thread Galen O'Sullivan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60442/#review179007 --- Ship it! Ship It! - Galen O'Sullivan On June 26, 2017, 6:31

Re: Review Request 59546: GEODE-2940 Remove verification of locator host on start

2017-05-25 Thread Galen O'Sullivan
/59546/#comment249432> Could this be split into two tests, one of which continues and the other of which triggers a failure later on? - Galen O'Sullivan On May 25, 2017, 5:12 p.m., Hitesh Khamesra wrote: > > --- > This i

Re: Review Request 59546: GEODE-2940 Remove verification of locator host on start

2017-05-25 Thread Galen O'Sullivan
> On May 25, 2017, 5:51 p.m., Galen O'Sullivan wrote: > > geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java > > Lines 346 (patched) > > <https://reviews.apache.org/r/59546/diff/1/?file=1732164#file1732164line346> >

Re: Review Request 59542: GEODE-2974: rename ResultBuilder methods: GemFire -> Geode

2017-05-26 Thread Galen O'Sullivan
> On May 25, 2017, 11:06 p.m., Darrel Schneider wrote: > > I think in this case it would have been better to just drop GemFire instead > > of renaming it to Geode. > > I see no reason why the product name should be included in these messages. > > The gfsh message can just say "Could not process

Re: Review Request 59866: GEODE-2555: Region Management docs page refers to the wrong field (id= should be refid=)

2017-06-07 Thread Galen O'Sullivan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59866/#review177196 --- Ship it! Ship It! - Galen O'Sullivan On June 6, 2017, 10:50

Re: Review Request 59850: GEODE-3023: TcpServer thread can be blocked in processRequest

2017-06-07 Thread Galen O'Sullivan
through the tests. - Galen O'Sullivan On June 6, 2017, 11:21 p.m., Udo Kohlmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Review Request 60157: GEODE-3075: initial work for feature flag and creation of a new subclass of `ServerConnection`.

2017-06-16 Thread Galen O'Sullivan
, but I'd consider it pretty much equivalent. If you want me to run again, I will.) Thanks, Galen O'Sullivan

Re: Please don't commit broken javadocs

2017-09-22 Thread Galen O'Sullivan
We'll move everything to internal soon, before public release. We have a ticket to fix the Javadocs and the links will be removed soon as well. The broken links are for generated files that don't seem to have been created when the javadoc task runs. On Fri, Sep 22, 2017 at 2:27 PM, Dan Smith

Re: [DISCUSS] Framework for concurrency tests

2017-09-15 Thread Galen O'Sullivan
+1 This is great! I'll take a look at your PR when I get the time. We may want to think carefully about how often we run these tests, because unlike regular unit tests, they will take forever to run. On Fri, Sep 15, 2017 at 1:42 PM, Michael William Dodge wrote: > +1 for unit

Re: Rebase and squash before merging PRs

2017-10-05 Thread Galen O'Sullivan
On Thu, Oct 5, 2017 at 3:14 PM, Jinmei Liao wrote: > Not sure if this is useful to everyone, but when I push a subsequent commit > to my feature branch, I always use "force push", so that it's only one commit > I need to rebase to develop. > > I use force push when I've made

Re: [VOTE] Apache Geode release - v1.2.1 RC2

2017-09-05 Thread Galen O'Sullivan
Looks like the repos have moved to gitbox. It should be as simple as changing the three git-wip-us.apache.org links to gitbox.apache.org . On Tue, Sep 5, 2017 at 5:01 PM, Karen Miller wrote: > A guarded +1 based on limited testing of the RC: > > Built the Geode RC from

Re: Review Request 62088: GEODE-3249 Validate internal client/server messages

2017-09-05 Thread Galen O'Sullivan
think `allow` would be clearer than `disallow` because it avoids double-negatives. Can we use `allow-internal-messages-without-credentials` and have it default to `true`? - Galen O'Sullivan On Sept. 5, 2017, 5:57 p.m., Bruce Schuchardt wrote

Re: New client/server protocol - seeking feedback

2017-09-25 Thread Galen O'Sullivan
Replies inline. On Mon, Sep 25, 2017 at 12:13 PM, Udo Kohlmeyer wrote: > Replies inline > On Mon, Sep 25, 2017 at 11:21 AM, Dan Smith wrote: > > > This actually brings up another point I was going to ask about. I don't > see > > any version information

Re: New Geode PMC Member: Galen O'Sullivan

2017-09-26 Thread Galen O'Sullivan
t; > > On Tue, Sep 26, 2017 at 10:48 AM, Mark Bretl <mbr...@apache.org> wrote: > > > > > The Apache Geode Project Management Committee has invited Galen > > > O'Sullivan to join the Geode PMC and we are pleased to announce he has > > > accepted. >

JPF breaking IntelliJ build?

2017-09-27 Thread Galen O'Sullivan
I'm having trouble running tests in IntelliJ because of errors with compilation. Does anyone know why? I've done a clean build and reloaded Gradle configuration in IntelliJ. Thanks, Galen Information:java: Errors occurred while compiling module 'geode-core' Information:javac 1.8.0_111 was used

New client protocol configuration

2017-10-02 Thread Galen O'Sullivan
Currently, we have a setting for the new client protocol that controls whether authentication is required or not. We expect to expand this in the future, and also that there may be more configuration options for the protocol. We would like to namespace the settings for this protocol but don't

Re: git repo is gone

2017-09-01 Thread Galen O'Sullivan
It looks like it's moved. If you go to the URL in a browser it will give you a hint. This command should work to update your remote: git remote set-url https://gitbox.apache.org/repos/asf/geode.git running `git remote -v` after should confirm the

Re: Review Request 61978: GEODE-3059: LoadMonitor.connectionClosed incrementing statistics only for client-server connection

2017-09-01 Thread Galen O'Sullivan
549> This comparison is changed with byte vs int rules. Probably better to use the int comparison. Thismight be even better as a method implemented on CommunicationMode. - Galen O'Sullivan On Aug. 30, 2017, 8:48 p.m., Bruce Schuchardt

Re: New client protocol configuration

2017-10-05 Thread Galen O'Sullivan
I don't care too much about exactly what the configuration looks like, but I want it to be unified, and I want it to be set when the cache starts. Checking system properties throughout the codebase whenever we feel like is a bit too magic for me. In addition, it seems that in order to add a new

Thinking about a "Public API" module

2017-10-11 Thread Galen O'Sullivan
Hi all, I'm thinking about the possibility of making a "Geode public API" module, mostly containing interfaces that Geode implements. Why would one do this, you may ask? Here are some reasons. * It makes clear what is our public API (as opposed to "internal" type packages). * Users only have to

Re: Font for geode.apache.org

2017-11-27 Thread Galen O'Sullivan
If you're talking about "Performance is Key. Consistency is a must", it looks like the font is "Klavika Web Basic Light". I see the following in the CSS for that element: font-family: 'klavika-web', 'Open Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif !important; On Mon, Nov 27, 2017 at

Re: ':geode-client-protocol' could not be found in project ':geode-assembly'

2017-11-03 Thread Galen O'Sullivan
Can you show me the particular error? Does it persist? As far as I can remember, we haven't changed that module this week. On Fri, Nov 3, 2017 at 9:24 AM, Kirk Lund <kl...@apache.org> wrote: > Precheckin last night failed to compile. > > On Fri, Nov 3, 2017 at 9:22 AM, Galen O'Su

Re: ':geode-client-protocol' could not be found in project ':geode-assembly'

2017-11-03 Thread Galen O'Sullivan
It may be that we missed one of the places where a module gets linked in. Where did you see this error? On Fri, Nov 3, 2017 at 9:20 AM, Kirk Lund wrote: > Anyone know why the build is failing in geode-assembly? > > * What went wrong: > A problem occurred evaluating project

Value Serialization Proposal for the New Client Protocol

2018-05-16 Thread Galen O'Sullivan
fleshed out, so if there are details you can think of that we might have missed or any other feedback you can give, I would love to hear it! https://cwiki.apache.org/confluence/display/GEODE/Protobuf+Value+Encoding+Scheme+Proposal Thanks, Galen O'Sullivan

Re: Apache Geode Logo

2018-05-23 Thread Galen O'Sullivan
Hi Mike, The attachment doesn't seem to have attached to your email. Thanks, Galen On Wed, May 23, 2018 at 2:29 PM, Michael Stolz wrote: > Can someone who has sufficient mojo please update the Apache Geode logo on > geode.apache.org with the attached version with the TM

Re: Value Serialization Proposal for the New Client Protocol

2018-05-18 Thread Galen O'Sullivan
: Hi Galen, I put a bunch of comments inline in your wiki page. One general question - is the a proposal to change the default value encoding with the protobuf protocol from JSON to this new format? -Dan On Wed, May 16, 2018 at 5:06 PM, Galen O'Sullivan <gosulli...@pivotal.io> wrote: Hi all,

Re: [VOTE] Apache Geode 1.6.0 RC1

2018-05-01 Thread Galen O'Sullivan
YS [3] https://dist.apache.org/repos/dist/release/geode/KEYS [4] https://mirror-vm.apache.org/~henkp/checker/faq.html On Apr 30, 2018, at 10:31 AM, Galen O'Sullivan <gosulli...@pivotal.io> wrote: -1 I don't see Mike's key in the KEYS file on either rel/v1.6.0.RC1 (5ce726bd7b4f8d2648fd011a807a1bcc624dd

Re: Reviewing our JIRA's

2018-04-26 Thread Galen O'Sullivan
I'm for it. Less noise is a good thing, and I don't think they're likely to get prioritized anyways. If we close as WONTFIX or similar, we can always look back for them later if we want. On 4/26/18 10:39 AM, Anthony Baker wrote: Thanks Lynn! As I first step I’d like to focus on issues

Re: Proposal: GEODE-4367 - Return PDXInstance when Domain Object can't be found

2018-01-25 Thread Galen O'Sullivan
lize it & return it? > What > > > > if it isn't PDX-serialized? Do we return a byte array? > > > > > > > > > > > > On 1/24/18 12:21 PM, Dan Smith wrote: > > > > > Is this really just a workaround for the fact that the > > re

Re: Proposal: GEODE-4367 - Return PDXInstance when Domain Object can't be found

2018-01-24 Thread Galen O'Sullivan
Hi Addison, What kind of setup do you have that is causing you to have PDX serialized objects that cannot be deserialized? Do you have classes that are present on some servers but not others? This change would break the contract of region.get() , which is that it returns a value of a type that

Re: Proposed API for a ConcurrencyTestHelper

2018-07-30 Thread Galen O'Sullivan
I like this API. I think it saves a lot of boilerplate, and a lot of subtle possible mistakes. I think the new API would be a bit cleaner than the RegionVersionVectorTest.doesNotHangIfOtherThreadChangedVersion that Kirk mentions. I would like to see generics, especially for `public void

Assertions in Geode

2018-07-18 Thread Galen O'Sullivan
Hi all, I'm wondering what the collective's opinion of assertions is. I notice there's an org.apache.geode.internal.Assert class, which is used some places, and that plain old Java assertions are used in other places. Can we remove one of these and use the other? Should we be including assertions

Re: [DISCUSS] Remove CatchException testing dependency

2018-09-05 Thread Galen O'Sullivan
+1 On Mon, Aug 27, 2018 at 10:06 AM, Kirk Lund wrote: > Jira ticket as been filed for this: > > GEODE-5639: Replace usage of CatchException with AssertJ and remove > CatchException dependency > > On Thu, Aug 23, 2018 at 4:03 PM, Kirk Lund wrote: > > > I goofed on #1. We should be using*

[DISCUSS] Wrapping log calls in Conditionals like isDebugEnabled, isTraceEnabled, etc.

2018-09-07 Thread Galen O'Sullivan
Hi all, I've noticed a pattern in Geode where we wrap a log call in a check at the same level, such as: if (logger.isDebugEnabled()) { logger.debug("cleaning up incompletely started DistributionManager due to exception", r); } Is there any reason to do this? To my mind,

Re: [DISCUSS] Wrapping log calls in Conditionals like isDebugEnabled, isTraceEnabled, etc.

2018-09-10 Thread Galen O'Sullivan
endar()); > > >3. } > > > > > > > > > Log4j2 lets you pass a format string, so you can just do this: > > > > > > logger.debug("Logging in user {} with birthday {}", user.getName(), > > > user.getBirthdayCalendar()); > > > > > > > >

Debugging Multiple DUnit VMs with IntelliJ

2018-09-10 Thread Galen O'Sullivan
Hi all, I remembered a couple email threads discussing how it was possible to debug multiple DUnit processes from IntelliJ, but couldn't find a wiki article or a very complete description, so I wrote a wiki article. It's a little barebones right now, but there it is. Please edit or comment with

Re: FunctionService Proposal

2018-03-08 Thread Galen O'Sullivan
+1 for making the function service not static, and splitting servers from clients. Also +1 for Dan's suggestion. On Wed, Mar 7, 2018 at 2:51 PM, Patrick Rhomberg wrote: > I did not know that! And then, yes, onRegion is much better. > > On Wed, Mar 7, 2018 at 2:43 PM, Dan

Reverted PutIfAbsent

2018-03-14 Thread Galen O'Sullivan
Hopefully this will fix the pipeline failures. Sorry for the inconvenience. Best, Galen

Broken Integration Tests due to putIfAbsent

2018-03-13 Thread Galen O'Sullivan
Hi all, I committed some code recently that looks like it broke the CI. However, based on the failure, I think it will still fail if I revert the commit. I'm currently testing a small hotfix and will apply it if it passes tests. Sorry for the inconvenience. Thanks, Galen GEODE-4836

[PROPOSAL] Deprecating DistributedSystem

2018-03-29 Thread Galen O'Sullivan
It looks like there are a few JIRA tasks open to deprecate methods on DistributedSystem, and it doesn't really have much functionality that would be useful to a user. I propose that we deprecate DistributedSystem itself, and keep the system management functionality internal. Is there any reason

New APIs, read-serialized and CacheListeners, PostProcessor etc

2018-03-26 Thread Galen O'Sullivan
Hi all, We're working on a new protocol for Geode, which means wrapping Geode APIs and exposing them via the new protocol. For performance and to avoid class/serialization logic, we want to get these objects in serialized form. This means calling cache.setReadSerializedForCurrentThread(true)

[DISCUSS] New List for Commit and CI Emails

2018-03-20 Thread Galen O'Sullivan
Hi all, I'm wondering if it would be possible to have a separate list for commit announcements, CI emails and similar automated messages, and keep this mailing list for discussion. It would make it easier to filter email, and I think it might help improve the discussion. I know I've missed

Re: [Discussion] Improving Spotless to be Even More Beautiful

2018-03-20 Thread Galen O'Sullivan
On Tue, Mar 20, 2018 at 9:06 AM, Patrick Rhomberg wrote: > > @Galen, I had thought it already did that. > I guess it does. I never knew. Excellent!

Re: [Discussion] Improving Spotless to be Even More Beautiful

2018-03-19 Thread Galen O'Sullivan
+1 to all of the above. Will Spotless also reorder imports according to our canonical ordering? On Mon, Mar 19, 2018 at 4:17 PM, Sai Boorlagadda wrote: > +1 > > On Mon, Mar 19, 2018 at 4:03 PM, Kirk Lund wrote: > > > +1 > > > > On Mon, Mar 19, 2018

Re: [DISCUSS] New List for Commit and CI Emails

2018-03-21 Thread Galen O'Sullivan
Yeah, I think I'm sending myself convinced by Swapnil's argument. How about muting the "nightly build succeeded" email? On Wed, Mar 21, 2018 at 9:58 AM, Sean Goller wrote: > Concourse sends mail whenever a job fails. > > On Wed, Mar 21, 2018 at 9:49 AM, Swapnil Bawaskar

Re: [Discuss] Scope of the CacheElement interface with the respect to the public cluster configuration API

2018-04-25 Thread Galen O'Sullivan
It looks like there are two classes called CacheElement in the codebase; am I correct in thinking that you're referring to org.apache.geode.cache.configuration.CacheElement? This class doesn't have a Javadoc, so it's a little hard as an outsider to understand exactly what it is or how it's

Re: DISCUSS: Refactor test source set for integrationTest and distributedTest

2018-06-28 Thread Galen O'Sullivan
+1 to the new test organization. And I agree with Patrick's explanation of which is the more stable test category. On Thu, Jun 28, 2018 at 9:47 AM, Patrick Rhomberg wrote: > @Dale > It seems to me that the Unit / Distributed / etc would be the more stable > purpose, and as importantly, each

Re: Running compatibility and upgrade tests using jdk9+

2018-10-11 Thread Galen O'Sullivan
I think we should run some sort of backwards-compatibility tests between Java 8 and Java 9/11+. We need testing of Geode (both old and current versions) on older JVMs talking to Geode on newer JVMs. (for example, what if Java built-in serialization changes in a way that breaks our code somehow?)

Re: proposing reduced default for "membership-port-range"

2018-10-11 Thread Galen O'Sullivan
Would it be feasible to reserve chosen ports before selecting ephemeral ports? I think this would resolve the collision issue described above. On Thu, Oct 11, 2018 at 9:58 AM Brian Rowe wrote: > I agree that we should have defaulted everything to using ephemeral ports > and forced clients to

  1   2   3   >