[GitHub] [yunikorn-core] wilfred-s commented on pull request #502: [YUNIKORN-1566] Config update can leave queue settings in an inconsistent state

["wilfred-s (via GitHub)" <gi...@apache.org>]

wilfred-s commented on PR #502:
URL: https://github.com/apache/yunikorn-core/pull/502#issuecomment-1430539454

   Looking through the changes I am not too worried about the new config not being OK to apply. The config validation looks at the new config and makes sure it all parses correctly and that the structure is valid. If we have a gap in that area we should update the config validator. BTW: I think we are missing the template validation against the queue it is defined on. We also copy the template to a parent queue below the point it is defined without validating.
   
   The new configured values should be applied recursively from the top down so that should all work.
   
   What I am worried about is that the new config does not map correctly to the existing in memory config.
   These are some of the cases I think we do not check for: 
   * the in memory config shows a queue `root.parent.leaf`  The newly created config has only got `root.parent`. Do we remove the queue `leaf`? Parent does not change type to a leaf and it was an auto parent before. How do we handle this...
   * the in memory config shows `root.my-queue` defined as a leaf. The newly created config has `root.my-queue.spark`. That as far as I can tell causes an automatic type change to parent for my-queue. What happens with the applications that run there?
   * the in memory config shows `root.my-queue.spark` Both `my-queue` and `spark` are dynamic queues. The newly created config has `root.my-queue` as a leaf. That breaks things badly I think.


-- 
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: reviews-unsubscribe@yunikorn.apache.org

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