No objections to the AffinityFunctionFactory approach from my side.
I think it should be based on cacheName, not groupId.
On Mon, Jul 11, 2022 at 3:52 PM Maxim Muzafarov <mmu...@apache.org> wrote:
> Folks,
>
> I've done research about the proposed solution and I'd like to discuss
> with you a possible options we may have. After we agreed on a solution
> design I'll create a new IEP with everything we've discussed above.
>
> So, let's consider the most completed case:
> the RendezvousAffinityFunction with a custom affinity mapper function is
> used.
>
>
> =====================
>
> The solution with sending AffintyFunction typeId.
>
> a. The deprecated AffinityKeyMapper used prior to the AffinityFunction
> for calculation a key to partition mapping.
> See the link [1] - affFunction.partition(affinityKey(key))
>
> b. We are able to map typeId of the RendezvousAffinityFunction to
> another AffinityFunction on the client side to solve the mapping
> problem with any combination of AffinityFunction and
> AffinityKeyMappers ware used.
>
> c. When the cache partitions mapping request [2] is executed we are
> able to send the typeId of the RendezvousAffinityFunction back to the
> client, however without sending the typeId's of custom affinity
> mappers we are not able to identify the case when a substitution of
> the AffinityFunction is required on the client side.
>
> For instance,
>
> cacheGroup_1 has RendezvousAffinityFunction + FirstAffinityKeyMapper
> cacheGroup_2 has RendezvousAffinityFunction + SecondAffinityKeyMapper
>
> Adding a deprecated classes and their corresponding typeId's to the
> exchange client-server protocol sound not a good idea. However, this
> will cover all the cases.
>
> =====================
>
> The solution with factory method for AffintyFunction on the client side.
>
> We can also improve the solution that was proposed by me a few
> messages ago. Instead of the withPartitionAwarenessKeyMapper() [4]
> method which looks a bit weird we can add a factory method to the
> IgniteClientConfiguration that will provide a custom AffinityFunction
> for caches wift `non-default` mappings.
>
> The factory may looks like:
>
> class AffinityFunctionFactory implements BiFunction<Integer, Integer,
> AffinityFunction> {
> @Override public AffinityFunction apply(Integer groupId, Integer
> parts) {
> return new RendezvousAffinityFunction(false, parts);
> }
> }
>
> This cases will have a bit straightforward implementation and explicit
> AffinityFunction initialization by a user.
>
>
> WDYT?
>
>
>
> [1]
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAffinityManager.java#L185
> [2]
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/ClientMessageParser.java#L535
> [3]
> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/cache/affinity/AffinityFunction.java#L53
> [4]
> https://github.com/apache/ignite/pull/10140/files#diff-fb60155466fa2c31d6d1270c0e08f15f1ad9c1a2272fc9bb137c07d9ae5c1b98R47
>
> On Mon, 11 Jul 2022 at 13:14, Ivan Daschinsky <ivanda...@gmail.com> wrote:
> >
> > Still don't understand how type id can help me to obtain valid mapper
> from
> > key to int32. Especially when we are talking about non-jvm languages.
> >
> > пн, 11 июл. 2022 г., 12:33 Николай Ижиков <nizhi...@apache.org>:
> >
> > > Pavel.
> > >
> > > > It is not cryptic, it is very straightforward and builds on existing
> > > binary type mechanism.
> > >
> > > I see the Ivan’s point here.
> > > As I said earlier - It must be absolutely clear for user - PA works or
> not.
> > >
> > > With the logic you propose it will be hiding inside Ignite machinery
> and
> > > logs.
> > >
> > > > Unfortunately, this is a breaking change. Currently this scenario
> works
> > > without errors - uses default socket for custom affinity caches.
> > >
> > > It’s OK from my point of view to introduce such a change.
> > >
> > >
> > > > 10 июля 2022 г., в 22:05, Pavel Tupitsyn <ptupit...@apache.org>
> > > написал(а):
> > > >
> > > >> providing simple function Object -> int32 in cache configuration is
> > > wrong
> > > > decision
> > > >
> > > > Because it is error-prone and does not work for all cases:
> > > > - You have to set it explicitly on the client for every cache.
> > > > - No way to do that if you get an existing cache by name.
> > > >
> > > >> still being unable to solve the problem
> > > >
> > > > I believe the proposed solution covers all use cases.
> > > >
> > > >> cryptic logic
> > > >
> > > > It is not cryptic, it is very straightforward and builds on existing
> > > binary
> > > > type mechanism.
> > > >
> > > >> feature flags
> > > >
> > > > What's bad about feature flags?
> > > >
> > > >> how an ability to obtain a classname of java class can help my
> python or
> > > > C++ client map key to partition
> > > >
> > > > Not a java class name, but type id. You can set up type id mapping
> > > properly
> > > > and use this in any thin client.
> > > > This will work for .NET client, not sure about Python and C++.
> > > >
> > > > On Sun, Jul 10, 2022 at 9:33 PM Ivan Daschinsky <ivanda...@gmail.com
> >
> > > wrote:
> > > >
> > > >> Guys, I simply cant understand why providing simple function Object
> ->
> > > >> int32 in cache configuration is wrong decision, but implementing
> cryptic
> > > >> logic with class names, feature flags and still being unable to
> solve
> > > the
> > > >> probem is ok. I don't understand how an ability to obtain a
> classname of
> > > >> java class can help my python or C++ client map key to partition.
> > > >>
> > > >> Max's proposal seems to be a good variant, just change naming a
> little
> > > bit,
> > > >> maybe
> > > >>
> > > >> пт, 8 июл. 2022 г., 15:35 Pavel Tupitsyn <ptupit...@apache.org>:
> > > >>
> > > >>> Nikolay,
> > > >>>
> > > >>>> Add ability to sent custom affinity class name.
> > > >>> Can you please elaborate? What is sent where?
> > > >>>
> > > >>>> If `partitionAwarenessEnabled=true` but custom affinity can’t be
> > > >>> instantiated on the client - throw an exception
> > > >>> Unfortunately, this is a breaking change. Currently this scenario
> works
> > > >>> without errors - uses default socket for custom affinity caches.
> > > >>>
> > > >>>
> > > >>> On Fri, Jul 8, 2022 at 3:23 PM Николай Ижиков <nizhi...@apache.org
> >
> > > >> wrote:
> > > >>>
> > > >>>> Hello, Pavel.
> > > >>>>
> > > >>>> I support your proposal to transparently create custom
> AffinityMapper
> > > >>>> based on server node classname, in general.
> > > >>>> Partition Awareness crucial point for a thin client performance
> so It
> > > >>>> should be absolutely clear for a user - PA works correctly or not.
> > > >>>>
> > > >>>> So, I think, we should implement the following:
> > > >>>>
> > > >>>> 0. Add ability to sent custom affinity class name.
> > > >>>> 1. If `partitionAwarenessEnabled=true` but custom affinity can’t
> be
> > > >>>> instantiated on the client - throw an exception.
> > > >>>>
> > > >>>> WDYT?
> > > >>>>
> > > >>>>> 8 июля 2022 г., в 08:37, Pavel Tupitsyn <ptupit...@apache.org>
> > > >>>> написал(а):
> > > >>>>>
> > > >>>>> To clarify: you can use a different AffinityFunction
> implementation
> > > >> on
> > > >>>> the
> > > >>>>> client side. It just has to map to the same binary typeId.
> > > >>>>> Even if there is a legacy setup with AffinityKeyMapper on
> servers,
> > > >> you
> > > >>>> can
> > > >>>>> craft an AffinityFunction for the client that achieves correct
> > > >>> behavior.
> > > >>>>> So I believe this should cover any use case.
> > > >>>>>
> > > >>>>> On Thu, Jul 7, 2022 at 10:36 PM Pavel Tupitsyn <
> ptupit...@apache.org
> > > >>>
> > > >>>> wrote:
> > > >>>>>
> > > >>>>>> AffinityKeyMapper is just a subset of AffinityFunction, isn't
> it?
> > > >>>>>> So by supporting AffinityFunction we cover all
> AffinityKeyMapper use
> > > >>>> cases.
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>> managing the classpath, a user should always keep in mind it
> > > >>>>>> I don't consider this a problem. This is how Java works, what
> can we
> > > >>> do?
> > > >>>>>> You can also stick the class into BinaryConfiguration on the
> client
> > > >> to
> > > >>>>>> make it explicit and make sure it is loaded.
> > > >>>>>>
> > > >>>>>>> it still possible having user bad implementations of
> > > >> AffinityFunction
> > > >>>>>> that can't be easily instantiated;
> > > >>>>>> It is also possible to provide a bad implementation of
> > > >> ToIntFunction,
> > > >>> no
> > > >>>>>> difference.
> > > >>>>>>
> > > >>>>>>> a question - what we should do in case of class instantiating
> > > >> loading
> > > >>>>>> fails?
> > > >>>>>> Currently we don't fail on custom affinity, right? So we should
> keep
> > > >>>> this
> > > >>>>>> behavior. Print a warning.
> > > >>>>>>
> > > >>>>>> On Thu, Jul 7, 2022 at 10:27 PM Maxim Muzafarov <
> mmu...@apache.org>
> > > >>>> wrote:
> > > >>>>>>
> > > >>>>>>> Pavel,
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>> Yes, you're right that AffinityKeyMapper is deprecated, this
> is sad
> > > >>> to
> > > >>>>>>> say but it still used.
> > > >>>>>>>
> > > >>>>>>> Let me describe the cases in more detail. In general, we have
> > > >>>>>>> customers which are using:
> > > >>>>>>>
> > > >>>>>>> 1. an own custom AffinityFunction;
> > > >>>>>>> 2. the default RendezvousAffinityFunction + an own deprecated
> > > >>>>>>> AffinityKeyMapper;
> > > >>>>>>> 3. an own custom AffinityFunction + an own deprecated
> > > >>>>>>> AffinityKeyMapper (I doubt about this case);
> > > >>>>>>>
> > > >>>>>>> I'd like to provide a general solution that solves all the
> cases
> > > >>>>>>> mentioned above, thus we can't discuss only AffinityFunction.
> It
> > > >>>>>>> doesn't fit the initial goals.
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>>> Can you please clarify the "tricky" part?
> > > >>>>>>>
> > > >>>>>>> Yes, from my point of view the tricky part here is:
> > > >>>>>>>
> > > >>>>>>> - managing the classpath, a user should always keep in mind it;
> > > >>>>>>> - it still possible having user bad implementations of
> > > >>>>>>> AffinityFunction that can't be easily instantiated;
> > > >>>>>>> - a question - what we should do in case of class instantiating
> > > >>>>>>> loading fails? drop the client, print the warn or do nothing.
> These
> > > >>>>>>> solutions have a different impact on a production environment
> and
> > > >>> user
> > > >>>>>>> may have different expectations on that;
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>> I'm not saying that solution you suggested is bad, it just has
> some
> > > >>>>>>> drawbacks (like other solutions) and doesn't solves all the
> cases
> > > >> if
> > > >>>>>>> we're not instantiating a deprecated AffinityKeyMapper (which
> > > >> looks a
> > > >>>>>>> bit ugly for me too).
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>> Actually, we are not limited with setting/instantiating
> > > >>>>>>> AffinityFunction. We can provide for a user a control on which
> > > >>> channel
> > > >>>>>>> (node) a particular cache request will be executed (this thing
> also
> > > >>>>>>> have a drawback - a transactional operation cannot be executed
> on
> > > >> an
> > > >>>>>>> arbitrary node, it should be executed on node it's started).
> > > >>>>>>>
> > > >>>>>>> On Thu, 7 Jul 2022 at 21:41, Pavel Tupitsyn <
> ptupit...@apache.org>
> > > >>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> AffinityKeyMapper (it will be required too)
> > > >>>>>>>> It is deprecated. We should not add any functionality on top
> of
> > > >>>>>>> deprecated
> > > >>>>>>>> stuff.
> > > >>>>>>>> Let's discuss only AffinityFunction.
> > > >>>>>>>>
> > > >>>>>>>>> this approach requires of these classes to be present on a
> thin
> > > >>>> client
> > > >>>>>>>> side and it can be tricky
> > > >>>>>>>> Any of the approaches discussed above requires some additional
> > > >> logic
> > > >>>> to
> > > >>>>>>> be
> > > >>>>>>>> present on the client.
> > > >>>>>>>> Can you please clarify the "tricky" part?
> > > >>>>>>>>
> > > >>>>>>>> All you need is to provide AffinityFunction implementation
> with a
> > > >>>> proper
> > > >>>>>>>> class name
> > > >>>>>>>> (or even use nameMapper/idMapper to map to a different name),
> it
> > > >> is
> > > >>> no
> > > >>>>>>>> different from the ToIntFunction approach.
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>> On Thu, Jul 7, 2022 at 9:23 PM Maxim Muzafarov <
> mmu...@apache.org
> > > >>>
> > > >>>>>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> 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
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>
> > > >>>>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
>
