Alex, Sergi suggested to deprecate this interface and it is already deprecated. However, internally it is still used and in particular we depend on it in IGFS inetrnals. It will require some time to refactor IGFS. Let's just throw an exception if both AffinityKeyMapper and affinity fields mappings are defined?
On Thu, Jun 8, 2017 at 3:45 PM, Alexey Goncharuk <alexey.goncha...@gmail.com > wrote: > Vladimir, > > Your concerns sound reasonable to me. However, I have a feeling that there > was a specific reason why added CacheKeyConfiguration to the > IgniteConfiguration and not to the CacheConfiguration. Looks like indeed, > our original approach is not flexible enough anymore and I am +1 for the > suggested change. > > BTW, do we really want to keep the AffinityMapper concept? For me, it looks > like the interface makes it impossible for SQL to route requests to correct > nodes because we cannot tell in advance which fields were selected as an > affinity key. > > --AG > > 2017-06-08 15:16 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > > > Folks, > > > > (Yakov, this is partially about your problem with DML) > > > > I found one nasty problem with our "affinity key field" concept. > Currently > > it works as follows: > > 1) Affinity keys are stored in binary metadata > > 2) Metadata is populated when object get's serialized for the first time. > > 3) There is an exception to this rule - > > IgntieConfiguration.cacheKeyConfiguration - which effectively overrides > > affinity key globally for certain types. > > 4) And finally we have AffinityKeyMapper interface which could ignore > > everything and simply define it's own logic. > > > > All in all this simply doesn't work. For example, if I serialize the type > > thus triggering metadata update, and then create a cache, affinity key > will > > be resolved properly in both cache and indexing internals. But if I > create > > a cache *before* type is registered, then all internal structures will > > think that affinity key is null, which might be wrong desicion. > > > > IgntieConfiguration.cacheKeyConfiguration could potentially solve this > > problem, but it must be defined *before node start*, and this it defeats > > the whole idea of dynamic cache start. Something is really wrong here. > > > > I thought a bit on this, and here is my proposal: > > 1) There should be no global affinity key for type. Current approach to > add > > affinity key to metadata is simply a huge design flaw. We will remove it. > > 2) There should be no static affinity key mapping either - > > IgntieConfiguration.cacheKeyConfiguration must be deprecated. > > 3) Instead, affinity key must be defined for each [cache, type] pair. > > 4) These mappings should be defined before cache start and cannot change > - > > we will add CacheConfiguration.cacheKeyConfiguration property. > > 5) Mappings are shared through discovery during cache start. In case of > > conflict we throw an exception. > > 6) Finally, we should define consistent affinity key resolution if both > > AffinityKeyMapper interface and affinity key mapping are defined. E.g., > > throw an exception if both are defined. > > > > What we got: > > 1) Predictable behavior - we can always deduce proper affinity key from > > cache configuration. No race conditions between cache start and dynamic > > type registration. > > 2) No static config which cannot be changed in runtime. > > 3) Affinity key mappings will be part of CacheConfiguration, and thus > will > > survive restarts when persistence is enabled. > > > > Thoughts? > > > > Vladimir. > > >
