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 Sun, 07 Aug 2016 06:11:28 GMT
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.

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

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