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