Dmitriy,
>>> Every time an object is used as a key in a cache, we automatically
generate hashcode for it. The first time we do it, we print out a warning
in the log, that the hashcodes will be automatically generated, if not
provided.
We will receive billion questions like "why did I put object to cache, but
cannot retrieve it?" when users created an object using builder (explicitly
or using SQL INSERT), and we auto-generated wrong hash code. "Wrong" means
the one which doesn't match a hash code of a relevant Java class.
I think we can do the following:
1) Add "has hash code" flag as Denis suggested.
2) If object without a hash code is put to cache, throw an exception.
3) Add *BinaryEqualsHashCodeResolver *interface.
4) Add default implementation which will auto-generate hash code. *Print a
warning when auto-generation occurs*, so that user is aware that he is
likely to have problems with normal GETs/PUTs.
5) Add another implementation which will use encoded string to calculate a
hash code. E.g. *new BinaryEqualsHashCodeResolver("{a} * 31 + {b}")*.
Originally proposed by Yakov some time ago.
Thoughts?
Vladimir.
On Sun, Aug 7, 2016 at 10:26 PM, Dmitriy Setrakyan <dsetrak...@apache.org>
wrote:
> On Sat, Aug 6, 2016 at 8:11 PM, Sergi Vladykin <sergi.vlady...@gmail.com>
> wrote:
>
> > I think it makes sense to always generate hashCode but allow overriding
> it
> > if really needed. Because this requirement to set it manually is a priori
> > usability issue and error prone approach.
> >
>
> Agree. Sounds like the best approach. I would still prefer 1 message in the
> log per JVM stating that the system has generated automatic hashCode and
> there is a way to override it manually.
>
>
> >
> > TBH, I do not even understand why we allow overriding it at all, if key
> > hashCode is not defined by it's fields, then there are good chances that
> it
> > will work wrong (current implementations of offheap depends on serialized
> > key equality for example).
> >
>
> I think there will be some use cases where users will want to control the
> hash code themselves, perhaps for the types that we don't serialize
> automatically. I think we need to provide that capability.
>
>
> >
> > Sergi
> >
> >
> >
> > 2016-08-06 22:58 GMT+03:00 Alexander Paschenko <
> > alexander.a.pasche...@gmail.com>:
> >
> > > Warning is of little help if there's no way to retrieve object from the
> > > cache by given key later, isn't it?
> > >
> > > — Alex
> > > 6 авг. 2016 г. 8:04 PM пользователь "Dmitriy Setrakyan" <
> > > dsetrak...@apache.org> написал:
> > >
> > > > Sergi, you are right. We keep jumping back and forth on this issue.
> > > >
> > > > How about this suggestion. We don't create any new configuration
> > > > properties. Every time an object is used as a key in a cache, we
> > > > automatically generate hashcode for it. The first time we do it, we
> > print
> > > > out a warning in the log, that the hashcodes will be automatically
> > > > generated, if not provided.
> > > >
> > > > This is as clean as it will ever get.
> > > >
> > > > Thoughts?
> > > >
> > > > D.
> > > >
> > > >
> > > > On Sat, Aug 6, 2016 at 1:25 AM, Sergi Vladykin <
> > sergi.vlady...@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Keep in mind that we need to support affinity keys in
> BinaryObjects.
> > It
> > > > > means that it will have to consist from at least two fields: one
> for
> > > > exact
> > > > > equality check and another one for hashCode calculation.
> > > > >
> > > > > It looks to me that configuration of cache key is a part of cache
> > > > > configuration. Thus cache key builder must be bound to cache.
> > > > >
> > > > > Sergi
> > > > >
> > > > > 2016-08-06 6:18 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org
> >:
> > > > >
> > > > > > How about we add a property - auto-generate hashCode() in binary
> > > > > > configuration. If set, then we auto-generate the hashCode() every
> > > time
> > > > a
> > > > > > binary object is built.
> > > > > >
> > > > > > Thoughts?
> > > > > >
> > > > > > On Fri, Aug 5, 2016 at 5:29 AM, Alexander Paschenko <
> > > > > > alexander.a.pasche...@gmail.com> wrote:
> > > > > >
> > > > > > > Denis,
> > > > > > >
> > > > > > > Thank you very much for your proposed solution, I will reflect
> it
> > > in
> > > > > > > issue's comments and implement this check in code. Most likely
> > this
> > > > > > > has to be an issue by itself.
> > > > > > >
> > > > > > > However, it all does not answer the main question of this
> thread
> > -
> > > > how
> > > > > > > do we automatically supply hash codes for newly built binary
> > > objects?
> > > > > > > This is very important for convenient use of SQL inserts.
> Please,
> > > > all,
> > > > > > > share your thoughts.
> > > > > > >
> > > > > > > - Alex
> > > > > > >
> > > > > > > 2016-08-03 3:23 GMT+03:00 Dmitriy Setrakyan <
> > dsetrak...@apache.org
> > > >:
> > > > > > > > On Tue, Aug 2, 2016 at 7:36 AM, Denis Magda <
> > dma...@gridgain.com
> > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > >> Vova,
> > > > > > > >>
> > > > > > > >> By default hasCode field will be initialized this way in
> > > > > > > >> BinaryObjectBuilderImpl
> > > > > > > >>
> > > > > > > >> /** */
> > > > > > > >> private static Integer DFLT_HASH_CODE_MAGIC = 0;
> > > > > > > >>
> > > > > > > >> /** */
> > > > > > > >> private Integer hashCode = DFLT_HASH_CODE_MAGIC;
> > > > > > > >>
> > > > > > > >> Also we will introduce the following flag in BinaryUtils
> > > > > > > >>
> > > > > > > >> /** Flag indicating whether as hashCode was explicitly set
> or
> > > not.
> > > > > **/
> > > > > > > >> public static final short EMPTY_HASH_CODE_FLAG = 0x0032;
> > > > > > > >>
> > > > > > > >> At the BinaryObjectBuilder.build() time we will perform the
> > > check
> > > > > > below
> > > > > > > >> and write hashCodeFlag.
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> short hashCodeFlag = hashCode == DFLT_HASH_CODE_MAGIC ?
> > > > > > > >> BinaryUtils.EMPTY_HASH_CODE_FLAG : 0;
> > > > > > > >>
> > > > > > > >> // Write hashCode flag as well.
> > > > > > > >> writer.postWrite(true, registeredType, hashCode,
> > hashCodeFlag);
> > > > > > > >>
> > > > > > > >> Later when a BinaryObject is used as a key we can check the
> > > value
> > > > of
> > > > > > > >> BinaryUtils.EMPTY_HASH_CODE_FLAG
> > > > > > > >> and throw an exception if it’s not empty.
> > > > > > > >>
> > > > > > > >> Makes sense?
> > > > > > > >>
> > > > > > > >
> > > > > > > > It does to me. If there are no objections, then we should
> list
> > > this
> > > > > in
> > > > > > > the
> > > > > > > > ticket and implement the proposed suggestion of throwing
> > > exception
> > > > > if a
> > > > > > > > binary object without hashcode is used as a key.
> > > > > > > >
> > > > > > > >
> > > > > > > >>
> > > > > > > >> —
> > > > > > > >> Denis
> > > > > > > >>
> > > > > > > >> > On Aug 2, 2016, at 7:09 AM, Vladimir Ozerov <
> > > > voze...@gridgain.com
> > > > > >
> > > > > > > >> wrote:
> > > > > > > >> >
> > > > > > > >> > Denis,
> > > > > > > >> >
> > > > > > > >> > I hardly can imagine how it could work in our binary
> > protocol.
> > > > Can
> > > > > > you
> > > > > > > >> > please elaborate?
> > > > > > > >> >
> > > > > > > >> > On Tue, Aug 2, 2016 at 5:06 PM, Denis Magda <
> > > > dma...@gridgain.com>
> > > > > > > wrote:
> > > > > > > >> >
> > > > > > > >> >> There is a technique we already use to see if a field is
> > > > > > initialized
> > > > > > > by
> > > > > > > >> >> application code. By default a field has to be a
> reference
> > > to a
> > > > > > > >> predefined
> > > > > > > >> >> object and the reference comparison (not equals) is used
> to
> > > > check
> > > > > > if
> > > > > > > the
> > > > > > > >> >> field is initialized by the user.
> > > > > > > >> >>
> > > > > > > >> >> Refer to IgniteConfiguration.failureDetectionTimeout and
> > > > > > > >> >> IgniteSpiAdapter.initializeFailureDetectionTimeout for
> > more
> > > > > > details.
> > > > > > > >> >>
> > > > > > > >> >> —
> > > > > > > >> >> Denis
> > > > > > > >> >>
> > > > > > > >> >>> On Aug 2, 2016, at 12:14 AM, Alexander Paschenko <
> > > > > > > >> >> alexander.a.pasche...@gmail.com> wrote:
> > > > > > > >> >>>
> > > > > > > >> >>> Dmitriy,
> > > > > > > >> >>>
> > > > > > > >> >>> Good point, however, currently there's no way to
> > distinguish
> > > > > hash
> > > > > > > code
> > > > > > > >> >>> of zero which is a valid case from missing hash code. We
> > > > > probably
> > > > > > > >> >>> should enhance binary builder for it to handle this
> case.
> > > > > > > >> >>>
> > > > > > > >> >>> - Alex
> > > > > > > >> >>>
> > > > > > > >> >>> 2016-08-02 9:47 GMT+03:00 Dmitriy Setrakyan <
> > > > > > dsetrak...@apache.org
> > > > > > > >:
> > > > > > > >> >>>> On Mon, Aug 1, 2016 at 11:38 PM, Vladimir Ozerov <
> > > > > > > >> voze...@gridgain.com>
> > > > > > > >> >>>> wrote:
> > > > > > > >> >>>>
> > > > > > > >> >>>>> Andrey,
> > > > > > > >> >>>>>
> > > > > > > >> >>>>> The question is when to print this warning. I doubt we
> > can
> > > > > > print a
> > > > > > > >> >> warning
> > > > > > > >> >>>>> when calling *BinaryObjectBuilder.build() *method,
> > because
> > > > an
> > > > > > > object
> > > > > > > >> >>>>> without a hash code is normal situation.
> > > > > > > >> >>>>>
> > > > > > > >> >>>>>
> > > > > > > >> >>>> I would not only print warning, but throw exception, if
> > an
> > > > > object
> > > > > > > >> >> without a
> > > > > > > >> >>>> hashCode ends up on a put or read operation in cache.
> > > > > > > >> >>>>
> > > > > > > >> >>>>
> > > > > > > >> >>>>> On Tue, Aug 2, 2016 at 9:00 AM, Andrey Gura <
> > > > > ag...@gridgain.com
> > > > > > >
> > > > > > > >> >> wrote:
> > > > > > > >> >>>>>
> > > > > > > >> >>>>>> I think we also should print some warning in case
> when
> > > > > > hashCode()
> > > > > > > >> >> wasn't
> > > > > > > >> >>>>>> called on BinaryObject explicitly.
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>> On Tue, Aug 2, 2016 at 2:20 AM, Dmitriy Setrakyan <
> > > > > > > >> >> dsetrak...@apache.org
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>> wrote:
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>>> On Mon, Aug 1, 2016 at 10:01 AM, Alexey Goncharuk <
> > > > > > > >> >>>>>>> alexey.goncha...@gmail.com> wrote:
> > > > > > > >> >>>>>>>
> > > > > > > >> >>>>>>>> Dmitriy,
> > > > > > > >> >>>>>>>>
> > > > > > > >> >>>>>>>> The question is how do you calculate the value of
> the
> > > > > > > hashCode? Do
> > > > > > > >> >>>>> you
> > > > > > > >> >>>>>>> want
> > > > > > > >> >>>>>>>> it to be specified explicitly in INSERT statement?
> > > > > > > >> >>>>>>>>
> > > > > > > >> >>>>>>>
> > > > > > > >> >>>>>>> I think optionally we should allow to specify
> hashCode
> > > as
> > > > > part
> > > > > > > of
> > > > > > > >> the
> > > > > > > >> >>>>>>> INSERT statement. However, if it is not specified,
> we
> > > > should
> > > > > > > >> >> calculate
> > > > > > > >> >>>>> it
> > > > > > > >> >>>>>>> automatically based in the key fields defined in the
> > > > > > > schema/type.
> > > > > > > >> >>>>> Agree?
> > > > > > > >> >>>>>>>
> > > > > > > >> >>>>>>>
> > > > > > > >> >>>>>>>>
> > > > > > > >> >>>>>>>> 2016-08-01 19:47 GMT+03:00 Dmitriy Setrakyan <
> > > > > > > >> dsetrak...@apache.org
> > > > > > > >> >>>>>> :
> > > > > > > >> >>>>>>>>
> > > > > > > >> >>>>>>>>> Alex,
> > > > > > > >> >>>>>>>>>
> > > > > > > >> >>>>>>>>> In your case, why not just explicitly set hashcode
> > > every
> > > > > > time
> > > > > > > you
> > > > > > > >> >>>>>>> create
> > > > > > > >> >>>>>>>> an
> > > > > > > >> >>>>>>>>> object? There is BinaryObjectBuilder.hashCode(.
> ..)
> > > > > method.
> > > > > > > >> >>>>>>>>>
> > > > > > > >> >>>>>>>>> D.
> > > > > > > >> >>>>>>>>>
> > > > > > > >> >>>>>>>>> On Mon, Aug 1, 2016 at 7:42 AM, al.psc <
> > > > > > > >> >>>>>>> alexander.a.pasche...@gmail.com>
> > > > > > > >> >>>>>>>>> wrote:
> > > > > > > >> >>>>>>>>>
> > > > > > > >> >>>>>>>>>> Guys,
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>> It seems like this problem has become an
> important
> > > one
> > > > > once
> > > > > > > >> >>>>> again.
> > > > > > > >> >>>>>>>>>> In the course of working on
> > > > > > > >> >>>>>>>>>> https://issues.apache.org/
> jira/browse/IGNITE-2294
> > > (DML
> > > > > > > support)
> > > > > > > >> >>>>>>>> there's
> > > > > > > >> >>>>>>>>>> need
> > > > > > > >> >>>>>>>>>> to support binary marshaller. And, although we
> can
> > > > build
> > > > > > just
> > > > > > > >> >>>>>>>>> BinaryObject
> > > > > > > >> >>>>>>>>>> and put it to cache, without adequate hash code
> it
> > > > won't
> > > > > be
> > > > > > > >> >>>>> stored
> > > > > > > >> >>>>>>>>>> properly.
> > > > > > > >> >>>>>>>>>> Currently SQL MERGE works simply by deserializing
> > > newly
> > > > > > built
> > > > > > > >> >>>>>> object,
> > > > > > > >> >>>>>>>> but
> > > > > > > >> >>>>>>>>>> it's obviously wrong and is just a workaround
> > rather
> > > a
> > > > > > > solution.
> > > > > > > >> >>>>>>>>>> Has anyone come with possible design proposals
> for
> > > this
> > > > > > > >> problem's
> > > > > > > >> >>>>>>>>> solution?
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>> Thanks.
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>> - Alex
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>> --
> > > > > > > >> >>>>>>>>>> View this message in context:
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>
> > > > > > > >> >>>>>>>>
> > > > > > > >> >>>>>>>
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>
> > > > > > > >> >>
> > > > > > > >> http://apache-ignite-developers.2346864.n4.nabble.
> > > > > > > com/All-BinaryObjects-created-by-BinaryObjectBuilder-stored-
> > > > > > > at-the-same-partition-by-default-tp8042p10304.html
> > > > > > > >> >>>>>>>>>> Sent from the Apache Ignite Developers mailing
> list
> > > > > archive
> > > > > > > at
> > > > > > > >> >>>>>>>>> Nabble.com.
> > > > > > > >> >>>>>>>>>>
> > > > > > > >> >>>>>>>>>
> > > > > > > >> >>>>>>>>
> > > > > > > >> >>>>>>>
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>> --
> > > > > > > >> >>>>>> Andrey Gura
> > > > > > > >> >>>>>> GridGain Systems, Inc.
> > > > > > > >> >>>>>> www.gridgain.com
> > > > > > > >> >>>>>>
> > > > > > > >> >>>>>
> > > > > > > >> >>
> > > > > > > >> >>
> > > > > > > >>
> > > > > > > >>
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
