Agreed, sounds like a plan. We should probably output a warning about deprecated interface usage and it's consequences.
2017-06-08 15:49 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > 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. > > > > > >
