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

Remove redundant default broker configurations #4106

Merged
merged 1 commit into from
Apr 17, 2019
Merged

Conversation

Jackie-Jiang
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang commented Apr 12, 2019

  • Remove config based routing configs because the config based
    routing was removed long time ago
  • Remove HelixBrokerStarter.getZkAddressForBroker() because it
    does not apply to the current implementation
  • Replace config key strings with constants
  • Change timeout for default broker and broker in integration
    test to 60s
  • Replace PropertiesConfiguration with light weight
    BaseConfiguration if not read from config file

@Jackie-Jiang Jackie-Jiang changed the title Remove redundant default broker configurations Remove redundant default broker configurations (rebase on #4105) Apr 12, 2019
@Jackie-Jiang Jackie-Jiang changed the title Remove redundant default broker configurations (rebase on #4105) Remove redundant default broker configurations Apr 12, 2019
@Jackie-Jiang Jackie-Jiang force-pushed the broker_config branch 2 times, most recently from 1de7523 to 454e550 Compare April 15, 2019 23:40
- Remove config based routing configs because the config based
  routing was removed long time ago
- Remove HelixBrokerStarter.getZkAddressForBroker() because it
  does not apply to the current implementation
- Replace config key strings with constants
- Change timeout for default broker and broker in integration
  test to 60s
- Replace PropertiesConfiguration with light weight
  BaseConfiguration if not read from config file
@codecov-io
Copy link

Codecov Report

Merging #4106 into master will decrease coverage by 0.03%.
The diff coverage is 73.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #4106      +/-   ##
============================================
- Coverage     66.88%   66.84%   -0.04%     
  Complexity       20       20              
============================================
  Files          1037     1036       -1     
  Lines         51444    51417      -27     
  Branches       7193     7190       -3     
============================================
- Hits          34407    34371      -36     
- Misses        14664    14683      +19     
+ Partials       2373     2363      -10
Impacted Files Coverage Δ Complexity Δ
.../pinot/broker/broker/helix/HelixBrokerStarter.java 76.66% <73.33%> (+0.15%) 0 <0> (ø) ⬇️
...ller/validation/OfflineSegmentIntervalChecker.java 26.43% <0%> (-48.28%) 0% <0%> (ø)
...apache/pinot/common/metrics/ValidationMetrics.java 43.47% <0%> (-36.24%) 0% <0%> (ø)
...egation/function/customobject/MinMaxRangePair.java 75.86% <0%> (-24.14%) 0% <0%> (ø)
...lix/EmptyBrokerOnlineOfflineStateModelFactory.java 86.66% <0%> (-13.34%) 0% <0%> (ø)
...mpl/dictionary/DoubleOffHeapMutableDictionary.java 69.09% <0%> (-9.1%) 0% <0%> (ø)
...nsport/netty/PooledNettyClientResourceManager.java 85.41% <0%> (-4.17%) 0% <0%> (ø)
...e/operator/dociditerators/BitmapDocIdIterator.java 60.71% <0%> (-3.58%) 0% <0%> (ø)
...impl/dictionary/FloatOffHeapMutableDictionary.java 85.45% <0%> (-1.82%) 0% <0%> (ø)
...ata/manager/realtime/RealtimeTableDataManager.java 41.17% <0%> (-1.48%) 0% <0%> (ø)
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88081bd...13890c3. Read the comment docs.

Copy link
Contributor

@snleee snleee left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for cleaning up

@Jackie-Jiang Jackie-Jiang merged commit 79b3368 into master Apr 17, 2019
@Jackie-Jiang Jackie-Jiang deleted the broker_config branch April 17, 2019 00:48
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