Skip to content

[GOBBLIN-1886] Upgrade to Helix 1.1.0 #3748

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

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

homatthew
Copy link
Contributor

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots (if applicable):

Who knows what changed between these versions. 1.0.3 release notes don't exist but here are the 1.0.4 ones 🤣

The main thing that seems important is ZK client version bumped from 3.4.9 to 3.6.3, which means the default ZK client version will have a client side jute max buffer decreased from 4MB to 1MB. Within LinkedIn, we addressed this by overriding with the following java arg. This client buffer will be exceeded if a znode exceeds ~8k children. This will occur within Gobblin if your Gobblin jobs are particularly large and you are using Zookeeper as your watermark state store. NOTE: if you were using zookeeper as your state store at this scale, you will see many other scaling issues. Some of which need to be fixed with just more ZK observers, a beefier ZK cluster, or more Helix controllers.

-Djute.maxbuffer=4194304

Bug Fixes
Fixed size history for Scheduled Workflow tasks (#2036)
fix bug where generateJobList would iterate until the parallelism is reached #2065
Fixed website deployment and bump up corresponding dependencies
Do not proceed with cluster creation if addCluster() fails. (#2068)
Improvements
Issue #1970: allow clients to prevent HelixProperty from cloning the ZNRecord #2072
Fix issue #2117: improve efficiency of ClusterStatusMonitor and PerInstanceResourceMonitor, plus misc code cleanup (#2118)
support common input for operational input on Maintenance Management API #2055
MaintenanceManagementService improvement - Allow implementation of OperationInterface to return null #2035
Allow using passed in connection config when reading cloud config (#2099)
Replace non-inclusive terms in tutorial.md (#2039)
[issue-1728] upgrade guava due to cve (#2042)
upgrade jackson to 2.12.6.1 due to cve (#2043)
[issue-1727] upgrade to commons-io 2.11.0 due to cve (#2040)
upgrade snakeyaml to v1.30 due to cve (#2041)
move to Apache Analytics(Matomo)
Bump up helix front from 5 to 13 for angular JS.

1.1.0

New ZkClient version bump up
The new ZkClient version bumped from 3.4.9 to 3.6.3

ACL support in Helix UI
Helix UI (helix-front) support ACLed access.

Detailed Changes
Bug Fixes
Fixes #2185 NullPointerException in ResourceUtil.java (#2186)
Reuse zkclient in BestPossibleExternalViewVerifier and fix resource leak (#2180)
Fix partition-list row expand bug (#2211)
Improvements
Cluster deletion confirmation for helix-front (#2274)
Implement ViewClusterRefresher caching synchronization (#2199)
Optimize HelixTaskExecutor reset() in event of shutdown (#2183)
Add TASK_THREADPOOL_RESET_TIMEOUT as system property (#2177)
Delete Cluster UI [helix-front] (#2159)

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    No tests because this is a dependency upgrade. It should be caught at compile time and via unit tests

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Copy link
Contributor

@ZihanLi58 ZihanLi58 left a comment

Choose a reason for hiding this comment

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

+1

@codecov-commenter
Copy link

codecov-commenter commented Aug 22, 2023

Codecov Report

Merging #3748 (49223ea) into master (4e6fa49) will increase coverage by 0.00%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #3748   +/-   ##
=========================================
  Coverage     47.00%   47.00%           
- Complexity    10848    10851    +3     
=========================================
  Files          2147     2147           
  Lines         84895    84895           
  Branches       9419     9419           
=========================================
+ Hits          39902    39905    +3     
- Misses        41359    41361    +2     
+ Partials       3634     3629    -5     

see 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ZihanLi58 ZihanLi58 merged commit 90380fc into apache:master Aug 24, 2023
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