Support initializing broker tags from config #12175
Conversation
Jackie-Jiang
added
feature
documentation
release-notes
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #12175 +/- ##
============================================
- Coverage 61.64% 61.61% -0.03%
Complexity 1153 1153
============================================
Files 2407 2407
Lines 130915 130920 +5
Branches 20225 20227 +2
============================================
- Hits 80697 80670 -27
- Misses 44320 44362 +42
+ Partials 5898 5888 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Jackie-Jiang
force-pushed
the
broker_tags_from_config
branch
from
bfefbcf
to
238bb19
Compare
| for (String instanceTag : StringUtils.split(instanceTagsConfig, ',')) { | ||
| Preconditions.checkArgument(TagNameUtils.isBrokerTag(instanceTag), "Illegal broker instance tag: %s", | ||
| instanceTag); | ||
| instanceConfig.addTag(instanceTag); |
There was a problem hiding this comment.
Considering this comes from a config file, shall we do instanceTag.trim()?
There was a problem hiding this comment.
Initially I did add trim(), but then decided to remove it to keep it consistent with PinotHelixResourceManager.updateInstanceTags().
If we find people making mistakes frequently, we can consider changing both to use trim(). Currently we don't have a check that tag cannot start/end with space, so this might cause backward-incompatible
Address #11755
Support initializing broker tags from config and automatically updating broker resource when broker joins the cluster for the first time.
Configuration
pinot.broker.instance.tags(comma-separated): Broker tags to set when broker joins the cluster for the first time