-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
HIVE-25996: Backport HIVE-25098 to branch-2.3 #3066
Conversation
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.
Just a question: What about branch-3.1
and branch-3.0
? Are these patches released officially in any Apache Hive artifacts after vote?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
cc @sunchao |
Thanks to the work done by @wangyum, Hive 2.3 is widely used in the industry and adopted by many downstream projects such as Apache Spark. A security patch release is greatly appreciated. |
Are there still plans to get this into the Hive 2.3 branch? |
cc @sunchao this is the first step of upgrading thrift to get rid of CVE (0.14.0+), as we are planning Hive 2.3.10, please reopen this PR. |
@wangyum could you rebase the PR? |
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.
This file removed by: 1945e2f
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.
This file removed by: 1945e2f
There are around 30~40 tests failure in the latest branch-2.3, some of them are flaky. Jenkins reports 41 test failures for this PR, which seems reasonable. |
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.
Verified thrift-generated files locally with
# install thrift compiler 0.14.1 at /home/chengpan/bin/thrift-0.14.1/compiler/cpp/cmake-build
# provide share/fb303/if/fb303.thrift
mvn -pl metastore -Pthriftif -Dthrift.home=/home/chengpan/bin/thrift-0.14.1/compiler/cpp/cmake-build compile -am
Without fb303.thrift
the compile command failed with the following error message, is it expected?
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-antrun-plugin:1.7:run (generate-thrift-sources) on project hive-metastore: An Ant BuildException has occured: The following error occurred while executing this line:
[ERROR] /home/chengpan/Projects/hive-2.3/metastore/target/antrun/build-main.xml:15: exec returned: 1
[ERROR] around Ant part ...<for param="thrift.file">... @ 9:28 in /home/chengpan/Projects/hive-2.3/metastore/target/antrun/build-main.xml
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <args> -rf :hive-metastore
@pan3793 It should throw
It works if add fb303.thrift to
|
Yes, I fixed it by adding |
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.
LGTM.
I manually verified the Thrift changes, and the CI results. There is one new test failure after this but it is unrelated.
This PR is important to fix a CVE issue in branch-2.3
@wangyum could you resolve the conflict? I'll merge this after it. |
Thank you @sunchao. Conflict resolved. |
Thanks! merged to branch-2.3 |
This reverts commit 8be7466.
What changes were proposed in this pull request?
Backport HIVE-25098 to branch-2.3.
Why are the changes needed?
Make the downstreams easy to upgrade their Thrift version.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Local test.