geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kirk Lund <kl...@apache.org>
Subject Re: [DISCUSS] changes to registerInterest API
Date Mon, 20 Nov 2017 17:05:21 GMT
John's approach looks best for when you need to specify keys.

For ALL_KEYS, what about an API that doesn't require a token or all keys:

public void registerInterestForAllKeys();

On Fri, Nov 17, 2017 at 1:24 PM, Jason Huynh <jhuynh@pivotal.io> wrote:

> Thanks John for the clarification!
>
> On Fri, Nov 17, 2017 at 1:12 PM John Blum <jblum@pivotal.io> wrote:
>
> > This...
> >
> > > The Iterable version would handle any collection type by having the
> user
> > pass
> > in the iterator for the collection.
> >
> > Is not correct.
> >
> > The Collection<E> interface itself "extends" the java.lang.Iterable<E>
> > interface (see here...
> > https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html
> under
> > "*All
> > Superinterfaces*").
> >
> > Therefore a user can simply to this...
> >
> > *List*<KeyType> keys = ...
> >
> > region.registerInterest(keys); *// calls the
> > Region.registerInterest(:Iterable<T>) method.*
> >
> > Alternatively, this would also be allowed...
> >
> > *Set*<KeyType> keys = ...
> >
> > region.registerInterest(keys);
> >
> >
> > On Fri, Nov 17, 2017 at 11:44 AM, Jason Huynh <jhuynh@pivotal.io> wrote:
> >
> > > Current idea is to:
> > > - deprecate current "ALL_KEYS" and List passing behavior in
> > > registerInterest()
> > > - add registerInterestAllKeys();
> > > - add registerInterest(T... keys) and registerInterest(Iterable<T>
> keys)
> > > and
> > > not have one specifically for List or specific collections.
> > >
> > > The Iterable version would handle any collection type by having the
> user
> > > pass in the iterator for the collection.
> > >
> > > On Fri, Nov 17, 2017 at 11:32 AM Jacob Barrett <jbarrett@pivotal.io>
> > > wrote:
> > >
> > > > I am failing to see where registerInterest(List<T> keys) is an issue
> > for
> > > > the key type in the region. If our region is Region<String> then
I
> > would
> > > > expect registerInterest(List<String>). If the keys are unknown or
a
> mix
> > > > then you should have Region<Object> and thus
> > > registerInterest(List<Object).
> > > >
> > > > I echo John's statements on VarArgs and type erasure as well as his
> > > > argument for Iterable<T>.
> > > >
> > > > Also, List<T> does not restrict you from List indexes. The region
> would
> > > be
> > > > Region<List<String>> with registerInterest<List<List<String>>().
> > > >
> > > > -Jake
> > > >
> > > >
> > > > On Fri, Nov 17, 2017 at 10:04 AM John Blum <jblum@pivotal.io> wrote:
> > > >
> > > > > Personally, I prefer the var args method (registerInterest(T...
> > keys))
> > > > > myself.  It is way more convenient if I only have a few keys when
> > > calling
> > > > > this method then to have to add the keys to a List, especially for
> > > > testing
> > > > > purposes.
> > > > >
> > > > > But, I typically like to pair that with a
> > registerInterest(Iterable<T>
> > > > > keys) method
> > > > > as well.  By having a overloaded Iterable variant, then I can pass
> in
> > > any
> > > > > Collection type I want (which shouldn't be restricted to just
> List).
> > > It
> > > > > also is a simple matter to convert any *Collection* (i.e. *List*,
> > > *Set*,
> > > > > etc) to an array, which can be passed to the var args method.  By
> > using
> > > > > List,
> > > > > you are implying that "order matters" since a List is a order
> > > collection
> > > > of
> > > > > elements.
> > > > >
> > > > > This ("*It might even cause problems of pushing in **multiple
> > different
> > > > > types.*"), regarding var args, does not even make sense.
> Technically,
> > > > > List<T> is no different.  Java's type erasure essentially equates
> var
> > > > args
> > > > > too "Object..." (or Object[]) and the List<T> to List (or a
List of
> > > > > Objects,
> > > > > essentially like if you just did this... List<Object>) So,
while
> the
> > > > > compiler ensures compile-time type-safety of generics, there is no
> > > > generics
> > > > > type-safety guarantees at runtime.
> > > > >
> > > > >
> > > > >
> > > > > On Fri, Nov 17, 2017 at 9:22 AM, Jason Huynh <jhuynh@pivotal.io>
> > > wrote:
> > > > >
> > > > > > Hi Mike,
> > > > > >
> > > > > > The current support for List leads to compilation issues if
the
> > > region
> > > > is
> > > > > > type constrained.  However I think you are suggesting instead
of
> a
> > > var
> > > > > args
> > > > > > method, instead provide a registerInterest(List keys) method?
> > > > > >
> > > > > > So far what I am hearing requested is:
> > > > > > deprecate current "ALL_KEYS" and List passing behavior
> > > > > > registerInterestAllKeys();
> > > > > > registerInterest(List<T> keys) instead of a registerInterest(T...
> > > keys)
> > > > > >
> > > > > > Will anyone ever actually have a List as the key itself? The
> > current
> > > > and
> > > > > > suggested changes would not allow it registering for a specific
> > List
> > > > > > object.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Thu, Nov 16, 2017 at 6:50 PM Jacob Barrett <
> jbarrett@pivotal.io
> > >
> > > > > wrote:
> > > > > >
> > > > > > > Geode Native C++ and .NET have:
> > > > > > >
> > > > > > >   virtual void registerKeys(const
> > > > > > > std::vector<std::shared_ptr<CacheableKey>>
& keys,
> > > > > > >                             bool isDurable = false,
> > > > > > >                             bool getInitialValues = false,
> > > > > > >                             bool receiveValues = true)
= 0;
> > > > > > >
> > > > > > >   virtual void unregisterKeys(const
> > > > > > > std::vector<std::shared_ptr<CacheableKey>>
& keys) = 0;
> > > > > > >
> > > > > > >   virtual void *registerAllKeys*(bool isDurable = false,
> > > > > > >                                bool getInitialValues =
false,
> > > > > > >                                bool receiveValues = true)
= 0;
> > > > > > >
> > > > > > >   virtual void unregisterAllKeys() = 0;
> > > > > > >
> > > > > > >   virtual void registerRegex(const std::string& regex,
> > > > > > >                              bool isDurable = false,
> > > > > > >                              bool getInitialValues = false,
> > > > > > >                              bool receiveValues = true)
= 0;
> > > > > > >
> > > > > > >   virtual void unregisterRegex(const char* regex) = 0;
> > > > > > >
> > > > > > > I dislike special values like this so yes please make it
go
> away!
> > > > > > >
> > > > > > > -Jake
> > > > > > >
> > > > > > >
> > > > > > > On Thu, Nov 16, 2017 at 5:20 PM Dan Smith <dsmith@pivotal.io>
> > > wrote:
> > > > > > >
> > > > > > > > I don't really like the regex option - it implies
that your
> > keys
> > > > are
> > > > > > all
> > > > > > > > strings. Will any other regular expressions work on
non
> string
> > > > > objects?
> > > > > > > > registerInterestAllKeys() seems like a better option.
> > > > > > > >
> > > > > > > > -Dan
> > > > > > > >
> > > > > > > > On Thu, Nov 16, 2017 at 4:34 PM, Michael Stolz <
> > > mstolz@pivotal.io>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > I don't like the vararg option.
> > > > > > > > > If i'm maintaining a list of keys i'm interested
in, I want
> > to
> > > be
> > > > > > able
> > > > > > > to
> > > > > > > > > pass that List in.
> > > > > > > > > Varargs is a poor substitute. It might even cause
problems
> of
> > > > > pushing
> > > > > > > in
> > > > > > > > > multiple different types. Keys must all be of
one type for
> a
> > > > given
> > > > > > > > Region.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > I'm very much in favor of deprecating the ALL_KEYS
string
> in
> > > > favor
> > > > > of
> > > > > > > > > something that is typed specially if you refer
to ALL_KEYS.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > If that works, then we don't necessarily need
the
> additional
> > > API
> > > > > > > > > registerInterestAllKeys(). But if ALL_KEYS can't
be a
> special
> > > > type
> > > > > to
> > > > > > > get
> > > > > > > > > over the compilation issues then we should go
with the new
> > API.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Mike Stolz
> > > > > > > > > Principal Engineer, GemFire Product Lead
> > > > > > > > > Mobile: +1-631-835-4771 <(631)%20835-4771>
> > <(631)%20835-4771> <(631)%20835-4771>
> > > > <(631)%20835-4771>
> > > > > <(631)%20835-4771>
> > > > > > > > >
> > > > > > > > > On Thu, Nov 16, 2017 at 7:02 PM, Anilkumar Gingade
<
> > > > > > > agingade@pivotal.io>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > +1 Deprecating ALL_KEYS option; I believe
this is added
> > > before
> > > > we
> > > > > > > > > supported
> > > > > > > > > > regex support.
> > > > > > > > > >
> > > > > > > > > >  Doesn't seems like a new API is needed.
The regex java
> doc
> > > > > clearly
> > > > > > > > > > specifies the effect of ".*".
> > > > > > > > > >
> > > > > > > > > > +1 for deprecating list argument; and replacing
with new
> > API.
> > > > > > > > > >
> > > > > > > > > > -Anil.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Thu, Nov 16, 2017 at 3:36 PM, Jason Huynh
<
> > > > jhuynh@pivotal.io>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > For GEODE-3813 <
> > > > > https://issues.apache.org/jira/browse/GEODE-3813
> > > > > > >:
> > > > > > > > > > Region
> > > > > > > > > > > registerInterest API usage of type
parameters is broken
> > > > > > > > > > > <https://issues.apache.org/jira/browse/GEODE-3813>
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > The current API to registerInterest
allows a special
> > string
> > > > > token
> > > > > > > > > > > “ALL_KEYS” to be passed in as the
parameter to
> > > > > registerInterest(T
> > > > > > > > key).
> > > > > > > > > > > This special token causes the registerInterest
to
> behave
> > > > > similar
> > > > > > to
> > > > > > > > > > > registerInterestRegex(“.*”).  As
the ticket states, if
> > the
> > > > > region
> > > > > > > has
> > > > > > > > > > been
> > > > > > > > > > > typed to anything other than Object
or String, the
> usage
> > of
> > > > > > > > “ALL_KEYS”
> > > > > > > > > > as a
> > > > > > > > > > > parameter results in a compilation
error.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Proposals:
> > > > > > > > > > >
> > > > > > > > > > > I would like to deprecate the special
string “ALL_KEYS”
> > and
> > > > > > > document
> > > > > > > > a
> > > > > > > > > > > workaround of using registerInterestRegex(“.*”)
or we
> can
> > > > add a
> > > > > > new
> > > > > > > > API
> > > > > > > > > > > called registerInterestAllKeys()
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > I think we should also deprecate passing
a List Object
> of
> > > > keys
> > > > > > into
> > > > > > > > > > > registerInterest.  It has the same
compilation
> > restrictions
> > > > as
> > > > > > > > > “ALL_KEYS”
> > > > > > > > > > > when the region is key constrained/typed.
 The reason
> why
> > > > List
> > > > > > > would
> > > > > > > > be
> > > > > > > > > > > used is to allow registering multiple
keys at once.
> > > Instead,
> > > > > we
> > > > > > > can
> > > > > > > > > add
> > > > > > > > > > a
> > > > > > > > > > > new var arg API like registerInterest(T…
keys).  This
> > > problem
> > > > > and
> > > > > > > > > > solution
> > > > > > > > > > > was also documented in the ticket by
the ticket creator
> > > (Kirk
> > > > > > Lund)
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > >
> > > > > > > > > > > -Jason
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -John
> > > > > john.blum10101 (skype)
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > -John
> > john.blum10101 (skype)
> >
>

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