Mailing-List: contact dev-help@flink.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@flink.apache.org
MIME-Version: 1.0
In-Reply-To: <CANMXwW3-4ZMQ=RWabH=LYFcH9TV9XnaNDvuG=794-8_ps9n8sg@mail.gmail.com>
References: <CAAdrtT0csPT_WMgAskcuhQ2Q2kQbDbeuLixS9CWzE0Uo=udVDQ@mail.gmail.com>
 <CANMXwW0FFx4GfpAYmL5e2E21LEASGb+eLrYQ28hFWGOEuvoo4g@mail.gmail.com>
 <db0a706c-8d15-42d4-e166-7b5abf6bc291@touk.pl> <CANC1h_uqmvxUaWiEOVT8tgW6D2=R0gTzHDdnhFQM=nMcD2bVAA@mail.gmail.com>
 <CAAdrtT0X5ZvrzYS-SUAcwY5QXqBd-uvBgk9LOw7NMhfMebd=xg@mail.gmail.com>
 <CANMXwW0mo_79+xis-xzBnzd7qctA2AmcX5jTEqbvQo7GpdPTYg@mail.gmail.com>
 <CAAdrtT2o5RFW75VMec09y=3oOwhkL=xXrcfFi80g1YUtA2yKRg@mail.gmail.com> <CANMXwW3-4ZMQ=RWabH=LYFcH9TV9XnaNDvuG=794-8_ps9n8sg@mail.gmail.com>
From: Fabian Hueske <fhueske@gmail.com>
Date: Tue, 22 Nov 2016 13:33:47 +0100
Message-ID: <CAAdrtT2M0wko_RLZzBzd_-trz3o+Fe46dVOy_0hpHKp+SBm26w@mail.gmail.com>
Subject: Re: [DISCUSS] Hold copies in HeapStateBackend
To: "dev@flink.apache.org" <dev@flink.apache.org>
Content-Type: multipart/alternative; boundary=001a11408008b03fec0541e2fd19
archived-at: Tue, 22 Nov 2016 12:36:29 -0000

--001a11408008b03fec0541e2fd19
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Does anybody have objections against copying the first record that goes
into the ReduceState?

2016-11-22 12:49 GMT+01:00 Aljoscha Krettek <aljoscha@apache.org>:

> That's right, yes.
>
> On Mon, 21 Nov 2016 at 19:14 Fabian Hueske <fhueske@gmail.com> wrote:
>
> > Right, but that would be a much bigger change than "just" copying the
> > *first* record that goes into the ReduceState, or am I missing somethin=
g?
> >
> >
> > 2016-11-21 18:41 GMT+01:00 Aljoscha Krettek <aljoscha@apache.org>:
> >
> > > To bring over my comment from the Github PR that started this
> discussion:
> > >
> > > @wuchong <https://github.com/wuchong>, yes this is a problem with the
> > > HeapStateBackend. The RocksDB backend does not suffer from this
> problem.
> > I
> > > think in the long run we should migrate the HeapStateBackend to alway=
s
> > keep
> > > data in serialised form, then we also won't have this problem anymore=
.
> > >
> > > So I'm very much in favour of keeping data serialised. Copying data
> would
> > > only ever be a stopgap solution.
> > >
> > > On Mon, 21 Nov 2016 at 15:56 Fabian Hueske <fhueske@gmail.com> wrote:
> > >
> > > > Another approach that would solve the problem for our use case
> (object
> > > > re-usage for incremental window ReduceFunctions) would be to copy t=
he
> > > first
> > > > object that is put into the state.
> > > > This would be a change on the ReduceState, not on the overall state
> > > > backend, which should be feasible, no?
> > > >
> > > >
> > > >
> > > > 2016-11-21 15:43 GMT+01:00 Stephan Ewen <sewen@apache.org>:
> > > >
> > > > > -1 for copying objects.
> > > > >
> > > > > Storing a serialized data where possible is good, but copying all
> > > objects
> > > > > by default is not a good idea, in my opinion.
> > > > > A lot of scenarios use data types that are hellishly expensive to
> > copy.
> > > > > Even the current copy on chain handover is a problem.
> > > > >
> > > > > Let's not introduce even more copies.
> > > > >
> > > > > On Mon, Nov 21, 2016 at 3:16 PM, Maciek Pr=C3=B3chniak <mpr@touk.=
pl>
> > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > it will come with performance overhead when updating the state,
> > but I
> > > > > > think it'll be possible to perform asynchronous snapshots using
> > > > > > HeapStateBackend (probably some changes to underlying data
> > structures
> > > > > would
> > > > > > be needed) - which would bring more predictable performance.
> > > > > >
> > > > > > thanks,
> > > > > > maciek
> > > > > >
> > > > > >
> > > > > > On 21/11/2016 13:48, Aljoscha Krettek wrote:
> > > > > >
> > > > > >> Hi,
> > > > > >> I would be in favour of this since it brings things in line wi=
th
> > the
> > > > > >> RocksDB backend. This will, however, come with quite the
> > performance
> > > > > >> overhead, depending on how fast the TypeSerializer can copy.
> > > > > >>
> > > > > >> Cheers,
> > > > > >> Aljoscha
> > > > > >>
> > > > > >> On Mon, 21 Nov 2016 at 11:30 Fabian Hueske <fhueske@gmail.com>
> > > wrote:
> > > > > >>
> > > > > >> Hi everybody,
> > > > > >>>
> > > > > >>> when implementing a ReduceFunction for incremental aggregatio=
n
> of
> > > > SQL /
> > > > > >>> Table API window aggregates we noticed that the
> HeapStateBackend
> > > does
> > > > > not
> > > > > >>> store copies but holds references to the original objects. In
> > case
> > > > of a
> > > > > >>> SlidingWindow, the same object is referenced from different
> > window
> > > > > panes.
> > > > > >>> Therefore, it is not possible to modify these objects (in ord=
er
> > to
> > > > > avoid
> > > > > >>> object instantiations, see discussion [1]).
> > > > > >>>
> > > > > >>> Other state backends serialize their data such that the
> behavior
> > is
> > > > not
> > > > > >>> consistent across backends.
> > > > > >>> If we want to have light-weight tests, we have to create new
> > > objects
> > > > in
> > > > > >>> the
> > > > > >>> ReduceFunction causing unnecessary overhead.
> > > > > >>>
> > > > > >>> I would propose to copy objects when storing them in a
> > > > > HeapStateBackend.
> > > > > >>> This would ensure that objects returned from state to the use=
r
> > > behave
> > > > > >>> identical for different state backends.
> > > > > >>>
> > > > > >>> We created a related JIRA [2] that asks to copy records that =
go
> > > into
> > > > an
> > > > > >>> incremental ReduceFunction. The scope is more narrow and woul=
d
> > > solve
> > > > > our
> > > > > >>> problem, but would leave the inconsistent behavior of state
> > > backends
> > > > in
> > > > > >>> place.
> > > > > >>>
> > > > > >>> What do others think?
> > > > > >>>
> > > > > >>> Cheers, Fabian
> > > > > >>>
> > > > > >>> [1]
> > https://github.com/apache/flink/pull/2792#discussion_r88653721
> > > > > >>> [2] https://issues.apache.org/jira/browse/FLINK-5105
> > > > > >>>
> > > > > >>>
> > > > > >
> > > > >
> > > >
> > >
> >
>

--001a11408008b03fec0541e2fd19--