Skip to content
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

[Refactor] Update iceberg version to 0.14.1 #12306

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

southernriver
Copy link
Contributor

@southernriver southernriver commented Oct 19, 2022

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Which issues of this PR fixes :

Since iceberg has updated to the version of 0.14.1, we may update to suit more core/api optimization from that.

Problem Summary(Required) :

For current version of 0.12.1, all dependency scope is set to compile
image

but for version 0.14.1, all dependency is set to runtime. (iceberg-mr module is different sine 0.13+ version.)
image

So we need to change dependencies to suit that.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • I have added user document for my new feature or new function

@southernriver
Copy link
Contributor Author

southernriver commented Oct 19, 2022

Another known risk is that starrocks-fe module forced to use avro-1.8.2 which introduced by spark-core, but iceberg itself has updated avro to 1.10.1 for iceberg version of 0.12.1, and updated avro to 1.10.2 for iceberg version of 0.14.1.

Maybe We need to unified that to suit latest version for iceberg.

@southernriver southernriver changed the title Update iceberg version to 0.14.1 [Refactor] Update iceberg version to 0.14.1 Oct 19, 2022
@southernriver southernriver changed the title [Refactor] Update iceberg version to 0.14.1 [WIP][Refactor] Update iceberg version to 0.14.1 Oct 19, 2022
@southernriver southernriver changed the title [WIP][Refactor] Update iceberg version to 0.14.1 [Refactor] Update iceberg version to 0.14.1 Oct 19, 2022
@wanpengfei-git
Copy link
Collaborator

[FE PR Coverage Check]

😞 fail : 1 / 2 (50.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/external/iceberg/hive/CachedClientPool.java 0 1 00.00% [59]
🔵 com/starrocks/external/iceberg/hive/HiveClientPool.java 1 1 100.00% []

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@southernriver
Copy link
Contributor Author

run starrocks_clang-tidy

@stephen-shelby
Copy link
Contributor

Thanks for your contribution. I want to know which optimization feature you want to use when upgrading. And why choose 0.14.1. Because 0.14.1 was released last month, the latest version of iceberg is 1.0.0.

@southernriver
Copy link
Contributor Author

Thanks for your contribution. I want to know which optimization feature you want to use when upgrading. And why choose 0.14.1. Because 0.14.1 was released last month, the latest version of iceberg is 1.0.0.

Thanks for your time. I concern about the following features:

  • Puffin format is upcoming to support serveral index in our product environment (It's a file format for statistics and index payloads or sketches)
  • ManifestFile read optimize or bug fix , related pr like #5206, #4083, (#4520, #4560, #4304

It would be better to keep the version iterating on a regular basis with the community. The current version we widely
used is 0.14.* , I think upgrating to 1.0.0 is also ok, how do you think?

@stephen-shelby
Copy link
Contributor

run starrocks_admit_test

@stephen-shelby
Copy link
Contributor

Thanks for your contribution. I want to know which optimization feature you want to use when upgrading. And why choose 0.14.1. Because 0.14.1 was released last month, the latest version of iceberg is 1.0.0.

Thanks for your time. I concern about the following features:

  • Puffin format is upcoming to support serveral index in our product environment (It's a file format for statistics and index payloads or sketches)
  • ManifestFile read optimize or bug fix , related pr like #5206, #4083, (#4520, #4560, #4304

It would be better to keep the version iterating on a regular basis with the community. The current version we widely used is 0.14.* , I think upgrating to 1.0.0 is also ok, how do you think?

I think 0.14.1 is ok. After removing the iceberg-mr, have you checked whether the dependencies about iceberg-* are redundancy or lost. I will trigger a regression task based on this pr. The result will come later.

@@ -38,7 +38,7 @@ under the License.
<starrocks.home>${basedir}/../../</starrocks.home>
<fe_ut_parallel>${env.FE_UT_PARALLEL}</fe_ut_parallel>
<jacoco.version>0.8.5</jacoco.version>
<iceberg.version>0.12.1</iceberg.version>
<iceberg.version>0.14.1</iceberg.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to move it to parent pom properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants