+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
> > > > > >
> > > > > >
> > > >
> >
