Skip to content

In TableCache, invoke callback on config change listener during registration#8302

Merged
Jackie-Jiang merged 1 commit intoapache:masterfrom
Jackie-Jiang:fix_table_cache_test
Mar 7, 2022
Merged

In TableCache, invoke callback on config change listener during registration#8302
Jackie-Jiang merged 1 commit intoapache:masterfrom
Jackie-Jiang:fix_table_cache_test

Conversation

@Jackie-Jiang
Copy link
Contributor

Currently when the config change listener is registered, the callback is not triggered automatically. It relies on the caller to set the current configs into the change listener, which is hard to use because there might be other changes invoking the onChange() callback at the same time.
This PR changes the listener registration method to directly call the onChange() with the current configs.
Also improve the test and fix #8153

@codecov-commenter
Copy link

codecov-commenter commented Mar 5, 2022

Codecov Report

Merging #8302 (6fd4bdf) into master (90d1136) will decrease coverage by 6.72%.
The diff coverage is 0.00%.

❗ Current head 6fd4bdf differs from pull request most recent head 920b44c. Consider uploading reports for the commit 920b44c to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8302      +/-   ##
============================================
- Coverage     70.77%   64.05%   -6.73%     
  Complexity     4246     4246              
============================================
  Files          1631     1586      -45     
  Lines         85485    83617    -1868     
  Branches      12878    12679     -199     
============================================
- Hits          60502    53559    -6943     
- Misses        20815    26196    +5381     
+ Partials       4168     3862     -306     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 66.95% <0.00%> (-0.03%) ⬇️
unittests2 14.16% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...pache/pinot/common/config/provider/TableCache.java 0.00% <0.00%> (-83.86%) ⬇️
...a/org/apache/pinot/common/metrics/MinionMeter.java 0.00% <0.00%> (-100.00%) ⬇️
...g/apache/pinot/common/metrics/ControllerMeter.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/BrokerQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/MinionQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
...ache/pinot/server/access/AccessControlFactory.java 0.00% <0.00%> (-100.00%) ⬇️
...he/pinot/common/messages/SegmentReloadMessage.java 0.00% <0.00%> (-100.00%) ⬇️
...pinot/core/data/manager/realtime/TimerService.java 0.00% <0.00%> (-100.00%) ⬇️
...che/pinot/controller/util/TableMetadataReader.java 0.00% <0.00%> (-100.00%) ⬇️
...pinot/minion/exception/TaskCancelledException.java 0.00% <0.00%> (-100.00%) ⬇️
... and 372 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 90d1136...920b44c. Read the comment docs.

Copy link
Contributor

@npawar npawar left a comment

Choose a reason for hiding this comment

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

lgtm

@Jackie-Jiang Jackie-Jiang merged commit 308b371 into apache:master Mar 7, 2022
@Jackie-Jiang Jackie-Jiang deleted the fix_table_cache_test branch March 7, 2022 21:08
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.

TableCacheTest is flaky

3 participants