ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Setrakyan <dsetrak...@apache.org>
Subject Re: All BinaryObjects created by BinaryObjectBuilder stored at the same partition by default
Date Sun, 07 Aug 2016 19:26:02 GMT
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