krishan1390 commented on PR #5443: URL: https://github.com/apache/hadoop/pull/5443#issuecomment-1451319920
Thanks for your feedback @slfan1989 . Please find my response below allKeys needs to be consistently updated across all router instances > Multiple Routers will share and store the Delegation token, there is no updated across all router instances. > > That is not the actual behaviour currently. Each router instance has its own set of master keys (allKeys & currentKey - these are setup on service startup through startThreads() & updated in rollMasterKey()). Even though they are stored in database, master key isn't looked up from database but just returned from the in memory variables (allKeys & currentKey). So a router instance can't renew tokens generated from another router instance. > > And even delegation tokens are not consistently updated across router instances. If a delegation token is present in currentTokens variable in multiple router instances but updated in one router instance (on token renewal), the other router instances will use their own in memory variable currentTokens rather than look up the database and thus can say the token is expired. DB update exceptions are swallowed & returned as a success if just in memory variables are updated > MemeoryStateStore is only used for verification and should not be used for production. SQLServerFederationStateStore will not swallow exceptions, and the client cannot complete verification with the old token. > > Yes. By in memory variables I meant the class instance variables like currentTokens, currentKey, allKeys, etc. Even though SQLFederationStateStore throws an exception, all the current methods in RouterDelegationTokenSecretManager catches these exceptions and either returns or terminates the app rather than throwing it back & handling appropriately. Purging Delegation Token / Master key on expiry assumes all tokens are available in memory > We only cache tokens in MemeoryStateStore, but MemeoryStateStore is not a distributed storage and can only be used for test verification. It is recommended to use ZKFederationStateStore Or SQLServerFederationStateStore. > > Yes by in memory variables I meant the class instance variables like currentTokens, currentKey, allKeys. Currently, it doesn't look at all variables in the database. APIs like get all tokens return only in memory data which is incorrect. > getAllToken is only used for test verification. > > Yes currently thats the case, but since its a public API its an incorrect design to return the in memory instance variables in case of router - since anyone can rely on this. > Sorry, I have been busy recently, I will add a design document, DB storage Delegation Token, I refer to the design of Hive storage Delegation Token, this is a stable capability. > > No problem. I will provide the test cases to correctly show these issues better > > My main thought at a high level, is that by design a stateless router should make no assumptions of underlying state management (AbstractDelegationTokenSecretManager functionalities are heavily relying on it being a single source of truth rather than being distributed) -- 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]
