[#2278] feat(coordinator): Introduce AccessSupportRssChecker to reject the un-support application earlier #2277
Conversation
maobaolong
changed the title
maobaolong
force-pushed
the
supportRssChecker
branch
from
5eb22b9 to
3d2c4df
Compare
| @@ -92,7 +93,8 @@ public class CoordinatorConf extends RssBaseConf { | |||
| .asList() | |||
| .defaultValues( | |||
| "org.apache.uniffle.coordinator.access.checker.AccessClusterLoadChecker", | |||
| "org.apache.uniffle.coordinator.access.checker.AccessQuotaChecker") | |||
| "org.apache.uniffle.coordinator.access.checker.AccessQuotaChecker", | |||
| AccessSupportRssChecker.class.getCanonicalName()) | |||
There was a problem hiding this comment.
Could you modify the document, too.
| @@ -92,7 +93,8 @@ public class CoordinatorConf extends RssBaseConf { | |||
| .asList() | |||
| .defaultValues( | |||
| "org.apache.uniffle.coordinator.access.checker.AccessClusterLoadChecker", | |||
| "org.apache.uniffle.coordinator.access.checker.AccessQuotaChecker") | |||
| "org.apache.uniffle.coordinator.access.checker.AccessQuotaChecker", | |||
There was a problem hiding this comment.
Could you use similar way to refactor the code like AccessSupportRssChecker.class.getCanonicalName()?
|
|
||
| @Override | ||
| public AccessCheckResult check(AccessInfo accessInfo) { | ||
| String serializer = accessInfo.getExtraProperties().get("serializer"); |
There was a problem hiding this comment.
This is related to the Spark engine. How to make this more common?
There was a problem hiding this comment.
@jerqi Yeah, so how about add a new config to make this abstract? lets say it unsupportConfigs,
For example unsupportConfigs="serializer:org.apache.hadoop.io.serializer.JavaSerialization"
There was a problem hiding this comment.
We would better make that unsupportConfigs become configs for the shuffle server.
|
@jerqi Thanks for the previous review and sorry for the late update. The comments are all addressed by the new commits, PTAL. |
| | rss.reconfigure.interval.sec | 5 | Reconfigure check interval. | | ||
| | rss.coordinator.rpc.audit.log.enabled | true | When set to true, for auditing purposes, the coordinator will log audit records for every rpc request operation. | | ||
| | rss.coordinator.rpc.audit.log.excludeList | appHeartbeat,heartbeat | Exclude record rpc audit operation list, separated by ','. | | ||
| | rss.coordinator.unsupportedConfigs | serializer:org.apache.hadoop.io.serializer.JavaSerialization | The unsupported config list separated by ',', the key value separated by ':'. If the client configures these properties and they are set to be denied access, the client's access will be rejected. | |
There was a problem hiding this comment.
Empty may be better the default value.
What changes were proposed in this pull request?
Introduce AccessSupportRssChecker to reject the un-support application earlier
Why are the changes needed?
Fix: #2278
Does this PR introduce any user-facing change?
No.
How was this patch tested?
UT