Skip to content

Conversation

@KKcorps
Copy link
Contributor

@KKcorps KKcorps commented Mar 30, 2023

We can set the following configs to configure the timeout

external.view.dropped.max.wait.ms - The duration of time in milliseconds to wait for the external view to be dropped. Default - 20 minutes.

external.view.check.interval.ms - The period in milliseconds in which to ping ZK for latest EV state.

@KKcorps KKcorps requested a review from Jackie-Jiang March 30, 2023 09:36
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023

Codecov Report

Merging #10510 (94eaec6) into master (3687b2b) will increase coverage by 6.70%.
The diff coverage is 80.00%.

@@             Coverage Diff              @@
##             master   #10510      +/-   ##
============================================
+ Coverage     63.23%   69.94%   +6.70%     
- Complexity     5905     6123     +218     
============================================
  Files          2036     2092      +56     
  Lines        110973   112716    +1743     
  Branches      16892    17143     +251     
============================================
+ Hits          70177    78836    +8659     
+ Misses        35606    28314    -7292     
- Partials       5190     5566     +376     
Flag Coverage Δ
integration1 24.50% <80.00%> (+<0.01%) ⬆️
integration2 24.21% <80.00%> (-0.15%) ⬇️
unittests1 67.60% <ø> (-0.35%) ⬇️
unittests2 13.89% <ø> (?)

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

Impacted Files Coverage Δ
...server/starter/helix/HelixInstanceDataManager.java 73.18% <66.66%> (-1.01%) ⬇️
.../starter/helix/HelixInstanceDataManagerConfig.java 80.00% <100.00%> (+0.83%) ⬆️

... and 414 files with indirect coverage changes

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

throws Exception {
// Wait externalview to converge
long endTimeMs = System.currentTimeMillis() + EXTERNAL_VIEW_DROPPED_MAX_WAIT_MS;
long endTimeMs = System.currentTimeMillis() + _instanceDataManagerConfig.getExternalViewDroppedMaxWaitMs();
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) Let's cache these 2 values into local variable (or as member variable during init())

// Check if the external view is dropped for a table, and if so, wait for the external view to
// be updated for a maximum of this time.
private static final String EXTERNAL_VIEW_DROPPED_MAX_WAIT_MS = "external.view.dropped.max.wait.ms";
private static final String EXTERNAL_VIEW_CHECK_INTERVAL_MS = "external.view.check.interval.ms";
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor)

Suggested change
private static final String EXTERNAL_VIEW_CHECK_INTERVAL_MS = "external.view.check.interval.ms";
private static final String EXTERNAL_VIEW_DROPPED_CHECK_INTERVAL_MS = "external.view.dropped.check.interval.ms";

@Jackie-Jiang Jackie-Jiang added the Configuration Config changes (addition/deletion/change in behavior) label Mar 30, 2023
@KKcorps KKcorps merged commit 3062bd0 into apache:master Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Configuration Config changes (addition/deletion/change in behavior)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants