yashmayya opened a new pull request, #19648: URL: https://github.com/apache/pinot/pull/19648
## The bug Two parsers split comma-separated values, but do not trim the parts. **Listener access protocols.** `ServiceStartableUtils.applyClusterConfig` copies cluster configs into the instance config with `PinotConfiguration.setProperty`. This method keeps the raw value. For the cluster config `controller.access.protocols=http, https`, `ListenerConfigUtil.buildListenerConfigs` gets the protocol name `" https"`. It then reads `controller.access.protocols. https.port`, finds no value, and controller startup fails with `null is not a valid port`. The broker, server and minion listeners use the same code. The same value in the instance config file works. A config that is built from a map or a file splits the value and trims each item. **`skipIndexes` query option.** `QueryOptionsUtils.getSkipIndexes` splits on `&`, `=` and `,`, but does not trim: - `SET skipIndexes='col1=inverted, range&col2=sorted'` fails with `No enum constant ...IndexType. RANGE`. - A column name with a space, for example `" col2"`, does not match the column, so the option has no effect for that column. ## The fix - `ListenerConfigUtil` trims each protocol name and skips empty names. A blank value still gives no listeners. - `QueryOptionsUtils` trims each column name and index type. It also uppercases the index type with `Locale.ROOT`, like the `SqlOptionsMode` parsing in the same class. ## Testing - `ListenerConfigUtilTest`: protocol values with spaces and empty names, set with `setProperty` like `applyClusterConfig` does, and blank values. - `QueryOptionsUtilsTest`: `skipIndexes` values with spaces around the names. - Each new test case fails on the old code. - `spotless`, `checkstyle` and `license` are clean. ## Notes - #19644 fixes the same cluster config problem for list configs that are read with `getProperty(String, List)`, including `ControllerConf.getControllerAccessProtocols()`. This PR trims inline, because `getCommaSeparatedList` from #19644 is not merged yet. - Only servers parse `skipIndexes`. During a rolling upgrade, old servers still reject values with spaces. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
