[DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

Stephan Ewen
Hi all!

I want to discuss merging this PR to the 1.11 release branch:
https://github.com/apache/flink/pull/12306

It contains the new FLIP-126 Watermarks, and per-partition watermarking to
the FLIP-27 sources. In that sense it is partially a new feature after the
feature freeze. Hence this discussion, and not just merging.

The reasons why I suggest to back-port this to 1.11 are
  - It is API breaking. Without this patch, we would release a Source API
and immediately break compatibility in the next release.
  - The FLIP-27 feature is experimental, but it should not be useless in
the sense that users have to re-write all implemented sources in the next
release.
  - It is a fairly isolated change, does not affect any existing feature in
the system

Please let me know if you have concerns about this.

Best,
Stephan
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

Becket Qin
Usually we should avoid checking in patches other than bug fix after
feature freeze. However, in this particular case, the code base is sort of
in an incomplete state - an exposed known-to-change feature - due to
missing this patch. Fixing forward seems the best option. Besides that,
FLIP-27 has been highly anticipated by many users. So if one patch
completes the story, personally speaking I am +1 to backport given the
isolated impact and significant benefit of doing that.

Thanks,

Jiangjie (Becket) Qin


On Tue, May 26, 2020 at 4:43 PM Stephan Ewen <[hidden email]> wrote:

> Hi all!
>
> I want to discuss merging this PR to the 1.11 release branch:
> https://github.com/apache/flink/pull/12306
>
> It contains the new FLIP-126 Watermarks, and per-partition watermarking to
> the FLIP-27 sources. In that sense it is partially a new feature after the
> feature freeze. Hence this discussion, and not just merging.
>
> The reasons why I suggest to back-port this to 1.11 are
>   - It is API breaking. Without this patch, we would release a Source API
> and immediately break compatibility in the next release.
>   - The FLIP-27 feature is experimental, but it should not be useless in
> the sense that users have to re-write all implemented sources in the next
> release.
>   - It is a fairly isolated change, does not affect any existing feature
> in the system
>
> Please let me know if you have concerns about this.
>
> Best,
> Stephan
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

Piotr Nowojski-4
Hi,

As we discussed this offline a bit, initially I was sceptical to merge it,
as:
- even it’s an isolated change, it can destabilise the builds and prolong
release testing period
- is distracting from solving release blockers etc

However all in all I’m +0.5 to merge it because of this argument:

> - It is API breaking. Without this patch, we would release a Source API
and immediately break compatibility in the next release.

And this:

>  - It is a fairly isolated change, does not affect any existing feature
in the system

Is limiting our risks, that we are not risking introducing bugs into the
existing features.

Piotrek

wt., 26 maj 2020 o 12:43 Becket Qin <[hidden email]> napisał(a):

> Usually we should avoid checking in patches other than bug fix after
> feature freeze. However, in this particular case, the code base is sort of
> in an incomplete state - an exposed known-to-change feature - due to
> missing this patch. Fixing forward seems the best option. Besides that,
> FLIP-27 has been highly anticipated by many users. So if one patch
> completes the story, personally speaking I am +1 to backport given the
> isolated impact and significant benefit of doing that.
>
> Thanks,
>
> Jiangjie (Becket) Qin
>
>
> On Tue, May 26, 2020 at 4:43 PM Stephan Ewen <[hidden email]> wrote:
>
>> Hi all!
>>
>> I want to discuss merging this PR to the 1.11 release branch:
>> https://github.com/apache/flink/pull/12306
>>
>> It contains the new FLIP-126 Watermarks, and per-partition watermarking
>> to the FLIP-27 sources. In that sense it is partially a new feature after
>> the feature freeze. Hence this discussion, and not just merging.
>>
>> The reasons why I suggest to back-port this to 1.11 are
>>   - It is API breaking. Without this patch, we would release a Source API
>> and immediately break compatibility in the next release.
>>   - The FLIP-27 feature is experimental, but it should not be useless in
>> the sense that users have to re-write all implemented sources in the next
>> release.
>>   - It is a fairly isolated change, does not affect any existing feature
>> in the system
>>
>> Please let me know if you have concerns about this.
>>
>> Best,
>> Stephan
>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

Zhijiang(wangzhijiang999)
In the beginning, I have somehow similar concerns as Piotr mentioned below.
After some offline discussions, also as explained by Stephan and Becket here, I am +1 to backport it to release-1.11.

Best,
Zhijiang


------------------------------------------------------------------
From:Piotr Nowojski <[hidden email]>
Send Time:2020年5月26日(星期二) 18:51
To:Becket Qin <[hidden email]>
Cc:Stephan Ewen <[hidden email]>; dev <[hidden email]>; zhijiang <[hidden email]>
Subject:Re: [DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

Hi,

As we discussed this offline a bit, initially I was sceptical to merge it,
as:
- even it’s an isolated change, it can destabilise the builds and prolong
release testing period
- is distracting from solving release blockers etc

However all in all I’m +0.5 to merge it because of this argument:

> - It is API breaking. Without this patch, we would release a Source API
and immediately break compatibility in the next release.

And this:

>  - It is a fairly isolated change, does not affect any existing feature
in the system

Is limiting our risks, that we are not risking introducing bugs into the
existing features.

Piotrek

wt., 26 maj 2020 o 12:43 Becket Qin <[hidden email]> napisał(a):

> Usually we should avoid checking in patches other than bug fix after
> feature freeze. However, in this particular case, the code base is sort of
> in an incomplete state - an exposed known-to-change feature - due to
> missing this patch. Fixing forward seems the best option. Besides that,
> FLIP-27 has been highly anticipated by many users. So if one patch
> completes the story, personally speaking I am +1 to backport given the
> isolated impact and significant benefit of doing that.
>
> Thanks,
>
> Jiangjie (Becket) Qin
>
>
> On Tue, May 26, 2020 at 4:43 PM Stephan Ewen <[hidden email]> wrote:
>
>> Hi all!
>>
>> I want to discuss merging this PR to the 1.11 release branch:
>> https://github.com/apache/flink/pull/12306
>>
>> It contains the new FLIP-126 Watermarks, and per-partition watermarking
>> to the FLIP-27 sources. In that sense it is partially a new feature after
>> the feature freeze. Hence this discussion, and not just merging.
>>
>> The reasons why I suggest to back-port this to 1.11 are
>>   - It is API breaking. Without this patch, we would release a Source API
>> and immediately break compatibility in the next release.
>>   - The FLIP-27 feature is experimental, but it should not be useless in
>> the sense that users have to re-write all implemented sources in the next
>> release.
>>   - It is a fairly isolated change, does not affect any existing feature
>> in the system
>>
>> Please let me know if you have concerns about this.
>>
>> Best,
>> Stephan
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27

Aljoscha Krettek-2
+1

I'm in favour of backporting this because we otherwise would immediately
break the API between 1.11 and 1.12.

Best,
Aljoscha

On 26.05.20 17:05, Zhijiang wrote:

> In the beginning, I have somehow similar concerns as Piotr mentioned below.
> After some offline discussions, also as explained by Stephan and Becket here, I am +1 to backport it to release-1.11.
>
> Best,
> Zhijiang
>
>
> ------------------------------------------------------------------
> From:Piotr Nowojski <[hidden email]>
> Send Time:2020年5月26日(星期二) 18:51
> To:Becket Qin <[hidden email]>
> Cc:Stephan Ewen <[hidden email]>; dev <[hidden email]>; zhijiang <[hidden email]>
> Subject:Re: [DISCUSS] Backpoint FLIP-126 (watermarks) integration with FLIP-27
>
> Hi,
>
> As we discussed this offline a bit, initially I was sceptical to merge it,
> as:
> - even it’s an isolated change, it can destabilise the builds and prolong
> release testing period
> - is distracting from solving release blockers etc
>
> However all in all I’m +0.5 to merge it because of this argument:
>
>> - It is API breaking. Without this patch, we would release a Source API
> and immediately break compatibility in the next release.
>
> And this:
>
>>   - It is a fairly isolated change, does not affect any existing feature
> in the system
>
> Is limiting our risks, that we are not risking introducing bugs into the
> existing features.
>
> Piotrek
>
> wt., 26 maj 2020 o 12:43 Becket Qin <[hidden email]> napisał(a):
>
>> Usually we should avoid checking in patches other than bug fix after
>> feature freeze. However, in this particular case, the code base is sort of
>> in an incomplete state - an exposed known-to-change feature - due to
>> missing this patch. Fixing forward seems the best option. Besides that,
>> FLIP-27 has been highly anticipated by many users. So if one patch
>> completes the story, personally speaking I am +1 to backport given the
>> isolated impact and significant benefit of doing that.
>>
>> Thanks,
>>
>> Jiangjie (Becket) Qin
>>
>>
>> On Tue, May 26, 2020 at 4:43 PM Stephan Ewen <[hidden email]> wrote:
>>
>>> Hi all!
>>>
>>> I want to discuss merging this PR to the 1.11 release branch:
>>> https://github.com/apache/flink/pull/12306
>>>
>>> It contains the new FLIP-126 Watermarks, and per-partition watermarking
>>> to the FLIP-27 sources. In that sense it is partially a new feature after
>>> the feature freeze. Hence this discussion, and not just merging.
>>>
>>> The reasons why I suggest to back-port this to 1.11 are
>>>    - It is API breaking. Without this patch, we would release a Source API
>>> and immediately break compatibility in the next release.
>>>    - The FLIP-27 feature is experimental, but it should not be useless in
>>> the sense that users have to re-write all implemented sources in the next
>>> release.
>>>    - It is a fairly isolated change, does not affect any existing feature
>>> in the system
>>>
>>> Please let me know if you have concerns about this.
>>>
>>> Best,
>>> Stephan
>>>
>>>
>