[
https://issues.apache.org/jira/browse/BEAM-11887?focusedWorklogId=564741&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-564741
]
ASF GitHub Bot logged work on BEAM-11887:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 11/Mar/21 16:53
Start Date: 11/Mar/21 16:53
Worklog Time Spent: 10m
Work Description: kennknowles commented on a change in pull request
#14120:
URL: https://github.com/apache/beam/pull/14120#discussion_r592528690
##########
File path:
runners/flink/src/main/java/org/apache/beam/runners/flink/translation/functions/FlinkMergingNonShuffleReduceFunction.java
##########
@@ -83,7 +83,7 @@ public void reduce(
new FlinkSideInputReader(sideInputs, getRuntimeContext());
AbstractFlinkCombineRunner<K, InputT, AccumT, OutputT, W> reduceRunner;
- if
(windowingStrategy.getWindowFn().windowCoder().equals(IntervalWindow.getCoder()))
{
+ if (windowingStrategy.needsMerge() && windowingStrategy.getWindowFn()
instanceof Sessions) {
Review comment:
I wonder if I was wrong actually and this is redundant. I would guess
`FlinkMergingNonShuffleReduceFunction` will not be chosen unless `needsMerge`
is already known to be true. Sorry I missed the context for this change. I
think it is fine, but maybe not needed after all.
##########
File path:
runners/flink/src/main/java/org/apache/beam/runners/flink/translation/functions/FlinkPartialReduceFunction.java
##########
@@ -98,11 +98,10 @@ public void combine(
if (groupedByWindow) {
reduceRunner = new SingleWindowFlinkCombineRunner<>();
} else {
- if (windowingStrategy.needsMerge()
- &&
!windowingStrategy.getWindowFn().windowCoder().equals(IntervalWindow.getCoder()))
{
- reduceRunner = new HashingFlinkCombineRunner<>();
- } else {
+ if (windowingStrategy.needsMerge() && windowingStrategy.getWindowFn()
instanceof Sessions) {
Review comment:
is `SingleWindowFlinkCombineRunner` actually what is used for
non-merging?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 564741)
Time Spent: 2h 20m (was: 2h 10m)
> testMergingCustomWindowsWithoutCustomWindowTypes failing on Flink VR
> --------------------------------------------------------------------
>
> Key: BEAM-11887
> URL: https://issues.apache.org/jira/browse/BEAM-11887
> Project: Beam
> Issue Type: Bug
> Components: test-failures
> Reporter: Kenneth Knowles
> Assignee: Yichi Zhang
> Priority: P1
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> https://ci-beam.apache.org/view/PostCommit/job/beam_PostCommit_Java_ValidatesRunner_Flink/8618/testReport/junit/org.apache.beam.sdk.transforms.windowing/WindowTest/testMergingCustomWindowsWithoutCustomWindowTypes/
> Maybe the category of test needs to be sickbayed. But maybe the new test is
> not quite right? Better look into it.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
