Folks, why should we construct a fully functional affinity function on the
client side by custom client code? We need only a key to partition mapping,
so only this simple mapper factory will be enough.
>From my point of view there are to options possible:
- provide ability to set partition to key mapper factory by the client code
or
- serialize custom affinity functions and custom affinity key mappers to
binary objects and send it together with ClientCachePartitionResponse
message. In this case, explicit client configuration or some kind of client
code is not required, affinity function (with type id and its internals)
will be deserialized if classes are present on thin-client side and will be
used implicitly (for java thin client). For other thin clients (except
java) type id can be retrieved from the binary object and some key to
partition mapper can be provided based on this type id (for example by
configured factory).
пн, 11 июл. 2022 г. в 16:22, Pavel Tupitsyn <ptupit...@apache.org>:
> 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
> > > > >>>>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>>>
> > > > >>>>>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>
> > > > >>>>
> > > > >>>>
> > > > >>>
> > > > >>
> > > >
> > > >
> >
>
