YARN-9708. Yarn Federation Router Support DelegationToken. #4746
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code, thank you very much! |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Can you help review this part of the code? Thank you very much! |
slfan1989
changed the title
goiri
reviewed
Aug 17, 2022
...test/java/org/apache/hadoop/yarn/server/federation/utils/TestFederationStateStoreFacade.java
Outdated
Show resolved
Hide resolved
...n/java/org/apache/hadoop/yarn/server/router/security/RouterDelegationTokenSecretManager.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, thank you very much! |
goiri
reviewed
Aug 23, 2022
| @@ -112,4 +115,14 @@ public YARNDelegationTokenIdentifierProto getProto() { | |||
| setBuilderFields(); | |||
| return builder.build(); | |||
| } | |||
|
|
|||
| @InterfaceAudience.Private | |||
There was a problem hiding this comment.
Can we import the InterfaceAudience.Private as others?
There was a problem hiding this comment.
I will modify the code.
|
|
||
| // Restore the DelegationKey from the request | ||
| RouterMasterKey masterKey = request.getRouterMasterKey(); | ||
| ByteBuffer keyByteBuf = masterKey.getKeyBytes(); |
There was a problem hiding this comment.
We seem to do this a few times. Can we extract?
There was a problem hiding this comment.
Thanks for your suggestion, I will refactor this part of the code.
...on/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/RouterMasterKey.java
Show resolved
Hide resolved
| @InterfaceAudience.Private | ||
| @InterfaceStability.Unstable | ||
| public static RouterMasterKeyResponse newInstance(RouterMasterKey masterKey) { | ||
| RouterMasterKeyResponse request = |
|
|
||
| // DTIdentifier -> renewDate | ||
| private Map<RMDelegationTokenIdentifier, Long> delegationTokenState = | ||
| new HashMap<RMDelegationTokenIdentifier, Long>(); |
There was a problem hiding this comment.
I will modify the code.
| facade.storeNewToken(dtId1, renewDate1); | ||
|
|
||
| Map<RMDelegationTokenIdentifier, Long> token1 = | ||
| new HashMap<RMDelegationTokenIdentifier, Long>(); |
| facade.storeNewToken(dtId1, renewDate1); | ||
|
|
||
| Map<RMDelegationTokenIdentifier, Long> token1 = | ||
| new HashMap<RMDelegationTokenIdentifier, Long>(); |
| YarnConfiguration.RM_DELEGATION_TOKEN_RENEW_INTERVAL_DEFAULT); | ||
|
|
||
| return new RouterDelegationTokenSecretManager(secretKeyInterval, | ||
| tokenMaxLifetime, tokenRenewInterval, 3600000); |
There was a problem hiding this comment.
3600000 make it 10 hours: 10 * 60 * 60 * 1000
There was a problem hiding this comment.
Thanks for your suggestion. I will fix it.
I think hardcoding 3600000 should not be a reasonable way, the parameters should be extracted into the configuration file, because this part is consistent with RM, it will use RM-related parameter names.
It is not very reasonable to extract this parameter in this pr into the configuration file, I submitted a separate pr for extracting this parameter into the configuration file.
JIRA: YARN-11253. Add Configuration to delegationToken RemoverScanInterval.
PR: #4751
Can you help review this pr? Thank you very much!
There was a problem hiding this comment.
3600000 make it 10 hours: 10 * 60 * 60 * 1000
I double checked, it should be 1 hour, 60 * 60 * 1000
| DelegationKey paramKey = new DelegationKey(1234, 4321, "keyBytes".getBytes()); | ||
| DelegationKey responseKey = secretManager.getMasterKeyByDelegationKey(paramKey); | ||
|
|
||
| Assert.assertNotNull(paramKey); |
| Assert.assertNotNull(paramKey); | ||
| Assert.assertEquals(storeKey.getExpiryDate(), responseKey.getExpiryDate()); | ||
| Assert.assertEquals(storeKey.getKeyId(), responseKey.getKeyId()); | ||
| Assert.assertTrue(Arrays.equals(storeKey.getEncodedKey(), responseKey.getEncodedKey())); |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, Thank you very much! |
goiri
reviewed
Aug 24, 2022
| */ | ||
| @InterfaceAudience.Public | ||
| package org.apache.hadoop.yarn.security.client.impl.pb; | ||
| import org.apache.hadoop.classification.InterfaceAudience; |
There was a problem hiding this comment.
I will modify the code.
| * @param masterKey masterKey | ||
| * @return DelegationKey | ||
| */ | ||
| private DelegationKey getDelegationKeyByMasterKey(RouterMasterKey masterKey) { |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, Thank you very much! |
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, Thank you very much! |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, Thank you very much! |
goiri
reviewed
Aug 29, 2022
| import org.apache.hadoop.yarn.proto.YarnSecurityTokenProtos.YARNDelegationTokenIdentifierProtoOrBuilder; | ||
| import org.apache.hadoop.yarn.security.client.YARNDelegationTokenIdentifier; | ||
|
|
||
| @InterfaceAudience.Private |
There was a problem hiding this comment.
Thank you very much for your help reviewing the code, I will fix it.
|
💔 -1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, Thank you very much! |
goiri
reviewed
Sep 1, 2022
|
|
||
| @Private | ||
| @Unstable | ||
| public class YARNDelegationTokenIdentifierPBImpl extends YARNDelegationTokenIdentifier { |
There was a problem hiding this comment.
Do we have PBImpl tests?
It would be good to split the PR if possible.
There was a problem hiding this comment.
Thank you very much for your help reviewing the code, I will add PBImpl tests. This pr code is a bit too much, I will split this pr.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA: YARN-9708. Yarn Router Support DelegationToken.
In the YARN non-federation mode,
RMDelegationTokenIdentifieris generated, updated, and deleted by RM.In the YARN federation mode, the Router replaces the RM to interact with the Client, so the operation of the DelegationToken originally performed by the RM will be replaced by the Router.
In this process, it is necessary to ensure that the original generation method of DelegationToken remains unchanged.
1.RM's management of DelegationToken
RMDelegationTokenIdentifieris managed byRMDelegationTokenSecretManager,RMDelegationTokenSecretManagerinherits fromAbstractDelegationTokenSecretManager.AbstractDelegationTokenSecretManageris responsible for the generation of MasterKey and Token.RMDelegationTokenSecretManageris responsible for storing MasterKey and Token.RMDelegationTokenSecretManagercontains 5 important methods.In the process of persisting Token, RM mainly relies on RMStateStore for storage, among which there are two key implementations, MemoryRMStateStore, ZKRMStateStore.
We can refer to the following figure
1.
AbstractDelegationTokenSecretManagercallsupdateCurrentKeyinstartThreadandrollMasterKey2.
updateCurrentKeycallsstoreDelegationKey3.
storeDelegationKeycallsstoreNewMasterKey,4.Because
AbstractDelegationTokenSecretManageris an abstract class, the concrete implementation will be called, andRMDelegationTokenSecretManagerwill be called forstoreNewMasterKey5.
storeNewMasterKeycall rm.getRMContext().getStateStore().storeRMDTMasterKey6.RM StateStore will send an RMStateStoreEventType.STORE_MASTERKEY event
7. We can find the following code logic in RMStateStore:
StoreRMDTMasterKeyTransitionwill finally process the request ofstoreRMDTMasterKeyState.8.1 MemoryRMStateStore#storeRMDTMasterKeyState
8.2 ZKRMStateStore#storeRMDTMasterKeyState
The calling logic of the other four methods(removeStoredMasterKey, storeNewToken, updateStoredToken, removeStoredToken) is similar to that of storeNewMasterKey.
2.Router's management of DelegationToken
The Router will replace the RM to interact with the client in the federation, and the client cannot directly connect to the RM. In order to ensure the function of RM DelegationToken, we should implement it on the Router side according to the implementation method of RM.
RMDelegationTokenSecretManager, the Router will InheritAbstractDelegationTokenSecretManagerand implementRouterDelegationTokenSecretManagerFederationStateStoreWe can refer to the following figure
1.
AbstractDelegationTokenSecretManagercallsupdateCurrentKeyinstartThreadandrollMasterKey2.
updateCurrentKeycallsstoreDelegationKey3.
storeDelegationKeycallsstoreNewMasterKey4.Because
AbstractDelegationTokenSecretManageris an abstract class, the concrete implementation will be called, andRouterDelegationTokenSecretManagerwill be called forstoreNewMasterKey5.
RouterDelegationTokenSecretManager#storeNewMasterKeycallsFederationFacade#storeNewMasterKey6.
FederationFacade#storeNewMasterKeycallsFederationStateStore#storeNewMasterKey7. For the specific implementation class of
FederationStateStore, the implementation of related methods will be completed.