You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by John Blum <jb...@pivotal.io> on 2017/05/17 23:42:16 UTC
org.apache.geode.internal.InternalDataSerializer API Changes...
Geode devs-
I am not sure how decisions are made when changing Geode's API, but I would
caution that more care should be taken when doing so, regardless of whether
the APIs are public or not, but especially when they are public.
Unfortunately, in this particular case, the API in question is deemed
"internal", which I know, are subject to change. However, the problem with
this is, the public API is insufficient in some cases, particularly for
"frameworks" (e.g. SDG) and "tooling" building on Geode and attempting to
uphold Geode's functional/behavioral contract.
By way of example, a recent *Spring Data Geode* build broke
<https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the
org.apache.geode.internal.InternalDataSerializer class was recently
changed. The scope of getSerializer(:Class):DataSerializer was changed
from *public
<https://github.com/apache/geode/blob/rel/v1.1.1/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#L1113>*
[2]
to *private
<https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd829315b34ce316/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#L1090>*
[3]
in a seemingly unrelated commit.
There is a class
<https://github.com/spring-projects/spring-data-geode/blob/master/src/main/java/org/springframework/data/gemfire/serialization/EnumSerializer.java#L39>
[4]
in SDG that extends DataSerializer
<http://gemfire-90-javadocs.docs.pivotal.io/org/apache/geode/DataSerializer.html>
[5]
(in Geode's public API) but must use the InternalDataSerializer (internal
Geode API) when changes (e.g. "supported classes") to the SDG serializer
need to be "distributed" across the cluster. SDG"s serializer use to call
this getSerializer(:Class):DataSerializer method. However, in this case, I
fixed the problem by not calling this "overloaded" method as it was not
strictly necessary.
While I am trying to avoid the use of "internal" Geode classes and APIs in
SDG in general, as I mention above, this is not always possible. For
instance, there are a few API blunders such as the ability to register
<http://gemfire-90-javadocs.docs.pivotal.io/org/apache/geode/DataSerializer.html#register-java.lang.Class->
[6]
a DataSerializer class but not "unregister" it; that is in
<https://github.com/apache/geode/blob/rel/v1.1.1/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#L1077>
[7]
InternalDataSerializer. The other bad API practices on this class (i.e.
(Internal)DataSerializer) alone.
Framework and tool developers must occasionally rely on "internal" APIs
(especially when the public API is insufficient) to order to achieve a
similar experience to Geode alone; something to keep in mind as Geode's
ecosystem grows.
Finally, I'll also add that, I do need to know anytime the public API
changes as well. Recently the getOnlineLocators() method
<https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6bd3baa942d3/geode-core/src/main/java/org/apache/geode/cache/client/Pool.java#L210>
[8]
was added to org.apache.geode.cache.client.Pool, which caused another SDG
build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail.
Special thank you to *Nabarun* and *Diane* for the recent heads about the
LuceneQueryFactory method name change... from setResultLimit(:int) to
setLimit(:int).
Regards,
--
-John
[1] https://build.spring.io/browse/SGF-NAG-556/log
[2]
https://github.com/apache/geode/blob/rel/v1.1.1/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#L1113
[3]
https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd829315b34ce316/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#L1090
[4]
https://github.com/spring-projects/spring-data-geode/blob/master/src/main/java/org/springframework/data/gemfire/serialization/EnumSerializer.java#L39
[5]
http://gemfire-90-javadocs.docs.pivotal.io/org/apache/geode/DataSerializer.html
[6]
http://gemfire-90-javadocs.docs.pivotal.io/org/apache/geode/DataSerializer.html#register-java.lang.Class-
[7]
https://github.com/apache/geode/blob/rel/v1.1.1/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#L1077
[8]
https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6bd3baa942d3/geode-core/src/main/java/org/apache/geode/cache/client/Pool.java#L210
[9] https://build.spring.io/browse/SGF-NAG-552
Re: org.apache.geode.internal.InternalDataSerializer API Changes...
Posted by John Blum <jb...@pivotal.io>.
Thanks Kirk and team!
+1 for bubbling up some of the internal APIs and having consistency and/or
functional parity.
One suggestion is, perhaps a quick note could be sent out to the Geode devs
with a title/subject of "*NOTICE: Frameworks/Tooling*" with a body
detailing the API (public/internal) changes that would affect 3rd party
projects (e.g. access modifier changes, removing methods, changing
signatures, introducing new types/properties, etc)
Another suggestion... it might be nice to list the more formal/official
projects, like SDG, that the community is working on for the Apache Geode
ecosystem. I literally have no idea what additional projects exist that
might benefit the community. *Pivotal GemFire* could be even counted among
these ecosystem projects/products.... perhaps another section similar to "
*Deployments*" on the Community page <http://geode.apache.org/community/>
[1].
Food for thought.
Thanks,
John
[1] http://geode.apache.org/community/
On Thu, May 18, 2017 at 10:31 AM, Kirk Lund <kl...@apache.org> wrote:
> I think we should definitely try to refactor any Geode internals that SDG
> is using to be public API or SPI. We'll need to take a closer look at how
> SDG is using those classes to plan what to do.
>
> If I change or any classes on your list (or public APIs), I'll give you
> heads up next time.
>
> On Thu, May 18, 2017 at 10:21 AM, John Blum <jb...@pivotal.io> wrote:
>
> > Hi Kirk (also responding to Bruce as I was writing this before he sent)-
> >
> > Thank you for following up.
> >
> > Regarding the addition of the getOnlineLocators()...
> >
> > SDG does implement
> > <http://docs.spring.io/spring-data-gemfire/docs/current/api/
> > org/springframework/data/gemfire/client/support/package-summary.html>
> > [1]
> > a few Geode interfaces, such as *org.apache.geode.cache.client.Pool*.
> > This
> > is due in part since not all the state for certain Geode objects (e.g.
> > Pool) is known in advance as the actual object may not have been created
> by
> > the *Spring* container yet. However, that state is captured in *Spring*
> > FactoryBeans, which will be used to construct the actual object at the
> > appropriate time.
> >
> > Anyway, like the "internal" API, I have tried to keep such
> implementations
> > of Geode interfaces/classes to a minimum. Still, it is a good indicator
> of
> > when something has changed since I will most certainly get a compilation
> > failure. This leads me to, I do need to know when APIs change to provide
> > appropriate support in SDG, possibly in configuration, whether XML of via
> > Java/annotations with the FactoryBeans, e.g. PoolFactoryBean
> > <http://docs.spring.io/spring-data-gemfire/docs/current/api/
> > org/springframework/data/gemfire/client/PoolFactoryBean.html>
> > [2],
> > especially if it is a configuration property/attribute. Other cases my
> be
> > important from an accessibility or information standpoint (probably more
> so
> > for tooling).
> >
> > As for the InternalDataSerializer...
> >
> > No worries. I found an acceptable and actually better workaround.
> Again,
> > I am trying to reduce or completely eliminate the use of "internal"
> classes
> > in SDG. Unfortunately, a quick search (using regex) in SDG for "import
> > org\.apache\.geode.*internal" reveals 35 occurrences, the most pertinent
> > ones being...
> >
> > org.apache.geode.internal.GemFireVersion (used in debugging)
> > org.apache.geode.internal.DistributionLocator
> > org.apache.geode.internal.InternalDataSerializer
> > org.apache.geode.internal.cache.GemFireCacheImpl
> > org.apache.geode.internal.cache.LocalRegion (used to inspect whether a
> > server proxy is present to acquire the appropriate QueryService)
> > org.apache.geode.internal.cache.PartitionedRegion
> > org.apache.geode.internal.cache.UserSpecifiedRegionAttributes
> > org.apache.geode.internal.cache.lru.LRUCapacityController
> > org.apache.geode.internal.cache.lru.MemLRUCapacityController
> > org.apache.geode.internal.cache.query.internal.ResultsBag
> > org.apache.geode.internal.distributed.internal.DistributionConfig
> > org.apache.geode.internal.distributed.internal.InternalDistributedSystem
> > org.apache.geode.internal.distributed.internal.InternalLocator
> > org.apache.geode.internal.datasource.ConfigProperty (JNDI support)
> > org.apache.geode.internal.jndi.JNDIInvoker (JNDI support)
> > org.apache.geode.internal.security.SecurityService (my doing)
> >
> > Most of this was added before my time. I just need time to review these
> > more carefully, and how each is used. I know many of these are used for
> > good reason.
> >
> > Thanks,
> > John
> >
> >
> > [1]
> > http://docs.spring.io/spring-data-gemfire/docs/current/api/
> > org/springframework/data/gemfire/client/support/package-summary.html
> > [2]
> > http://docs.spring.io/spring-data-gemfire/docs/current/api/
> > org/springframework/data/gemfire/client/PoolFactoryBean.html
> >
> >
> > On Thu, May 18, 2017 at 10:18 AM, Bruce Schuchardt <
> bschuchardt@pivotal.io
> > >
> > wrote:
> >
> > > John, is it possible to list the internal APIs needed by SDG? I don't
> > > think we can live with a requirement that all internal API changes need
> > to
> > > be communicated and blessed. We need to address SDG's needs in the
> > public
> > > APIs and get rid of this dependency.
> > >
> > >
> > > Le 5/18/2017 à 9:36 AM, Kirk Lund a écrit :
> > >
> > >> Hi John! How did the addition of getOnlineLocators() cause SDG build
> to
> > >> fail? I checked [9] but I couldn't quite grok what happened. I
> would've
> > >> thought that adding would be ok but removing could potentially break
> > SDG.
> > >>
> > >> The InternalDataSerializer change was probably mine. I reduced scope
> of
> > >> methods and variables where possible. Sorry about that. Do you need me
> > to
> > >> restore getSerializer to public?
> > >>
> > >> On Wed, May 17, 2017 at 4:42 PM, John Blum <jb...@pivotal.io> wrote:
> > >>
> > >> Geode devs-
> > >>>
> > >>> I am not sure how decisions are made when changing Geode's API, but I
> > >>> would
> > >>> caution that more care should be taken when doing so, regardless of
> > >>> whether
> > >>> the APIs are public or not, but especially when they are public.
> > >>>
> > >>> Unfortunately, in this particular case, the API in question is deemed
> > >>> "internal", which I know, are subject to change. However, the
> problem
> > >>> with
> > >>> this is, the public API is insufficient in some cases, particularly
> for
> > >>> "frameworks" (e.g. SDG) and "tooling" building on Geode and
> attempting
> > to
> > >>> uphold Geode's functional/behavioral contract.
> > >>>
> > >>> By way of example, a recent *Spring Data Geode* build broke
> > >>> <https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the
> > >>> org.apache.geode.internal.InternalDataSerializer class was recently
> > >>> changed. The scope of getSerializer(:Class):DataSerializer was
> changed
> > >>> from *public
> > >>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> > >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> > >>> ializer.java#
> > >>> L1113>*
> > >>> [2]
> > >>> to *private
> > >>> <https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
> > >>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
> > >>> InternalDataSerializer.java#L1090>*
> > >>> [3]
> > >>> in a seemingly unrelated commit.
> > >>>
> > >>> There is a class
> > >>> <https://github.com/spring-projects/spring-data-geode/
> > >>> blob/master/src/main/java/org/springframework/data/gemfire/
> > >>> serialization/EnumSerializer.java#L39>
> > >>> [4]
> > >>> in SDG that extends DataSerializer
> > >>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> > >>> geode/DataSerializer.html>
> > >>> [5]
> > >>> (in Geode's public API) but must use the InternalDataSerializer
> > (internal
> > >>> Geode API) when changes (e.g. "supported classes") to the SDG
> > serializer
> > >>> need to be "distributed" across the cluster. SDG"s serializer use to
> > >>> call
> > >>> this getSerializer(:Class):DataSerializer method. However, in this
> > >>> case,
> > >>> I
> > >>> fixed the problem by not calling this "overloaded" method as it was
> not
> > >>> strictly necessary.
> > >>>
> > >>> While I am trying to avoid the use of "internal" Geode classes and
> APIs
> > >>> in
> > >>> SDG in general, as I mention above, this is not always possible. For
> > >>> instance, there are a few API blunders such as the ability to
> register
> > >>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> > >>> geode/DataSerializer.html#register-java.lang.Class->
> > >>> [6]
> > >>> a DataSerializer class but not "unregister" it; that is in
> > >>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> > >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> > >>> ializer.java#
> > >>> L1077>
> > >>> [7]
> > >>> InternalDataSerializer. The other bad API practices on this class
> > (i.e.
> > >>> (Internal)DataSerializer) alone.
> > >>>
> > >>> Framework and tool developers must occasionally rely on "internal"
> APIs
> > >>> (especially when the public API is insufficient) to order to achieve
> a
> > >>> similar experience to Geode alone; something to keep in mind as
> Geode's
> > >>> ecosystem grows.
> > >>>
> > >>> Finally, I'll also add that, I do need to know anytime the public API
> > >>> changes as well. Recently the getOnlineLocators() method
> > >>> <https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
> > >>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
> > >>> cache/client/Pool.java#L210>
> > >>> [8]
> > >>> was added to org.apache.geode.cache.client.Pool, which caused
> another
> > >>> SDG
> > >>> build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail.
> > >>>
> > >>> Special thank you to *Nabarun* and *Diane* for the recent heads about
> > the
> > >>> LuceneQueryFactory method name change... from setResultLimit(:int) to
> > >>> setLimit(:int).
> > >>>
> > >>> Regards,
> > >>>
> > >>> --
> > >>> -John
> > >>>
> > >>>
> > >>> [1] https://build.spring.io/browse/SGF-NAG-556/log
> > >>> [2]
> > >>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> > >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> > >>> ializer.java#
> > >>> L1113
> > >>> [3]
> > >>> https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
> > >>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
> > >>> InternalDataSerializer.java#L1090
> > >>> [4]
> > >>> https://github.com/spring-projects/spring-data-geode/
> > >>> blob/master/src/main/java/org/springframework/data/gemfire/
> > >>> serialization/EnumSerializer.java#L39
> > >>> [5]
> > >>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> > >>> geode/DataSerializer.html
> > >>> [6]
> > >>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> > >>> geode/DataSerializer.html#register-java.lang.Class-
> > >>> [7]
> > >>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> > >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> > >>> ializer.java#
> > >>> L1077
> > >>> [8]
> > >>> https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
> > >>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
> > >>> cache/client/Pool.java#L210
> > >>> [9] https://build.spring.io/browse/SGF-NAG-552
> > >>>
> > >>>
> > >
> >
> >
> > --
> > -John
> > john.blum10101 (skype)
> >
>
--
-John
john.blum10101 (skype)
Re: org.apache.geode.internal.InternalDataSerializer API Changes...
Posted by Kirk Lund <kl...@apache.org>.
I think we should definitely try to refactor any Geode internals that SDG
is using to be public API or SPI. We'll need to take a closer look at how
SDG is using those classes to plan what to do.
If I change or any classes on your list (or public APIs), I'll give you
heads up next time.
On Thu, May 18, 2017 at 10:21 AM, John Blum <jb...@pivotal.io> wrote:
> Hi Kirk (also responding to Bruce as I was writing this before he sent)-
>
> Thank you for following up.
>
> Regarding the addition of the getOnlineLocators()...
>
> SDG does implement
> <http://docs.spring.io/spring-data-gemfire/docs/current/api/
> org/springframework/data/gemfire/client/support/package-summary.html>
> [1]
> a few Geode interfaces, such as *org.apache.geode.cache.client.Pool*.
> This
> is due in part since not all the state for certain Geode objects (e.g.
> Pool) is known in advance as the actual object may not have been created by
> the *Spring* container yet. However, that state is captured in *Spring*
> FactoryBeans, which will be used to construct the actual object at the
> appropriate time.
>
> Anyway, like the "internal" API, I have tried to keep such implementations
> of Geode interfaces/classes to a minimum. Still, it is a good indicator of
> when something has changed since I will most certainly get a compilation
> failure. This leads me to, I do need to know when APIs change to provide
> appropriate support in SDG, possibly in configuration, whether XML of via
> Java/annotations with the FactoryBeans, e.g. PoolFactoryBean
> <http://docs.spring.io/spring-data-gemfire/docs/current/api/
> org/springframework/data/gemfire/client/PoolFactoryBean.html>
> [2],
> especially if it is a configuration property/attribute. Other cases my be
> important from an accessibility or information standpoint (probably more so
> for tooling).
>
> As for the InternalDataSerializer...
>
> No worries. I found an acceptable and actually better workaround. Again,
> I am trying to reduce or completely eliminate the use of "internal" classes
> in SDG. Unfortunately, a quick search (using regex) in SDG for "import
> org\.apache\.geode.*internal" reveals 35 occurrences, the most pertinent
> ones being...
>
> org.apache.geode.internal.GemFireVersion (used in debugging)
> org.apache.geode.internal.DistributionLocator
> org.apache.geode.internal.InternalDataSerializer
> org.apache.geode.internal.cache.GemFireCacheImpl
> org.apache.geode.internal.cache.LocalRegion (used to inspect whether a
> server proxy is present to acquire the appropriate QueryService)
> org.apache.geode.internal.cache.PartitionedRegion
> org.apache.geode.internal.cache.UserSpecifiedRegionAttributes
> org.apache.geode.internal.cache.lru.LRUCapacityController
> org.apache.geode.internal.cache.lru.MemLRUCapacityController
> org.apache.geode.internal.cache.query.internal.ResultsBag
> org.apache.geode.internal.distributed.internal.DistributionConfig
> org.apache.geode.internal.distributed.internal.InternalDistributedSystem
> org.apache.geode.internal.distributed.internal.InternalLocator
> org.apache.geode.internal.datasource.ConfigProperty (JNDI support)
> org.apache.geode.internal.jndi.JNDIInvoker (JNDI support)
> org.apache.geode.internal.security.SecurityService (my doing)
>
> Most of this was added before my time. I just need time to review these
> more carefully, and how each is used. I know many of these are used for
> good reason.
>
> Thanks,
> John
>
>
> [1]
> http://docs.spring.io/spring-data-gemfire/docs/current/api/
> org/springframework/data/gemfire/client/support/package-summary.html
> [2]
> http://docs.spring.io/spring-data-gemfire/docs/current/api/
> org/springframework/data/gemfire/client/PoolFactoryBean.html
>
>
> On Thu, May 18, 2017 at 10:18 AM, Bruce Schuchardt <bschuchardt@pivotal.io
> >
> wrote:
>
> > John, is it possible to list the internal APIs needed by SDG? I don't
> > think we can live with a requirement that all internal API changes need
> to
> > be communicated and blessed. We need to address SDG's needs in the
> public
> > APIs and get rid of this dependency.
> >
> >
> > Le 5/18/2017 à 9:36 AM, Kirk Lund a écrit :
> >
> >> Hi John! How did the addition of getOnlineLocators() cause SDG build to
> >> fail? I checked [9] but I couldn't quite grok what happened. I would've
> >> thought that adding would be ok but removing could potentially break
> SDG.
> >>
> >> The InternalDataSerializer change was probably mine. I reduced scope of
> >> methods and variables where possible. Sorry about that. Do you need me
> to
> >> restore getSerializer to public?
> >>
> >> On Wed, May 17, 2017 at 4:42 PM, John Blum <jb...@pivotal.io> wrote:
> >>
> >> Geode devs-
> >>>
> >>> I am not sure how decisions are made when changing Geode's API, but I
> >>> would
> >>> caution that more care should be taken when doing so, regardless of
> >>> whether
> >>> the APIs are public or not, but especially when they are public.
> >>>
> >>> Unfortunately, in this particular case, the API in question is deemed
> >>> "internal", which I know, are subject to change. However, the problem
> >>> with
> >>> this is, the public API is insufficient in some cases, particularly for
> >>> "frameworks" (e.g. SDG) and "tooling" building on Geode and attempting
> to
> >>> uphold Geode's functional/behavioral contract.
> >>>
> >>> By way of example, a recent *Spring Data Geode* build broke
> >>> <https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the
> >>> org.apache.geode.internal.InternalDataSerializer class was recently
> >>> changed. The scope of getSerializer(:Class):DataSerializer was changed
> >>> from *public
> >>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> >>> ializer.java#
> >>> L1113>*
> >>> [2]
> >>> to *private
> >>> <https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
> >>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
> >>> InternalDataSerializer.java#L1090>*
> >>> [3]
> >>> in a seemingly unrelated commit.
> >>>
> >>> There is a class
> >>> <https://github.com/spring-projects/spring-data-geode/
> >>> blob/master/src/main/java/org/springframework/data/gemfire/
> >>> serialization/EnumSerializer.java#L39>
> >>> [4]
> >>> in SDG that extends DataSerializer
> >>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> >>> geode/DataSerializer.html>
> >>> [5]
> >>> (in Geode's public API) but must use the InternalDataSerializer
> (internal
> >>> Geode API) when changes (e.g. "supported classes") to the SDG
> serializer
> >>> need to be "distributed" across the cluster. SDG"s serializer use to
> >>> call
> >>> this getSerializer(:Class):DataSerializer method. However, in this
> >>> case,
> >>> I
> >>> fixed the problem by not calling this "overloaded" method as it was not
> >>> strictly necessary.
> >>>
> >>> While I am trying to avoid the use of "internal" Geode classes and APIs
> >>> in
> >>> SDG in general, as I mention above, this is not always possible. For
> >>> instance, there are a few API blunders such as the ability to register
> >>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> >>> geode/DataSerializer.html#register-java.lang.Class->
> >>> [6]
> >>> a DataSerializer class but not "unregister" it; that is in
> >>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> >>> ializer.java#
> >>> L1077>
> >>> [7]
> >>> InternalDataSerializer. The other bad API practices on this class
> (i.e.
> >>> (Internal)DataSerializer) alone.
> >>>
> >>> Framework and tool developers must occasionally rely on "internal" APIs
> >>> (especially when the public API is insufficient) to order to achieve a
> >>> similar experience to Geode alone; something to keep in mind as Geode's
> >>> ecosystem grows.
> >>>
> >>> Finally, I'll also add that, I do need to know anytime the public API
> >>> changes as well. Recently the getOnlineLocators() method
> >>> <https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
> >>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
> >>> cache/client/Pool.java#L210>
> >>> [8]
> >>> was added to org.apache.geode.cache.client.Pool, which caused another
> >>> SDG
> >>> build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail.
> >>>
> >>> Special thank you to *Nabarun* and *Diane* for the recent heads about
> the
> >>> LuceneQueryFactory method name change... from setResultLimit(:int) to
> >>> setLimit(:int).
> >>>
> >>> Regards,
> >>>
> >>> --
> >>> -John
> >>>
> >>>
> >>> [1] https://build.spring.io/browse/SGF-NAG-556/log
> >>> [2]
> >>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> >>> ializer.java#
> >>> L1113
> >>> [3]
> >>> https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
> >>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
> >>> InternalDataSerializer.java#L1090
> >>> [4]
> >>> https://github.com/spring-projects/spring-data-geode/
> >>> blob/master/src/main/java/org/springframework/data/gemfire/
> >>> serialization/EnumSerializer.java#L39
> >>> [5]
> >>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> >>> geode/DataSerializer.html
> >>> [6]
> >>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> >>> geode/DataSerializer.html#register-java.lang.Class-
> >>> [7]
> >>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> >>> core/src/main/java/org/apache/geode/internal/InternalDataSer
> >>> ializer.java#
> >>> L1077
> >>> [8]
> >>> https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
> >>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
> >>> cache/client/Pool.java#L210
> >>> [9] https://build.spring.io/browse/SGF-NAG-552
> >>>
> >>>
> >
>
>
> --
> -John
> john.blum10101 (skype)
>
Re: org.apache.geode.internal.InternalDataSerializer API Changes...
Posted by John Blum <jb...@pivotal.io>.
Hi Kirk (also responding to Bruce as I was writing this before he sent)-
Thank you for following up.
Regarding the addition of the getOnlineLocators()...
SDG does implement
<http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/support/package-summary.html>
[1]
a few Geode interfaces, such as *org.apache.geode.cache.client.Pool*. This
is due in part since not all the state for certain Geode objects (e.g.
Pool) is known in advance as the actual object may not have been created by
the *Spring* container yet. However, that state is captured in *Spring*
FactoryBeans, which will be used to construct the actual object at the
appropriate time.
Anyway, like the "internal" API, I have tried to keep such implementations
of Geode interfaces/classes to a minimum. Still, it is a good indicator of
when something has changed since I will most certainly get a compilation
failure. This leads me to, I do need to know when APIs change to provide
appropriate support in SDG, possibly in configuration, whether XML of via
Java/annotations with the FactoryBeans, e.g. PoolFactoryBean
<http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/PoolFactoryBean.html>
[2],
especially if it is a configuration property/attribute. Other cases my be
important from an accessibility or information standpoint (probably more so
for tooling).
As for the InternalDataSerializer...
No worries. I found an acceptable and actually better workaround. Again,
I am trying to reduce or completely eliminate the use of "internal" classes
in SDG. Unfortunately, a quick search (using regex) in SDG for "import
org\.apache\.geode.*internal" reveals 35 occurrences, the most pertinent
ones being...
org.apache.geode.internal.GemFireVersion (used in debugging)
org.apache.geode.internal.DistributionLocator
org.apache.geode.internal.InternalDataSerializer
org.apache.geode.internal.cache.GemFireCacheImpl
org.apache.geode.internal.cache.LocalRegion (used to inspect whether a
server proxy is present to acquire the appropriate QueryService)
org.apache.geode.internal.cache.PartitionedRegion
org.apache.geode.internal.cache.UserSpecifiedRegionAttributes
org.apache.geode.internal.cache.lru.LRUCapacityController
org.apache.geode.internal.cache.lru.MemLRUCapacityController
org.apache.geode.internal.cache.query.internal.ResultsBag
org.apache.geode.internal.distributed.internal.DistributionConfig
org.apache.geode.internal.distributed.internal.InternalDistributedSystem
org.apache.geode.internal.distributed.internal.InternalLocator
org.apache.geode.internal.datasource.ConfigProperty (JNDI support)
org.apache.geode.internal.jndi.JNDIInvoker (JNDI support)
org.apache.geode.internal.security.SecurityService (my doing)
Most of this was added before my time. I just need time to review these
more carefully, and how each is used. I know many of these are used for
good reason.
Thanks,
John
[1]
http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/support/package-summary.html
[2]
http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/PoolFactoryBean.html
On Thu, May 18, 2017 at 10:18 AM, Bruce Schuchardt <bs...@pivotal.io>
wrote:
> John, is it possible to list the internal APIs needed by SDG? I don't
> think we can live with a requirement that all internal API changes need to
> be communicated and blessed. We need to address SDG's needs in the public
> APIs and get rid of this dependency.
>
>
> Le 5/18/2017 à 9:36 AM, Kirk Lund a écrit :
>
>> Hi John! How did the addition of getOnlineLocators() cause SDG build to
>> fail? I checked [9] but I couldn't quite grok what happened. I would've
>> thought that adding would be ok but removing could potentially break SDG.
>>
>> The InternalDataSerializer change was probably mine. I reduced scope of
>> methods and variables where possible. Sorry about that. Do you need me to
>> restore getSerializer to public?
>>
>> On Wed, May 17, 2017 at 4:42 PM, John Blum <jb...@pivotal.io> wrote:
>>
>> Geode devs-
>>>
>>> I am not sure how decisions are made when changing Geode's API, but I
>>> would
>>> caution that more care should be taken when doing so, regardless of
>>> whether
>>> the APIs are public or not, but especially when they are public.
>>>
>>> Unfortunately, in this particular case, the API in question is deemed
>>> "internal", which I know, are subject to change. However, the problem
>>> with
>>> this is, the public API is insufficient in some cases, particularly for
>>> "frameworks" (e.g. SDG) and "tooling" building on Geode and attempting to
>>> uphold Geode's functional/behavioral contract.
>>>
>>> By way of example, a recent *Spring Data Geode* build broke
>>> <https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the
>>> org.apache.geode.internal.InternalDataSerializer class was recently
>>> changed. The scope of getSerializer(:Class):DataSerializer was changed
>>> from *public
>>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>>> core/src/main/java/org/apache/geode/internal/InternalDataSer
>>> ializer.java#
>>> L1113>*
>>> [2]
>>> to *private
>>> <https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
>>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
>>> InternalDataSerializer.java#L1090>*
>>> [3]
>>> in a seemingly unrelated commit.
>>>
>>> There is a class
>>> <https://github.com/spring-projects/spring-data-geode/
>>> blob/master/src/main/java/org/springframework/data/gemfire/
>>> serialization/EnumSerializer.java#L39>
>>> [4]
>>> in SDG that extends DataSerializer
>>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>>> geode/DataSerializer.html>
>>> [5]
>>> (in Geode's public API) but must use the InternalDataSerializer (internal
>>> Geode API) when changes (e.g. "supported classes") to the SDG serializer
>>> need to be "distributed" across the cluster. SDG"s serializer use to
>>> call
>>> this getSerializer(:Class):DataSerializer method. However, in this
>>> case,
>>> I
>>> fixed the problem by not calling this "overloaded" method as it was not
>>> strictly necessary.
>>>
>>> While I am trying to avoid the use of "internal" Geode classes and APIs
>>> in
>>> SDG in general, as I mention above, this is not always possible. For
>>> instance, there are a few API blunders such as the ability to register
>>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>>> geode/DataSerializer.html#register-java.lang.Class->
>>> [6]
>>> a DataSerializer class but not "unregister" it; that is in
>>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>>> core/src/main/java/org/apache/geode/internal/InternalDataSer
>>> ializer.java#
>>> L1077>
>>> [7]
>>> InternalDataSerializer. The other bad API practices on this class (i.e.
>>> (Internal)DataSerializer) alone.
>>>
>>> Framework and tool developers must occasionally rely on "internal" APIs
>>> (especially when the public API is insufficient) to order to achieve a
>>> similar experience to Geode alone; something to keep in mind as Geode's
>>> ecosystem grows.
>>>
>>> Finally, I'll also add that, I do need to know anytime the public API
>>> changes as well. Recently the getOnlineLocators() method
>>> <https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
>>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
>>> cache/client/Pool.java#L210>
>>> [8]
>>> was added to org.apache.geode.cache.client.Pool, which caused another
>>> SDG
>>> build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail.
>>>
>>> Special thank you to *Nabarun* and *Diane* for the recent heads about the
>>> LuceneQueryFactory method name change... from setResultLimit(:int) to
>>> setLimit(:int).
>>>
>>> Regards,
>>>
>>> --
>>> -John
>>>
>>>
>>> [1] https://build.spring.io/browse/SGF-NAG-556/log
>>> [2]
>>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>>> core/src/main/java/org/apache/geode/internal/InternalDataSer
>>> ializer.java#
>>> L1113
>>> [3]
>>> https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
>>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
>>> InternalDataSerializer.java#L1090
>>> [4]
>>> https://github.com/spring-projects/spring-data-geode/
>>> blob/master/src/main/java/org/springframework/data/gemfire/
>>> serialization/EnumSerializer.java#L39
>>> [5]
>>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>>> geode/DataSerializer.html
>>> [6]
>>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>>> geode/DataSerializer.html#register-java.lang.Class-
>>> [7]
>>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>>> core/src/main/java/org/apache/geode/internal/InternalDataSer
>>> ializer.java#
>>> L1077
>>> [8]
>>> https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
>>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
>>> cache/client/Pool.java#L210
>>> [9] https://build.spring.io/browse/SGF-NAG-552
>>>
>>>
>
--
-John
john.blum10101 (skype)
Re: org.apache.geode.internal.InternalDataSerializer API Changes...
Posted by Bruce Schuchardt <bs...@pivotal.io>.
John, is it possible to list the internal APIs needed by SDG? I don't
think we can live with a requirement that all internal API changes need
to be communicated and blessed. We need to address SDG's needs in the
public APIs and get rid of this dependency.
Le 5/18/2017 à 9:36 AM, Kirk Lund a écrit :
> Hi John! How did the addition of getOnlineLocators() cause SDG build to
> fail? I checked [9] but I couldn't quite grok what happened. I would've
> thought that adding would be ok but removing could potentially break SDG.
>
> The InternalDataSerializer change was probably mine. I reduced scope of
> methods and variables where possible. Sorry about that. Do you need me to
> restore getSerializer to public?
>
> On Wed, May 17, 2017 at 4:42 PM, John Blum <jb...@pivotal.io> wrote:
>
>> Geode devs-
>>
>> I am not sure how decisions are made when changing Geode's API, but I would
>> caution that more care should be taken when doing so, regardless of whether
>> the APIs are public or not, but especially when they are public.
>>
>> Unfortunately, in this particular case, the API in question is deemed
>> "internal", which I know, are subject to change. However, the problem with
>> this is, the public API is insufficient in some cases, particularly for
>> "frameworks" (e.g. SDG) and "tooling" building on Geode and attempting to
>> uphold Geode's functional/behavioral contract.
>>
>> By way of example, a recent *Spring Data Geode* build broke
>> <https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the
>> org.apache.geode.internal.InternalDataSerializer class was recently
>> changed. The scope of getSerializer(:Class):DataSerializer was changed
>> from *public
>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
>> L1113>*
>> [2]
>> to *private
>> <https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
>> InternalDataSerializer.java#L1090>*
>> [3]
>> in a seemingly unrelated commit.
>>
>> There is a class
>> <https://github.com/spring-projects/spring-data-geode/
>> blob/master/src/main/java/org/springframework/data/gemfire/
>> serialization/EnumSerializer.java#L39>
>> [4]
>> in SDG that extends DataSerializer
>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>> geode/DataSerializer.html>
>> [5]
>> (in Geode's public API) but must use the InternalDataSerializer (internal
>> Geode API) when changes (e.g. "supported classes") to the SDG serializer
>> need to be "distributed" across the cluster. SDG"s serializer use to call
>> this getSerializer(:Class):DataSerializer method. However, in this case,
>> I
>> fixed the problem by not calling this "overloaded" method as it was not
>> strictly necessary.
>>
>> While I am trying to avoid the use of "internal" Geode classes and APIs in
>> SDG in general, as I mention above, this is not always possible. For
>> instance, there are a few API blunders such as the ability to register
>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>> geode/DataSerializer.html#register-java.lang.Class->
>> [6]
>> a DataSerializer class but not "unregister" it; that is in
>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
>> L1077>
>> [7]
>> InternalDataSerializer. The other bad API practices on this class (i.e.
>> (Internal)DataSerializer) alone.
>>
>> Framework and tool developers must occasionally rely on "internal" APIs
>> (especially when the public API is insufficient) to order to achieve a
>> similar experience to Geode alone; something to keep in mind as Geode's
>> ecosystem grows.
>>
>> Finally, I'll also add that, I do need to know anytime the public API
>> changes as well. Recently the getOnlineLocators() method
>> <https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
>> cache/client/Pool.java#L210>
>> [8]
>> was added to org.apache.geode.cache.client.Pool, which caused another SDG
>> build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail.
>>
>> Special thank you to *Nabarun* and *Diane* for the recent heads about the
>> LuceneQueryFactory method name change... from setResultLimit(:int) to
>> setLimit(:int).
>>
>> Regards,
>>
>> --
>> -John
>>
>>
>> [1] https://build.spring.io/browse/SGF-NAG-556/log
>> [2]
>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
>> L1113
>> [3]
>> https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
>> InternalDataSerializer.java#L1090
>> [4]
>> https://github.com/spring-projects/spring-data-geode/
>> blob/master/src/main/java/org/springframework/data/gemfire/
>> serialization/EnumSerializer.java#L39
>> [5]
>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>> geode/DataSerializer.html
>> [6]
>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
>> geode/DataSerializer.html#register-java.lang.Class-
>> [7]
>> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
>> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
>> L1077
>> [8]
>> https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
>> d3baa942d3/geode-core/src/main/java/org/apache/geode/
>> cache/client/Pool.java#L210
>> [9] https://build.spring.io/browse/SGF-NAG-552
>>
Re: org.apache.geode.internal.InternalDataSerializer API Changes...
Posted by Kirk Lund <kl...@apache.org>.
Hi John! How did the addition of getOnlineLocators() cause SDG build to
fail? I checked [9] but I couldn't quite grok what happened. I would've
thought that adding would be ok but removing could potentially break SDG.
The InternalDataSerializer change was probably mine. I reduced scope of
methods and variables where possible. Sorry about that. Do you need me to
restore getSerializer to public?
On Wed, May 17, 2017 at 4:42 PM, John Blum <jb...@pivotal.io> wrote:
> Geode devs-
>
> I am not sure how decisions are made when changing Geode's API, but I would
> caution that more care should be taken when doing so, regardless of whether
> the APIs are public or not, but especially when they are public.
>
> Unfortunately, in this particular case, the API in question is deemed
> "internal", which I know, are subject to change. However, the problem with
> this is, the public API is insufficient in some cases, particularly for
> "frameworks" (e.g. SDG) and "tooling" building on Geode and attempting to
> uphold Geode's functional/behavioral contract.
>
> By way of example, a recent *Spring Data Geode* build broke
> <https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the
> org.apache.geode.internal.InternalDataSerializer class was recently
> changed. The scope of getSerializer(:Class):DataSerializer was changed
> from *public
> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
> L1113>*
> [2]
> to *private
> <https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
> InternalDataSerializer.java#L1090>*
> [3]
> in a seemingly unrelated commit.
>
> There is a class
> <https://github.com/spring-projects/spring-data-geode/
> blob/master/src/main/java/org/springframework/data/gemfire/
> serialization/EnumSerializer.java#L39>
> [4]
> in SDG that extends DataSerializer
> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> geode/DataSerializer.html>
> [5]
> (in Geode's public API) but must use the InternalDataSerializer (internal
> Geode API) when changes (e.g. "supported classes") to the SDG serializer
> need to be "distributed" across the cluster. SDG"s serializer use to call
> this getSerializer(:Class):DataSerializer method. However, in this case,
> I
> fixed the problem by not calling this "overloaded" method as it was not
> strictly necessary.
>
> While I am trying to avoid the use of "internal" Geode classes and APIs in
> SDG in general, as I mention above, this is not always possible. For
> instance, there are a few API blunders such as the ability to register
> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> geode/DataSerializer.html#register-java.lang.Class->
> [6]
> a DataSerializer class but not "unregister" it; that is in
> <https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
> L1077>
> [7]
> InternalDataSerializer. The other bad API practices on this class (i.e.
> (Internal)DataSerializer) alone.
>
> Framework and tool developers must occasionally rely on "internal" APIs
> (especially when the public API is insufficient) to order to achieve a
> similar experience to Geode alone; something to keep in mind as Geode's
> ecosystem grows.
>
> Finally, I'll also add that, I do need to know anytime the public API
> changes as well. Recently the getOnlineLocators() method
> <https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
> d3baa942d3/geode-core/src/main/java/org/apache/geode/
> cache/client/Pool.java#L210>
> [8]
> was added to org.apache.geode.cache.client.Pool, which caused another SDG
> build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail.
>
> Special thank you to *Nabarun* and *Diane* for the recent heads about the
> LuceneQueryFactory method name change... from setResultLimit(:int) to
> setLimit(:int).
>
> Regards,
>
> --
> -John
>
>
> [1] https://build.spring.io/browse/SGF-NAG-556/log
> [2]
> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
> L1113
> [3]
> https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293
> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/
> InternalDataSerializer.java#L1090
> [4]
> https://github.com/spring-projects/spring-data-geode/
> blob/master/src/main/java/org/springframework/data/gemfire/
> serialization/EnumSerializer.java#L39
> [5]
> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> geode/DataSerializer.html
> [6]
> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/
> geode/DataSerializer.html#register-java.lang.Class-
> [7]
> https://github.com/apache/geode/blob/rel/v1.1.1/geode-
> core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java#
> L1077
> [8]
> https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b
> d3baa942d3/geode-core/src/main/java/org/apache/geode/
> cache/client/Pool.java#L210
> [9] https://build.spring.io/browse/SGF-NAG-552
>