[GitHub] [incubator-yunikorn-core] wilfred-s commented on pull request #207: [YUNIKORN-405] Added checksum validation

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

wilfred-s commented on pull request #207:
URL: https://github.com/apache/incubator-yunikorn-core/pull/207#issuecomment-706013750


   > If the checksum failed, updateConfig fails, the client-side could be very confusing... what do we expect them to do? Is it supposed to ask them to call a GET to reload the latest config from YK, and overwrite their existing configs in whatever storage? What if users already made some changes in their local, how they can restore that, and apply to the latest content? This is very complex.
   
   Yes, the clients needs to do the GET and update its configuration. It is not complex. It is for the client to handle that issue.
   If they really do not care they can do a GET, replace the checksum and push the change again. However, it does put the onus on the client to do the right thing.
   The simple thing that the checksum prevents is clients from updating the configuration without thinking about whom else might have updated the running configuration. It could be that the client is out of sync, been down for a while or the user has left an old update sitting in an uncommitted state, changes could have been made via a different client too.
   I would see a client workflow as follows:
   1. GET config 
   2. make change (keep delta) 
   3. PUT changed config
   4. PUT:  OK update checksum, persist config
     PUT: fail, show message and either go directly back to 1 and let user redo it from scratch or
   5. GET config, apply delta 
   6. Show changed config (delta applied) go back to 2.
   
   >The point to have a source-of-truth discussion is to make this model simpler. If we allow user to update configs via configUpdate API, that means the client side will have storage used as the source-of-truth for configs, not something in YK's memory. Think YARN as an example, the storage will be either a file, or ZK, or DB. This way, client just need to maintain the configs in their end, overwrite the YK side changes when needed.
   
   The YARN API is a single change at a time: add, remove or update a single queue or update a set of values in the global config. YARN is the source of truth and *implicitly* expects that the client knows the current state. In YARN you pass a delta to be applied on top of the current config, it does not accept a complete new configuration. It still can fail every single change. A completely different point to start from.
   
   For us we *cannot* set the source of truth outside of YuniKorn. We do not control the client, which would mean we give up control completely. YuniKorn has to be the source of truth, with the checksum check we at least push the client to think about what they are doing.
   YuniKorn runs with a configuration. It has queues and applications running in that queue etc. A configuration change that would just blindly overwrite what is running within the system is dangerous. YuniKorn needs to be able to push back and in some way say _Are you sure that you want to do this?_ If it is then accept it and make the change, with all the side effects it has.
   
   


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