-
Notifications
You must be signed in to change notification settings - Fork 575
[GLUTEN-10317][FLINK] Support state related operation #10320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format?
See also: |
.github/workflows/flink.yml
Outdated
@@ -43,7 +43,7 @@ jobs: | |||
source /opt/rh/gcc-toolset-11/enable | |||
sudo dnf install -y patchelf | |||
git clone -b gluten-0530 https://github.com/bigo-sg/velox4j.git | |||
cd velox4j && git reset --hard 0eb9eef589692dbde953c36ecd2d8f9d3a34a59d | |||
cd velox4j && git reset --hard 7bf3bd14624733230a395306275ccc3bed671b6c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about use tag instead of commit hash to track the version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, we now use a private branch and it does't update with main branch, we can use git tag when after merge with the oap velox.
int rowtimeIndex = windowSpecParams.f3; | ||
int windowType = windowSpecParams.f4; | ||
PartitionFunctionSpec sliceAssignerSpec = | ||
new WindowPartitionFunctionSpec(inputType, rowtimeIndex, size, slide, offset, windowType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I note Flink's window is completely different from Spark SQL's. It would be better to use a better name to easily distinguish in Velox4j and Velox, e.g., StreamWindowPartitionFunctionSpec
looks better for clarity than WindowPartitionFunctionSpec
. cc @zhztheplayer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the WindowPartitionFunctionSpec
is newly added in bigo-sg
velox,and it’s specifically for flink to divide the window into different slices by the input time
field,and spark window does not need this. and I think it is better to use some name that can be identified as flink specific. @PHILO-HE @shuai-xu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have rename related classes to begin with Stream, please help to review again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
What changes were proposed in this pull request?
This pr update velox to a version, which contains support for state. And make serval operators contains Join, WindowJoin, GlobalAgg, LocalAgg use state for calculation. These operators's bahivor is like flink now, but still not the same.
The main changes are in Velox.
(Fixes: #10317)
How was this patch tested?
This patch was tested by manual tests.