bq 1. ApplicationClientProtocol#updateApplicationTimeouts .Could this be an Evolving api?.
the API is already UnStable, I think should be fine. cc:/Jian He
2. ApplicationClientProtocolPBClientImpl#updateApplicationTimeouts. Does exception handling block needs return? RPCUtil method will throw exception, correct?
3. In ApplicationClientProtocolPBServiceImpl#updateApplicationTimeouts, we use catch (YarnException| IOException e).
frankly I just copy pasted from other API implementation from PBImpl. It looks to be all API are doing the same way. If really want to clean, we can take separate task to do refactoring PBClientImpl classes. Thoughts?
5. Given a writeLock in RMAppImpl#updateApplicationTimeout, why do we need another lock in RMAppManager#updateApplicationTimeout. Is this to handle some race conditions while app update event is waiting in StateStore dispatcher queue? I would love to have some more comments in these synchronized blocks or write locks to give a brief explanation. It will help us later
Good question!! Jian also had same doubt. Let me brief aobut it. Anything holding writeLock in RMAppImpl blocks the main AsyncDiapatcher. This is costly operation. For updateTimeout, we need to wait for transaction to complete which can not be done holding RMAppImpl lock. So, in-memory updations happens from RMAppImpl and trigger an event to StateStore and release the writeLock. Wait for statestore update call from RMAppManager in separate lock.
4. On a different note, i think COMPLETED_APP_STATES could be defined by RMAppImpl itself and expose a read-only api. This can help to cleanup local states definitions. could be done in another patch.
I would prefer to do in separate JIRA. May be can you raise it?
6. RMApp is generally considered as read-only. updateApplicationTimeout will violate that. we can place this api in RMAppImpl itself, and in client side, we could convert to RMAppImpl object and use. ProportionPolicy, New Global Scheduler etc are using this way.
I would prefer to add in RMApp itself rather than type casting in RMAppManager. It is internal interface used for both read and write.
7. Timeout is to be part of ApplicationReport correct? Is that a part of this patch?
this will be done in