Alex, the second option is exactly what I've proposed above.
On Tue, Jul 12, 2022 at 9:46 AM Alex Plehanov <plehanov.a...@gmail.com>
wrote:
> 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
> > > > > >>>>>>>>>>>>>>>>>
> > > > > >>>>>>>>>>>>>>>>>
> > > > > >>>>>>>>>>>>>>>
> > > > > >>>>>>>>>>>>>
> > > > > >>>>>>>>>
> > > > > >>>>>>>
> > > > > >>>>>>
> > > > > >>>>
> > > > > >>>>
> > > > > >>>
> > > > > >>
> > > > >
> > > > >
> > >
> >
>
