geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patrick Rhomberg <prhomb...@pivotal.io>
Subject Re: [Discuss] Scope of the CacheElement interface with the respect to the public cluster configuration API
Date Wed, 25 Apr 2018 23:11:10 GMT
Again, my concern is with documentation and direction, not necessarily
going through and making these changes at right now.

On Wed, Apr 25, 2018 at 3:45 PM, Jinmei Liao <jiliao@pivotal.io> wrote:

> Any config object that needs to be Identifiable and Serializable should be
> identified as a CacheElement, but we don't have to go through all of the
> objects and identify them at once. When we are refactoring each individual
> commands and recognize the need, we will identify the object to be a
> CacheElement.
>
> On Wed, Apr 25, 2018 at 3:41 PM, Patrick Rhomberg <prhomberg@pivotal.io>
> wrote:
>
> > Sorry, I forgot to step away from the In The Weeds where I am.
> >
> > o.a.g.cache.configuration.CacheElement is the experimental interface
> meant
> > to identify an extension's configuration classes for consumption with the
> > new cluster configuration interface.  Extension developers can then use
> the
> > configuration service to disseminate these configuration objects via
> member
> > groups, consistent with other configuration elements (e.g., indices and
> > gateways).  See [1] for the API's proposal.
> >
> > We also have been using this interface internally on some of our own
> > configuration objects, for instance the JDBC's ConnectorService.  This
> > allowed us to use some utilities we had made for the easy retrieval of
> the
> > configuration objects.
> >
> > The lack of documentation on this interface is my core concern.  The
> > initial intent was to label those extension's configuration classes, but
> > beyond that, its use is ambiguous.  Even within that purview, it's not
> > clear whether an inner class should be required to implement
> CacheElement.
> >
> > Jinmei and I had a thread recently on a pull request of hers [2]
> suggesting
> > perhaps "anything you want to be searchable in the config" should have
> the
> > interface applied to it.  That remains more subjective than I'd
> personally
> > like, but it would at least be documentable.
> >
> > So to return to my initial concerns: our current use of the interface is
> > inconsistent, and with it being an external API, we should properly
> define
> > the interface's scope for both external and internal use.  I propose four
> > possible scopes in the initial message.
> >
> > @Udo, I haven't added a section on the linked proposal, but the page
> could
> > do with a larger update in any case.  I'll start that straight away.
> >
> > [1] Public API for Cluster Configuration Proposal:
> > https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=75975896
> > [2] PR #1853: https://github.com/apache/geode/pull/1853
> >
> > On Wed, Apr 25, 2018 at 12:56 PM, Galen O'Sullivan <
> gosullivan@pivotal.io>
> > wrote:
> >
> > > 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 used. Can you explain the
> > basics
> > > of how it's used?
> > >
> > > Thanks,
> > >
> > > Galen
> > >
> > >
> > >
> > > On 4/19/18 5:03 PM, Patrick Rhomberg wrote:
> > >
> > >> Hello all!
> > >>
> > >>    We introduced the CacheElement interface as part of our
> experimental
> > >> API
> > >> to update the cluster configuration.  I'd like to solidify and
> document
> > >> our
> > >> intent for the interface and the extent to which it is expected to
> > apply.
> > >>    In its current form, the CacheElement interface extends
> Serlializable
> > >> and
> > >> Identifiable<String>.  Serialization is required for communication
> > between
> > >> members, and Identifiable is useful during lookup, modification, and
> > >> removal of an existing configuration object.
> > >>    At our current iteration, it is not entirely clear which of our own
> > >> configuration objects should or should not implement CacheElement.  I
> > >> think
> > >> one of following interpretations may be best, but don't know which
> would
> > >> be
> > >> most natural.  And, of course, if you have another that I've
> overlooked,
> > >> I'd love to hear it.
> > >>
> > >> Classes that implement CacheElement are...
> > >>
> > >> A) ... only custom configuration elements provided by extension
> > >> developers.  This declaration will appear at a top-level class
> > declaration
> > >> that is directly consumed by the CacheConfig or RegionConfig.
> > >>
> > >> B) ... custom configuration elements, as well as any configuration
> > object
> > >> that we wish to be searchable within the configuration.
> > >>
> > >> C) ... custom configuration elements, as well as any configuration
> > object
> > >> appearing at the same level in the configuration hierarchy.  That is,
> > >> every
> > >> getter of both CacheConfig and RegionConfig should return either a
> > >> CacheElement, or a List<CacheElement>.
> > >>
> > >> D) ... any object, excepting those in java.lang.*, that appears
> anywhere
> > >> in
> > >> the cluster configuration's hierarchy, including all inner classes.
> For
> > >> example, ConnectorService.RegionMapping implements CacheElement.
> > >>
> > >>
> > >>    Each comes with its own potential pitfalls.
> > >>    Option (A) seems restrictive and sparse.
> > >>    Option (B) is subjective.
> > >>    Many classes do not have a `name' or `id' field, making option (D)
> > >> difficult.
> > >>    I like option (C), although we have already moved beyond that
> > >> specification in our current iteration, with
> > >> ConnectorService.RegionMapping
> > >> implementing CacheElement.
> > >>
> > >> Thoughts?
> > >>
> > >> Imagination is Change.
> > >> ~Patrick Rhomberg
> > >>
> > >>
> > >
> >
>
>
>
> --
> Cheers
>
> Jinmei
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message