ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergi Vladykin <sergi.vlady...@gmail.com>
Subject Re: All BinaryObjects created by BinaryObjectBuilder stored at the same partition by default
Date Mon, 08 Aug 2016 09:38:27 GMT
I like what Vladimir proposed.

As for `encoded string` implementation, I guess it should be just an
implementation with a list of fields which have to participate in hashCode
and equals. I guess in 99% of cases everyone generates hashCode and equals
with IDE, we will just implement the same logic in our HashResolver. I
guess IDEA, Eclipse, Netbeans generate this code in the same way, but of
course we should check.

Sergi


2016-08-08 11:54 GMT+03:00 Alexey Goncharuk <alexey.goncharuk@gmail.com>:

> I like the idea with string-based BinaryEqualsHashCodeResolver a lot. I
> think it should be as flexible as possible. Since EqualsHashCode resolver
> becomes a part of cache configuration, we get another part of configuration
> which must be deployed on server nodes and defeats benefits of
> BinaryMarshaller.
>
> Also, I think it would be great to have an ability to add Comparable
> implementation in this interface.
>
> 2016-08-08 10:30 GMT+03:00 Vladimir Ozerov <vozerov@gridgain.com>:
>
> > 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 <
> dsetrakyan@apache.org>
> > wrote:
> >
> > > On Sat, Aug 6, 2016 at 8:11 PM, Sergi Vladykin <
> sergi.vladykin@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.paschenko@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"
<
> > > > > dsetrakyan@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.vladykin@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 <
> > dsetrakyan@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.paschenko@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 <
> > > > dsetrakyan@apache.org
> > > > > >:
> > > > > > > > > > On Tue, Aug 2, 2016 at 7:36 AM, Denis Magda
<
> > > > dmagda@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 <
> > > > > > vozerov@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 <
> > > > > > dmagda@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.paschenko@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 <
> > > > > > > > dsetrakyan@apache.org
> > > > > > > > > >:
> > > > > > > > > >> >>>> On Mon, Aug 1, 2016
at 11:38 PM, Vladimir Ozerov <
> > > > > > > > > >> vozerov@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 <
> > > > > > > agura@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 <
> > > > > > > > > >> >> dsetrakyan@apache.org
> > > > > > > > > >> >>>>>>
> > > > > > > > > >> >>>>>> wrote:
> > > > > > > > > >> >>>>>>
> > > > > > > > > >> >>>>>>> On Mon,
Aug 1, 2016 at 10:01 AM, Alexey
> Goncharuk
> > <
> > > > > > > > > >> >>>>>>> alexey.goncharuk@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 <
> > > > > > > > > >> dsetrakyan@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.paschenko@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
> > > > > > > > > >> >>>>>>
> > > > > > > > > >> >>>>>
> > > > > > > > > >> >>
> > > > > > > > > >> >>
> > > > > > > > > >>
> > > > > > > > > >>
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message