[GitHub] [storm] Ethanlm edited a comment on pull request #3365: STORM-3727 handle long values for SUPERVISOR_SLOTS_PORTS

[GitBox <gi...@apache.org>]

Ethanlm edited a comment on pull request #3365:
URL: https://github.com/apache/storm/pull/3365#issuecomment-754775896


   I think I know what's going on here now.
   
   As @agresch mentioned, this issue can be reproduced by `./bin/storm supervisor -c supervisor.slots.ports="[6700]"`
   
   When supervisor starts, it reads defaults.yaml, storm.yaml and the command option, then validate the configs.
   
   https://github.com/apache/storm/blob/7bef73a6faa14558ef254efe74cbe4bfef81c2e2/storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java#L126
   
   https://github.com/apache/storm/blob/7bef73a6faa14558ef254efe74cbe4bfef81c2e2/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java#L438-L442
   
   However, the object type of `supervisor.slots.ports` interpreted by `readCommandLineOpts`, which uses json parser, is `JSONArray` and the element in the JSONArray is `Long`. The code is:
   https://github.com/apache/storm/blob/f451be2ef81c0821f65a1a4d671b90c221ef99db/storm-client/src/jvm/org/apache/storm/utils/Utils.java#L287-L298
   
   On the other hand, the object type of `supervisor.slots.ports` interpreted by `findAndReadConfigFile`, which uses `Yaml`, is `ArrayList<List>`. The code is 
   https://github.com/apache/storm/blob/f451be2ef81c0821f65a1a4d671b90c221ef99db/storm-client/src/jvm/org/apache/storm/utils/Utils.java#L179-L182
   
   
   But the `IntegerValidator` doesn't enforce the object type to be Integer. 
   https://github.com/apache/storm/blob/7bef73a6faa14558ef254efe74cbe4bfef81c2e2/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java#L404-L415
   
   It will pass the check if the value has `longValue() == doubleValue()` and and the value is in the range of `[Integer.MIN_VALUE, Integer.MAX_VALUE]`, even "1.0" meets the requirement.
   
   Changing the `IntegerValidator` code to 
   ```
   public void validateField(String name, Object o) {
               SimpleTypeValidator.validateField(name, Integer.class, o);
           }
   ```
   like https://github.com/apache/storm/blob/7bef73a6faa14558ef254efe74cbe4bfef81c2e2/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java#L390
   
   will give us:
   
   ```
   java.lang.Error: java.lang.IllegalArgumentException: Field SUPERVISOR_SLOTS_PORTS list entry must be of type class java.lang.Integer. Object: 6700 actual type: class java.lang.Long
   ```
   when the command is `./bin/storm supervisor -c supervisor.slots.ports="[6700]"`
   
   With that being said, I am okay with reverting the code back to original to fix this issue STORM-3727. 
   
   Filed a separate jira STORM-3734


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org