Re: Request for review : IGNITE-3303 Apache Flink Integration - Flink source

2018-09-08 Thread Saikat Maitra 
Hi Andrew, Alexey

I have incorporated the review changes.

I have also refactored the CacheEventSerializer class and moved it to test
folder because it is used only in the FlinkIgniteSourceSelfExample and not
required for IgniteSource.

Build links https://ci.ignite.apache.org/viewLog.html?buildId=1821778;

https://ci.ignite.apache.org/viewLog.html?buildId=1821774;

Please review and share feedback.

Regards
Saikat

On Tue, Sep 4, 2018 at 9:57 PM, Saikat Maitra 
wrote:

> Hi Alexey,
>
> Thank you for reviewing the changes and sharing feedback, I am updating
> the PR. I will share the changes shortly.
>
> Regards,
> Saikat
>
> On Tue, Sep 4, 2018 at 10:59 AM, Alexey Goncharuk <
> alexey.goncha...@gmail.com> wrote:
>
>> Hello Saikat,
>>
>> I see a few fellow Igniters added some comments to your PR (including me).
>> I believe the PR can be merged after you address them.
>>
>> Thanks,
>> AG
>>
>> пт, 31 авг. 2018 г. в 3:11, Saikat Maitra :
>>
>> > Thank you, Denis
>> >
>> > Regards,
>> > Saikat
>> >
>> > On Thu, Aug 30, 2018 at 7:01 PM, Denis Magda  wrote:
>> >
>> > > Hello Saikat,
>> > >
>> > > Hopefully, someone from the community will review the changes in the
>> > > nearest time.
>> > >
>> > > --
>> > > Denis
>> > >
>> > > On Thu, Aug 30, 2018 at 4:37 PM Saikat Maitra <
>> saikat.mai...@gmail.com>
>> > > wrote:
>> > >
>> > > > Hello,
>> > > >
>> > > > The changes for IGNITE-3303 for IgniteSource is complete. This will
>> > help
>> > > is
>> > > > streaming data from Ignite cluster and process, filter, transform
>> and
>> > > > publish it back to Ignite using IgniteSink or in any other data
>> sink.
>> > > >
>> > > > I was hoping if the changes can be approved I can go ahead merge the
>> > > > changes.
>> > > >
>> > > >
>> > > > Regards,
>> > > > Saikat
>> > > >
>> > > >
>> > > >
>> > > > On Tue, Aug 28, 2018 at 12:56 AM, Saikat Maitra <
>> > saikat.mai...@gmail.com
>> > > >
>> > > > wrote:
>> > > >
>> > > > > Hi Andrew,
>> > > > >
>> > > > > As discussed I have incorporated the changes. Please review and
>> let
>> > me
>> > > > > know if any changes required.
>> > > > >
>> > > > > Regards,
>> > > > > Saikat
>> > > > >
>> > > > > On Mon, Aug 27, 2018 at 1:45 AM, Saikat Maitra <
>> > > saikat.mai...@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > >> Hi,
>> > > > >>
>> > > > >> I have updated the PR with additional tests.
>> > > > >>
>> > > > >> Please review and share feedback.
>> > > > >>
>> > > > >> This PR is related to IgniteSink but allows to stream data from
>> > > Ignite.
>> > > > >>
>> > > > >> PR https://github.com/apache/ignite/pull/870/files
>> > > > >>
>> > > > >> Review https://reviews.ignite.apache.
>> org/ignite/review/IGNT-CR-135
>> > > > >>
>> > > > >> Regards,
>> > > > >> Saikat
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

Re: Critical worker threads liveness checking drawbacks

2018-09-08 Thread Andrey Kuznetsov 
David, Yakov, I understand your fears. But liveness checks deal with
_critical_ conditions, i.e. when such a condition is met we conclude the
node as totally broken, and there is no sense to keep it alive regardless
the data it contains. If we want to give it a chance, then the condition
(long fsync etc.) should not considered as critical at all.

сб, 8 сент. 2018 г. в 15:18, Yakov Zhdanov :

> Agree with David. We need to have an opporunity set backups count threshold
> (at runtime also!) that will not allow any automatic stop if there will be
> a data loss. Andrey, what do you think?
>
> --Yakov
>


-- 
Best regards,
  Andrey Kuznetsov.

Re: Critical worker threads liveness checking drawbacks

2018-09-08 Thread Yakov Zhdanov 
Agree with David. We need to have an opporunity set backups count threshold
(at runtime also!) that will not allow any automatic stop if there will be
a data loss. Andrey, what do you think?

--Yakov