Deprecate instanceId Config For Broker/Minion Specific Configs

[ankitsultana]

fixes #9290

I felt it might be best to mark instanceId as deprecated and create a new config for Minion ID. Lmk your thoughts.

cc: @Jackie-Jiang

[codecov-commenter]

Codecov Report

Merging #9308 (62d9b00) into master (0adf5ef) will increase coverage by 6.05%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##             master    #9308      +/-   ##
============================================
+ Coverage     63.64%   69.70%   +6.05%     
- Complexity     4984     5060      +76     
============================================
  Files          1809     1882      +73     
  Lines         96993   100177    +3184     
  Branches      14832    15232     +400     
============================================
+ Hits          61735    69831    +8096     
+ Misses        30707    25397    -5310     
- Partials       4551     4949     +398     

Flag	Coverage Δ
integration1	26.19% <57.14%> (?)
integration2	24.77% <57.14%> (?)
unittests1	66.89% <ø> (-0.22%)	⬇️
unittests2	15.22% <71.42%> (-0.17%)	⬇️

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

Impacted Files	Coverage Δ
...va/org/apache/pinot/spi/utils/CommonConstants.java	27.69% <ø> (ø)
.../main/java/org/apache/pinot/minion/MinionConf.java	87.50% <50.00%> (+47.50%)	⬆️
...e/pinot/broker/broker/helix/BaseBrokerStarter.java	79.06% <100.00%> (+7.23%)	⬆️
...ot/segment/spi/index/reader/SortedIndexReader.java	33.33% <0.00%> (-33.34%)	⬇️
...ava/org/apache/pinot/spi/utils/ArrayCopyUtils.java	63.95% <0.00%> (-24.62%)	⬇️
...a/org/apache/pinot/segment/spi/ColumnMetadata.java	80.00% <0.00%> (-20.00%)	⬇️
...cheduler/resources/PolicyBasedResourceManager.java	75.00% <0.00%> (-18.75%)	⬇️
...ore/query/scheduler/resources/ResourceManager.java	84.00% <0.00%> (-12.00%)	⬇️
...java/org/apache/pinot/core/common/DataFetcher.java	77.81% <0.00%> (-11.93%)	⬇️
...che/pinot/controller/api/access/AccessControl.java	27.27% <0.00%> (-9.10%)	⬇️
... and 525 more

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

[Jackie-Jiang]

We can deprecate instanceId

Currently controller and server is using pinot.<type>.instance.id as the key, I'd suggest making broker and minion following the same pattern to be consistent

[ankitsultana]

Currently controller and server is using pinot..instance.id as the key, I'd suggest making broker and minion following the same pattern to be consistent

@Jackie-Jiang : We might need to deprecate pinot.broker.id as well in that case. Is that okay?

[Jackie-Jiang]

Currently controller and server is using pinot..instance.id as the key, I'd suggest making broker and minion following the same pattern to be consistent

@Jackie-Jiang : We might need to deprecate pinot.broker.id as well in that case. Is that okay?

@ankitsultana Per the current logic, I think we can directly modify the CONFIG_OF_BROKER_ID to pinot.broker.instance.id without worrying about backward compatibility because the field is always set within the BaseBrokerStarter and then read again

[ankitsultana]

The field is read in BrokerAdminApiApplication and BaseBrokerRequestHandler.

But you are right, I took a look and both of these classes get the value from the one set by BaseBrokerStarter. I'll update the PR.

[ankitsultana]

@Jackie-Jiang : Updated. One of the integration tests is failing with a "ServerSegmentMissing" error. Not sure if it is related.

[Jackie-Jiang]

LGTM. Let me re-run the failed test