Re: Discussions about concerns over User API changes

[Owen Nichols]

While changing a void method to have a return type does not break source 
compatibility, it appears likely to break binary compatibility[1].

So, if you are compiling your client from source, it will compile successfully 
against either Geode 1.12 or Geode 1.13.  But if your client was already 
compiled [against Geode 1.12] and then you upgrade to Geode 1.13, without 
recompiling your client, I your client will throw MethodNotFoundException[2].

[1] 
https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_interfaces_-_API_methods
[2] 
https://stackoverflow.com/questions/47476509/can-i-change-a-return-type-to-be-a-strict-subtype-and-retain-binary-compatibilit

> On Feb 27, 2020, at 5:09 PM, Kirk Lund  wrote:
> 
> Remember, if there are any concerns about recent backwards-compatible
> changes to Geode user APIs, they should be brought on the dev list.
> 
> Also, backward-compatible changes are by definition safe and ok for a user
> API because it won't break the user's code.
> 
> Here's an example of a user API that I recently fixed...
> 
> The ClientRegionFactory is a builder with methods that are supposed to
> follow fluent-style API (ie, return the ClientRegionFactory instance so you
> can chain the calls).
> 
> Whoever added setConcurrencyChecksEnabled goofed up and made the return
> type void. Changing void to ClientRegionFactory is a fully backwards
> compatible change which corrects the API. Since this fixes the API and
> doesn't actually change the user API, this should be very safe and improves
> Geode by correcting a broken API:
> 
> *diff --git
> a/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java
> b/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*
> 
> *index add35f01a2..2a08307adc 100644*
> 
> *---
> a/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*
> 
> *+++
> b/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*
> 
> @@ -216,7 +216,7 @@ public interface ClientRegionFactory {
> 
>* @since GemFire 7.0
> 
>* @param concurrencyChecksEnabled whether to perform concurrency checks
> on operations
> 
>*/
> 
> -  void setConcurrencyChecksEnabled(boolean concurrencyChecksEnabled);
> 
> +  ClientRegionFactory setConcurrencyChecksEnabled(boolean
> concurrencyChecksEnabled);
> 
> 
> 
>   /**
> 
>* Sets the DiskStore name attribute. This causes the region to belong
> to the DiskStore.
> 
> *diff --git
> a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java
> b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*
> 
> *index 64256e8f8e..920deba055 100644*
> 
> *---
> a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*
> 
> *+++
> b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*
> 
> @@ -186,8 +186,9 @@ public class ClientRegionFactoryImpl implements
> ClientRegionFactory
> 
>   }
> 
> 
> 
>   @Override
> 
> -  public void setConcurrencyChecksEnabled(boolean
> concurrencyChecksEnabled) {
> 
> +  public ClientRegionFactory setConcurrencyChecksEnabled(boolean
> concurrencyChecksEnabled) {
> 
> 
> this.attrsFactory.setConcurrencyChecksEnabled(concurrencyChecksEnabled);
> 
> +return this;
> 
>   }
> 
> 
> 
>   @Override
> 
> Does anyone have concerns over fixing "void setConcurrencyChecksEnabled"
> by changing it to "ClientRegionFactory setConcurrencyChecksEnabled"? If
> there is a concern, is the concern about the change itself or because this
> was fixed without following a more heavy-weight process?
> 
> Thanks,
> Kirk

Discussions about concerns over User API changes

[Kirk Lund]

Remember, if there are any concerns about recent backwards-compatible
changes to Geode user APIs, they should be brought on the dev list.

Also, backward-compatible changes are by definition safe and ok for a user
API because it won't break the user's code.

Here's an example of a user API that I recently fixed...

The ClientRegionFactory is a builder with methods that are supposed to
follow fluent-style API (ie, return the ClientRegionFactory instance so you
can chain the calls).

Whoever added setConcurrencyChecksEnabled goofed up and made the return
type void. Changing void to ClientRegionFactory is a fully backwards
compatible change which corrects the API. Since this fixes the API and
doesn't actually change the user API, this should be very safe and improves
Geode by correcting a broken API:

*diff --git
a/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java
b/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*

*index add35f01a2..2a08307adc 100644*

*---
a/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*

*+++
b/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*

@@ -216,7 +216,7 @@ public interface ClientRegionFactory {

* @since GemFire 7.0

* @param concurrencyChecksEnabled whether to perform concurrency checks
on operations

*/

-  void setConcurrencyChecksEnabled(boolean concurrencyChecksEnabled);

+  ClientRegionFactory setConcurrencyChecksEnabled(boolean
concurrencyChecksEnabled);



   /**

* Sets the DiskStore name attribute. This causes the region to belong
to the DiskStore.

*diff --git
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*

*index 64256e8f8e..920deba055 100644*

*---
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*

*+++
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*

@@ -186,8 +186,9 @@ public class ClientRegionFactoryImpl implements
ClientRegionFactory

   }



   @Override

-  public void setConcurrencyChecksEnabled(boolean
concurrencyChecksEnabled) {

+  public ClientRegionFactory setConcurrencyChecksEnabled(boolean
concurrencyChecksEnabled) {


this.attrsFactory.setConcurrencyChecksEnabled(concurrencyChecksEnabled);

+return this;

   }



   @Override

Does anyone have concerns over fixing "void setConcurrencyChecksEnabled"
by changing it to "ClientRegionFactory setConcurrencyChecksEnabled"? If
there is a concern, is the concern about the change itself or because this
was fixed without following a more heavy-weight process?

Thanks,
Kirk

Re: [PROPOSAL]: Include GEODE-7814 in Release 1.12.0

[Ernest Burghardt]

There appears to be consensus that this is a critical fix.

The following commit has been brought into release/1.12 as the
critical fix for GEODE-7814:

git cherry-pick -x db86faec699aca67c02325bca22dcd5b913ddfed

GEODE-7814 has been marked as 'resolved in' 1.12.

Regards
EB


On Thu, Feb 27, 2020 at 7:39 AM Bruce Schuchardt 
wrote:

> +1
>
> The change Juan has made corrects a problem introduced during membership
> refactoring.  Every cache operation message that's sent allocates several
> objects that used to be held in statics but were moved into instance
> variables at one point.  Juan's change moves these back into static
> variables.
>
> On 2/27/20, 1:49 AM, "Ju@N"  wrote:
>
> Hello devs,
>
> I'd like to include the fix for GEODE-7814 [1] in release 1.12.0.
> The change prevents a huge amount of unnecessary allocation of objects
> while sending/receiving messages, improving the overall performance.
> The SHA is db86faec699aca67c02325bca22dcd5b913ddfed [2].
> Best regards.
>
> [1]: https://issues.apache.org/jira/browse/GEODE-7814
> [2]:
>
> https://github.com/apache/geode/commit/db86faec699aca67c02325bca22dcd5b913ddfed
>
> --
> Ju@N
>
>
>
>

Re: [DISCUSSION] - ClassLoader Isolation

[John Blum]

Bruce - The primary gist of it is, client applications do not use the
preconfigured classpath determined by Geode itself, such as would be the
case when you start servers using *Gfsh*.  Clients are not started with
*Gfsh*, or any other Geode script for that matter.

On Thu, Feb 27, 2020 at 8:53 AM Udo Kohlmeyer  wrote:

> @Bruce, Thank you for bringing this up. The first iteration of this
> would specifically target only the server-side. BUT, I do agree, that
> clients could suffer similar problems. Yet, from experience, I (and many
> other users) have deployed different versions of the Spring Framework
> (compared to Geode) with success.
>
> Generally it is easier to resolve client-side dependencies than server,
> as one has more control over the client than a deployed (managed) server
> instance.
>
> But if we think that there is a specific scenario that a client is
> suffering from, I would be more than happy to make sure that this is
> rolled out to the client shortly after having delivered it into the server.
>
> --Udo
>
> On 2/27/20 8:45 AM, Bruce Schuchardt wrote:
> > Udo, how does this relate to the client cache?  I assume people have the
> same problems with dependencies in client-cache applications that they have
> in functions that they deploy on a server-cache.
> >
> > On 2/26/20, 10:10 AM, "Udo Kohlmeyer"  wrote:
> >
> >  Hi there Geode Dev's.
> >
> >  There is a new RFC proposal on ClassLoader Isolation. The review end
> >  date is 13 Feb 2020.
> >
> >
> https://cwiki.apache.org/confluence/display/GEODE/ClassLoader+Isolation
> >  <
> https://cwiki.apache.org/confluence/display/GEODE/ClassLoader+Isolation>
> >
> >  Please review and discuss in this thread.
> >
> >  --Udo
> >
> >
> >
> >
>


-- 
-John
Spring Data Team

Re: [DISCUSSION] - ClassLoader Isolation

[Udo Kohlmeyer]

@Bruce, Thank you for bringing this up. The first iteration of this 
would specifically target only the server-side. BUT, I do agree, that 
clients could suffer similar problems. Yet, from experience, I (and many 
other users) have deployed different versions of the Spring Framework 
(compared to Geode) with success.


Generally it is easier to resolve client-side dependencies than server, 
as one has more control over the client than a deployed (managed) server 
instance.


But if we think that there is a specific scenario that a client is 
suffering from, I would be more than happy to make sure that this is 
rolled out to the client shortly after having delivered it into the server.


--Udo

On 2/27/20 8:45 AM, Bruce Schuchardt wrote:

Udo, how does this relate to the client cache?  I assume people have the same 
problems with dependencies in client-cache applications that they have in 
functions that they deploy on a server-cache.

On 2/26/20, 10:10 AM, "Udo Kohlmeyer"  wrote:

 Hi there Geode Dev's.
 
 There is a new RFC proposal on ClassLoader Isolation. The review end

 date is 13 Feb 2020.
 
 https://cwiki.apache.org/confluence/display/GEODE/ClassLoader+Isolation

 
 
 Please review and discuss in this thread.
 
 --Udo
 
 

Re: [DISCUSSION] - ClassLoader Isolation

[Bruce Schuchardt]

Udo, how does this relate to the client cache?  I assume people have the same 
problems with dependencies in client-cache applications that they have in 
functions that they deploy on a server-cache.

On 2/26/20, 10:10 AM, "Udo Kohlmeyer"  wrote:

Hi there Geode Dev's.

There is a new RFC proposal on ClassLoader Isolation. The review end 
date is 13 Feb 2020.

https://cwiki.apache.org/confluence/display/GEODE/ClassLoader+Isolation 


Please review and discuss in this thread.

--Udo

Re: [PROPOSAL]: Include GEODE-7814 in Release 1.12.0

[Bruce Schuchardt]

+1

The change Juan has made corrects a problem introduced during membership 
refactoring.  Every cache operation message that's sent allocates several 
objects that used to be held in statics but were moved into instance variables 
at one point.  Juan's change moves these back into static variables.

On 2/27/20, 1:49 AM, "Ju@N"  wrote:

Hello devs,

I'd like to include the fix for GEODE-7814 [1] in release 1.12.0.
The change prevents a huge amount of unnecessary allocation of objects
while sending/receiving messages, improving the overall performance.
The SHA is db86faec699aca67c02325bca22dcd5b913ddfed [2].
Best regards.

[1]: https://issues.apache.org/jira/browse/GEODE-7814
[2]:

https://github.com/apache/geode/commit/db86faec699aca67c02325bca22dcd5b913ddfed

-- 
Ju@N

Re: [PROPOSAL]: Include GEODE-7814 in Release 1.12.0

[Donal Evans]

+1

On Thu, Feb 27, 2020, 06:43 Owen Nichols  wrote:

> +1
>
> On Thu, Feb 27, 2020 at 1:49 AM Ju@N  wrote:
>
> > Hello devs,
> >
> > I'd like to include the fix for GEODE-7814 [1] in release 1.12.0.
> > The change prevents a huge amount of unnecessary allocation of objects
> > while sending/receiving messages, improving the overall performance.
> > The SHA is db86faec699aca67c02325bca22dcd5b913ddfed [2].
> > Best regards.
> >
> > [1]: https://issues.apache.org/jira/browse/GEODE-7814
> > [2]:
> >
> >
> https://github.com/apache/geode/commit/db86faec699aca67c02325bca22dcd5b913ddfed
> >
> > --
> > Ju@N
> >
>

Re: [PROPOSAL]: Include GEODE-7814 in Release 1.12.0

[Owen Nichols]

+1

On Thu, Feb 27, 2020 at 1:49 AM Ju@N  wrote:

> Hello devs,
>
> I'd like to include the fix for GEODE-7814 [1] in release 1.12.0.
> The change prevents a huge amount of unnecessary allocation of objects
> while sending/receiving messages, improving the overall performance.
> The SHA is db86faec699aca67c02325bca22dcd5b913ddfed [2].
> Best regards.
>
> [1]: https://issues.apache.org/jira/browse/GEODE-7814
> [2]:
>
> https://github.com/apache/geode/commit/db86faec699aca67c02325bca22dcd5b913ddfed
>
> --
> Ju@N
>

Fwd: Function execute method thread safety

[John Martin]

Forwarding message that was initially sent to u...@geode.apache.org.

-- Forwarded message --
From: *aashish choudhary* 
Date: Thursday, February 27, 2020
Subject: Function execute method thread safety
To: u...@geode.apache.org


No-one?
We ended up synchronising execute method call. Is this a good approach
tonthis solution?


With best regards,
Ashish

On Tue, Feb 25, 2020, 11:42 PM aashish choudhary <
aashish.choudha...@gmail.com> wrote:

> We are suspecting that during call to execute method when we get arguments
> that time shared state of request object is running into threading issue.
> Is that one of the possibility?
>
> With best regards,
> Ashish
>
> On Tue, Feb 25, 2020, 4:32 PM aashish choudhary <
> aashish.choudha...@gmail.com> wrote:
>
>> We are experiencing some threading issue while executing function with
>> multiple threads. Although documents says that execute method needs to be
>> thread safe. https://geode.apache.org/releases/latest/javadoc/org/
>> apache/geode/cache/execute/Function.html
>>
>> Will it not make the calls sequential and slow things down.?
>>
>> Thoughts?
>>
>> With best regards,
>> Ashish
>>
>


-- 
*John Martin*
Staff Product Manager, Pivotal Cloud Cache
Cell: 734.545.2153   |   jomar...@pivotal.io

[PROPOSAL]: Include GEODE-7814 in Release 1.12.0

[Ju@N]

Hello devs,

I'd like to include the fix for GEODE-7814 [1] in release 1.12.0.
The change prevents a huge amount of unnecessary allocation of objects
while sending/receiving messages, improving the overall performance.
The SHA is db86faec699aca67c02325bca22dcd5b913ddfed [2].
Best regards.

[1]: https://issues.apache.org/jira/browse/GEODE-7814
[2]:
https://github.com/apache/geode/commit/db86faec699aca67c02325bca22dcd5b913ddfed

-- 
Ju@N