[
https://issues.apache.org/jira/browse/YARN-10022?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17016697#comment-17016697
]
Prabhu Joseph edited comment on YARN-10022 at 1/20/20 11:16 AM:
----------------------------------------------------------------
Thanks [~kmarton] for the patch. Have few comments on the patch
1. CapacityScheduler#reinitialize replaces calling validateConf() with below
which is not required.
{code:java}
CapacitySchedulerConfigValidator.validateMemoryAllocation(this.conf);
CapacitySchedulerConfigValidator.validateVCores(this.conf);
{code}
2. In CapacityScheduler#reinitialize, distinguishRuleSet returned by
validatePlacementRules can be used.
{code:java}
+CapacitySchedulerConfigValidator
+ .validatePlacementRules(placementRuleStrs);
+ Set<String> distinguishRuleSet = new HashSet<>(placementRuleStrs);
{code}
3. In CapacitySchedulerQueueManager, there is a typo
{code:java}
- // When failing over, if using configuration store, don't validate queue
+ // When failing over, if using configuration store, don't validate queueR
{code}
4. In RMWSConsts, method name is validateAndGetSchedulerConfiguration
{code:java}
+ /** Path for {@code RMWebServiceProtocol#validateCapacitySchedulerConfig}. */
{code}
5. In RMWebServices, it has to be initForWritableEndpoints. Only admin user is
allowed to read scheduler conf,
in order to avoid leaking sensitive info, such as ACLs. Reference:
RMWebServices#getSchedulerConfiguration()
{code:java}
+ initForReadableEndpoints();
{code}
6. Below lines of code is not straightforward
{code:java}
+ Configuration config = new Configuration(false);
+ rm.getRMContext().getRMAdminService().getConfiguration(config,
+ YarnConfiguration.CS_CONFIGURATION_FILE);
+ MutableCSConfigurationProvider provider
+ = new MutableCSConfigurationProvider(null);
+
+ CapacitySchedulerConfiguration capacitySchedulerConfig =
+ new CapacitySchedulerConfiguration(config, false);
+ Configuration newConfig =
provider.applyChanges(capacitySchedulerConfig,
+ mutationInfo);
{code}
can be replaced similar to the one in RMWebServices#getSchedulerConfiguration()
like below
{code:java}
MutableConfigurationProvider mutableConfigurationProvider =
((MutableConfScheduler) scheduler).getMutableConfProvider();
Configuration schedulerConf = mutableConfigurationProvider
.getConfiguration();
Configuration newConfig =
mutableConfigurationProvider.applyChanges(schedulerConf,
mutationInfo);
{code}
*Change in AdminService.java is not required with above.
*getConfiguration() has to be added into the interface
MutableConfigurationProvider
7. Error message "CS" can be expanded to "CapacityScheduler"
{code:java}
+ String errorMsg = "CS configuration validation failed: "
+ + e.toString();
{code}
8. Error Message is not added in the Error Response.
{code:java}
+ return Response.status(Status.BAD_REQUEST)
+ .build();
{code}
to
{code:java}
return Response.status(Status.BAD_REQUEST).entity(errorMsg)
.build();
{code}
9. Below error message is wrong
{code:java}
+ String errorMsg = "Configuration change only supported by " +
+ "MutableConfScheduler.";
{code}
was (Author: prabhu joseph):
Thanks [~kmarton] for the patch. Have few comments on the patch
1. CapacityScheduler#reinitialize replaces calling validateConf() with below
which is not required.
{code:java}
CapacitySchedulerConfigValidator.validateMemoryAllocation(this.conf);
CapacitySchedulerConfigValidator.validateVCores(this.conf);
{code}
2. In CapacityScheduler#reinitialize, distinguishRuleSet returned by
validatePlacementRules can be used.
{code:java}
+CapacitySchedulerConfigValidator
+ .validatePlacementRules(placementRuleStrs);
+ Set<String> distinguishRuleSet = new HashSet<>(placementRuleStrs);
{code}
3. In CapacitySchedulerQueueManager, there is a typo
{code:java}
- // When failing over, if using configuration store, don't validate queue
+ // When failing over, if using configuration store, don't validate queueR
{code}
4. In RMWSConsts, method name is validateAndGetSchedulerConfiguration
{code:java}
+ /** Path for {@code RMWebServiceProtocol#validateCapacitySchedulerConfig}. */
{code}
5. In RMWebServices, it has to be initForWritableEndpoints. Only admin user is
allowed to read scheduler conf,
in order to avoid leaking sensitive info, such as ACLs. Reference:
RMWebServices#getSchedulerConfiguration()
{code:java}
+ initForReadableEndpoints();
{code}
6. Below lines of code is not straightforward
{code:java}
+ Configuration config = new Configuration(false);
+ rm.getRMContext().getRMAdminService().getConfiguration(config,
+ YarnConfiguration.CS_CONFIGURATION_FILE);
+ MutableCSConfigurationProvider provider
+ = new MutableCSConfigurationProvider(null);
+
+ CapacitySchedulerConfiguration capacitySchedulerConfig =
+ new CapacitySchedulerConfiguration(config, false);
+ Configuration newConfig =
provider.applyChanges(capacitySchedulerConfig,
+ mutationInfo);
{code}
can be replaced similar to the one in RMWebServices#getSchedulerConfiguration()
like below
{code:java}
MutableConfigurationProvider mutableConfigurationProvider =
((MutableConfScheduler) scheduler).getMutableConfProvider();
Configuration schedulerConf = mutableConfigurationProvider
.getConfiguration();
Configuration newConfig =
provider.applyChanges(capacitySchedulerConfig,
mutationInfo);
{code}
Change in AdminService.java is not required with above.
7. Error message "CS" can be expanded to "CapacityScheduler"
{code:java}
+ String errorMsg = "CS configuration validation failed: "
+ + e.toString();
{code}
8. Error Message is not added in the Error Response.
{code:java}
+ return Response.status(Status.BAD_REQUEST)
+ .build();
{code}
to
{code:java}
return Response.status(Status.BAD_REQUEST).entity(errorMsg)
.build();
{code}
9. Below error message is wrong
{code:java}
+ String errorMsg = "Configuration change only supported by " +
+ "MutableConfScheduler.";
{code}
> Create RM Rest API to validate a CapacityScheduler Configuration
> ----------------------------------------------------------------
>
> Key: YARN-10022
> URL: https://issues.apache.org/jira/browse/YARN-10022
> Project: Hadoop YARN
> Issue Type: New Feature
> Reporter: Kinga Marton
> Assignee: Kinga Marton
> Priority: Major
> Attachments: YARN-10022.WIP.patch, YARN-10022.WIP2.patch
>
>
> RMWebService should expose a new api which gets a CapacityScheduler
> Configuration as an input, validates it and returns success / failure.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
