Folks,
I'd like also to mention since we are talking about a customer's
custom affinity function and affinity mapper implementations we could
face with some issues during these classes instantiation. It could
work for the customer which I've faced on, but I cannoun be sure that
this approach will work for all of the cases.
I think it is better to avoid such an issues and leave it up to a
user. Nevertheless, the worst scenario for the user would be having 2
hops on each put/get as it was before this thread.
On Thu, 7 Jul 2022 at 21:12, Maxim Muzafarov <mmu...@apache.org> wrote:
>
> Folks,
>
> I'm not familiar with a thin client protocol, so, please, correct me
> if I'm wrong - is it possible sending classes over the network for
> different type of thin clients? It seems to me it will not work. I
> think it is possible to use class name/or type id and we are capable
> to instantiate both of the AffinityFunction and AffinityKeyMapper (it
> will be required too) classes. However, this approach requires of
> these classes to be present on a thin client side and it can be tricky
> to configure if this client is used inside a container (e.g. with
> spring data or something).
>
> Setting directly some kind of a known mapper looks more
> straightforward and error prone approach.
>
> Please, correct me if I am wrong.
>
> On Thu, 7 Jul 2022 at 21:02, Семён Данилов <samvi...@yandex.ru> wrote:
> >
> > +1 to Pavel’s approach. Giving user the ability to set affinity function
> > that is potentially different from the one that is on server doesn’t look
> > good.
> >
> > I’d even go further and try sending the affinity function class itself over
> > the network, so users won’t have to alter the classpath by adding the class
> > that is not directly used in the codebase.
> > On 7 Jul 2022, 21:41 +0400, Pavel Tupitsyn <ptupit...@apache.org>, wrote:
> > > Can we actually avoid any public API changes and do the following:
> > >
> > > When a feature flag is present and there is a custom affinity function on
> > > the server, send back the class name (or binary type id, if possible) of
> > > the affinity function?
> > > Then simply instantiate it on the client and call `partition(Object key)`
> > > on it?
> > >
> > > On Thu, Jul 7, 2022 at 8:32 PM Maxim Muzafarov <mmu...@apache.org> wrote:
> > >
> > > > Pavel,
> > > >
> > > > Here is my thoughts.
> > > >
> > > > As I mentioned before, we will need only the
> > > > AffinityFunction#partition(Object key) method for calculation on the
> > > > client side due to all the partition-to-node mappings will be
> > > > requested asynchronously from a server node how it is happening now
> > > > for cache groups with the default affinity function. Thus setting the
> > > > AffinityFunction looks on the client side redundant and it can be
> > > > transformed to a simple ToInFunction<Object> interface.
> > > >
> > > > public interface ToIntFunction<Object> {
> > > > int applyAsInt(Object key);
> > > > }
> > > >
> > > > Another point that I'd like also to mention is that the cache already
> > > > created in a cluster, so it will be better to eliminate the duplicate
> > > > affinity function configuration (setting the number of partitions). It
> > > > is fully possible to receive the number of partitions from a server
> > > > node (the same way as it currently happening). Thus instead of the
> > > > ToInFunction<Object> interface it will be better to use
> > > > ToInBiFunction<Object, Integer>.
> > > >
> > > > public interface ToIntBiFunction<Object, Integer> {
> > > > int applyAsInt(Object key, Integer parts);
> > > > }
> > > >
> > > >
> > > > I agree with you that "setPartitionAwarenessKeyMapper" may be is not
> > > > good naming here, we can rename it and move it to the public API, of
> > > > course.
> > > >
> > > > On Thu, 7 Jul 2022 at 20:06, Pavel Tupitsyn <ptupit...@apache.org>
> > > > wrote:
> > > > >
> > > > > Maxim,
> > > > >
> > > > > I am confused. We were talking about a custom Affinity Function.
> > > > > As you noted, AffinityKeyMapper is deprecated, why do we add something
> > > > > named "setPartitionAwarenessKeyMapper"?
> > > > >
> > > > > Internal API approach is hacky.
> > > > > IMO we should either develop a proper feature with a good public API,
> > > > > or
> > > > > not add anything at all.
> > > > >
> > > > > On Thu, Jul 7, 2022 at 6:34 PM Maxim Muzafarov <mmu...@apache.org>
> > > > wrote:
> > > > >
> > > > > > Folks,
> > > > > >
> > > > > >
> > > > > > Thank you for your comments.
> > > > > >
> > > > > > First of all, I'd like to point out that if the cache is created
> > > > > > with
> > > > > > a custom affinity function or the legacy AffinityKeyMapper
> > > > > > interface,
> > > > > > the thin client should be able to provide only a `key-to-partition`
> > > > > > mapping function to handle the case described above. The
> > > > > > `partition-to-node` mappings the client will receive from a server
> > > > > > node it connected to. This will simplify a bit the final solution.
> > > > > >
> > > > > > ==================
> > > > > >
> > > > > > I've checked your suggestions and it looks like both of them have
> > > > > > some
> > > > > > sufficient drawbacks:
> > > > > >
> > > > > > 1. Using SystemProperty looks really hacky and we are explicitly
> > > > > > complicate a thin client configuration for an end user.
> > > > > >
> > > > > > 2. Extending the ClientCacheConfiguration is a very good and
> > > > > > straightforward idea, however it doesn't fit the case described
> > > > > > above.
> > > > > > Caches previously created with custom affinity functions/key mappers
> > > > > > already present in the cluster, so in this case we are forcing a
> > > > > > user
> > > > > > to store an additional ClientCacheConfiguration. This is not good.
> > > > > > The
> > > > > > cache.getOrCreate(cfg) method will also be used and fire in turn
> > > > > > CACHE_GET_OR_CREATE_WITH_CONFIGURATION request which is not
> > > > > > necessary
> > > > > > here. For this case using cache.name(str) is only enough.
> > > > > >
> > > > > > ==================
> > > > > >
> > > > > > I propose the following two solutions that looks very promising:
> > > > > >
> > > > > > 3. Extending cache create methdos with a ClientCacheContext in the
> > > > > > IgniteClient interface. This context will contain all additional
> > > > > > cache
> > > > > > attributes like custom cache affinity mappers that map cache keys to
> > > > > > partitions if a custom affinity was used on the server side (note
> > > > > > that
> > > > > > all partition-to-node mappings will be received by thin client from
> > > > > > a
> > > > > > server node).
> > > > > >
> > > > > > interface IgniteClientEx extends IgniteClient {
> > > > > > ClientCache<K, V> name(String name, ClientCacheContext cctx);
> > > > > > ClientCache<K, V> getOrCreateCache(String name, ClientCacheContext
> > > > > > cctx);
> > > > > > ClientCache<K, V> getOrCreateCache(ClientCacheConfiguration cfg,
> > > > > > ClientCacheContext cctx);
> > > > > > }
> > > > > >
> > > > > > class ClientCacheContext {
> > > > > > setPartitionAwarenessKeyMapper(ToIntBiFunction<Object, Integer>
> > > > > > mapper);
> > > > > > }
> > > > > >
> > > > > > 4. Use the same approach as the IgniteCache interface does for the
> > > > > > same things - adding withPartitionAwarenessKeyMapper() to the
> > > > > > interface. This method will allow to configure the thin client
> > > > > > execution behaviour for the partition awareness feature by setting a
> > > > > > custom cache key mapper.
> > > > > >
> > > > > > ==================
> > > > > >
> > > > > > I've used the 4-th solution due to it brings much less source code
> > > > > > to
> > > > > > the Apache Ignite codebase and looks a bit simpler to configure for
> > > > > > a
> > > > > > user. I've also move the withPartitionAwarenessKeyMapper() method to
> > > > > > an internal API interface which still solves a user issue with the
> > > > > > partition awareness, but also specifies that the custom mapping
> > > > > > function and deprecated AffinityKeyMapper should not be used, in
> > > > > > general.
> > > > > >
> > > > > > Please, take a look at my patch:
> > > > > >
> > > > > > https://issues.apache.org/jira/browse/IGNITE-17316
> > > > > > https://github.com/apache/ignite/pull/10140/files
> > > > > >
> > > > > >
> > > > > > On Fri, 1 Jul 2022 at 14:41, Pavel Tupitsyn <ptupit...@apache.org>
> > > > wrote:
> > > > > > >
> > > > > > > I have no objections to extending the Thin Client configuration
> > > > > > > with
> > > > a
> > > > > > > pluggable Affinity Function.
> > > > > > > Let's make it a normal Java setter though, system properties are
> > > > hacky.
> > > > > > > Especially when only some of the caches use custom affinity, as
> > > > > > > Maxim
> > > > > > > mentioned.
> > > > > > >
> > > > > > > On Wed, Jun 29, 2022 at 7:20 PM Николай Ижиков
> > > > > > > <nizhi...@apache.org>
> > > > > > wrote:
> > > > > > >
> > > > > > > > +1 to have ability to specify custom affinity for PA on thin
> > > > client.
> > > > > > > >
> > > > > > > > It seems to me custom affinity is a rare use-case of Ignite API.
> > > > > > > > Propose to have SystemProperty that can specify affinity
> > > > implementation
> > > > > > > > for a thin client.
> > > > > > > >
> > > > > > > >
> > > > > > > > > 29 июня 2022 г., в 18:53, Maxim Muzafarov <mmu...@apache.org>
> > > > > > > > написал(а):
> > > > > > > > >
> > > > > > > > > Igniters,
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > I've faced with a customer's cluster which has more than 150
> > > > nodes
> > > > > > and
> > > > > > > > > most for them are the thick-client nodes. Due to each
> > > > thick-client is
> > > > > > > > > a full-fledged cluster topology participant it affects the
> > > > cluster
> > > > > > > > > discovery machinery during the system operation and adding an
> > > > > > > > > additional overhead for using/deploying a new nodes in
> > > > > > > > > Kubernetes
> > > > > > > > > environment. However, the main thing from my point of view it
> > > > > > prevents
> > > > > > > > > updating the client side and server side components
> > > > > > > > > independently
> > > > > > > > > (Apache Ignite doesn't support rolling upgrade).
> > > > > > > > >
> > > > > > > > > Accordingly to the assumptions above using thin clients
> > > > > > > > > become a
> > > > > > > > > necessary. This looks even more attractive, since the thin
> > > > client has
> > > > > > > > > a fairly rich API over the past few releases.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > The MAIN ISSUE here that blocks thin client usage is that for
> > > > some of
> > > > > > > > > cache groups a custom affinity function (and an
> > > > AffinityKeyMapper)
> > > > > > was
> > > > > > > > > used which prevents enabling the Partition Awareness thin
> > > > > > > > > client
> > > > > > > > > feature. Thus each cache request will have two hops.
> > > > > > > > >
> > > > > > > > > Of course, we can force users to migrate to a new API, but
> > > > > > > > > this
> > > > > > > > > becomes more difficult when Apache Ignite is part of a much
> > > > larger
> > > > > > > > > architectural solution and thus it is doent' looks so
> > > > > > > > > friendly.
> > > > > > > > >
> > > > > > > > > The MAIN QUESTION here - does anyone know our users who have
> > > > > > > > > encountered with the same issue? I want to solve such a
> > > > > > > > > problem
> > > > once
> > > > > > > > > and make all such users happy by implementing the general
> > > > approach.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > = Possible solutions =
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 1. Making an affinity function pluggable (mapping
> > > > > > > > > calculations)
> > > > on
> > > > > > the
> > > > > > > > > thin clients side. Currently the RendezvousAffinityFunction
> > > > > > > > > [1]
> > > > is
> > > > > > > > > only supported
> > > > > > > > > for the partition awareness. A user's affinity function seems
> > > > > > > > > to
> > > > be
> > > > > > > > > the stateless function due to there is no machinery to
> > > > > > > > > transfer
> > > > > > states
> > > > > > > > > to the thin client.
> > > > > > > > >
> > > > > > > > > Pros - a general solution for all such cases;
> > > > > > > > > Cons - unnecessary complexity, extending public API;
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 2. Creating an Ignite extension which will extend the thin
> > > > client API
> > > > > > > > > thus a user will have a full control over a destination node
> > > > > > > > > to
> > > > which
> > > > > > > > > requests being sent.
> > > > > > > > >
> > > > > > > > > Pros - isolated solution, simple implementation;
> > > > > > > > > Cons - hard to support spring-boot-thin-client etc. and other
> > > > > > > > > extensions based on the thin client API;
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Folks, please share your thoughts.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > [1]
> > > > > > > >
> > > > > >
> > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/cache/ClientCachePartitionsRequest.java#L206
> > > > > > > >
> > > > > > > >
> > > > > >
> > > >
