This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new a3e2c91ca [#1178] improvement: set
`rss.coordinator.quota.default.app.num` default -1 to indicate no quota check
(#1186)
a3e2c91ca is described below
commit a3e2c91ca8c917ff73eef96b126aafa7670947f7
Author: xumanbu <[email protected]>
AuthorDate: Tue Sep 5 00:43:33 2023 +0800
[#1178] improvement: set `rss.coordinator.quota.default.app.num` default
-1 to indicate no quota check (#1186)
### What changes were proposed in this pull request?
1、set `rss.coordinator.quota.default.app.num` default value as -1
2、the logic for quota check is the user app quota value < 0, then this
user app quota check would passed
### Why are the changes needed?
Fix: #1178
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
UT
Co-authored-by: jam.xu <[email protected]>
---
.../uniffle/coordinator/CoordinatorConf.java | 10 +++--
.../apache/uniffle/coordinator/QuotaManager.java | 4 +-
.../uniffle/coordinator/QuotaManagerTest.java | 49 ++++++++++++++++++++++
.../src/test/resources/quotaFile.properties | 2 +
4 files changed, 60 insertions(+), 5 deletions(-)
diff --git
a/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
b/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
index 8472ae56d..466dda8b2 100644
---
a/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
+++
b/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
@@ -201,13 +201,17 @@ public class CoordinatorConf extends RssBaseConf {
public static final ConfigOption<Integer> COORDINATOR_QUOTA_DEFAULT_APP_NUM =
ConfigOptions.key("rss.coordinator.quota.default.app.num")
.intType()
- .defaultValue(5)
- .withDescription("Default number of apps at user level");
+ .defaultValue(-1)
+ .withDescription(
+ "Default number of apps at user level, if set value < 0,then
this user app quota "
+ + "check would passed");
public static final ConfigOption<String> COORDINATOR_QUOTA_DEFAULT_PATH =
ConfigOptions.key("rss.coordinator.quota.default.path")
.stringType()
.noDefaultValue()
- .withDescription("A configuration file for the number of apps for a
user-defined user");
+ .withDescription(
+ "A configuration file for the number of apps for a user-defined
user, if set value < 0,then this user app quota "
+ + "check would passed.");
public static final ConfigOption<Long> COORDINATOR_QUOTA_UPDATE_INTERVAL =
ConfigOptions.key("rss.coordinator.quota.update.interval")
.longType()
diff --git
a/coordinator/src/main/java/org/apache/uniffle/coordinator/QuotaManager.java
b/coordinator/src/main/java/org/apache/uniffle/coordinator/QuotaManager.java
index 597932099..d661f90df 100644
--- a/coordinator/src/main/java/org/apache/uniffle/coordinator/QuotaManager.java
+++ b/coordinator/src/main/java/org/apache/uniffle/coordinator/QuotaManager.java
@@ -118,10 +118,10 @@ public class QuotaManager {
public boolean checkQuota(String user, String uuid) {
Map<String, Long> appAndTimes =
currentUserAndApp.computeIfAbsent(user, x ->
JavaUtils.newConcurrentMap());
- Integer defaultAppNum = defaultUserApps.computeIfAbsent(user, x ->
quotaAppNum);
+ Integer userAppQuotaNum = defaultUserApps.computeIfAbsent(user, x ->
quotaAppNum);
synchronized (this) {
int currentAppNum = appAndTimes.size();
- if (currentAppNum >= defaultAppNum) {
+ if (userAppQuotaNum >= 0 && currentAppNum >= userAppQuotaNum) {
return true;
} else {
appAndTimes.put(uuid, System.currentTimeMillis());
diff --git
a/coordinator/src/test/java/org/apache/uniffle/coordinator/QuotaManagerTest.java
b/coordinator/src/test/java/org/apache/uniffle/coordinator/QuotaManagerTest.java
index dd9d682d5..ff378d63d 100644
---
a/coordinator/src/test/java/org/apache/uniffle/coordinator/QuotaManagerTest.java
+++
b/coordinator/src/test/java/org/apache/uniffle/coordinator/QuotaManagerTest.java
@@ -44,6 +44,7 @@ public class QuotaManagerTest {
private final String quotaFile =
Objects.requireNonNull(this.getClass().getClassLoader().getResource(fileName)).getFile();
private static final String fileName = "quotaFile.properties";
+ private static final AtomicInteger uuid = new AtomicInteger();
@BeforeAll
public static void setup() {
@@ -91,6 +92,7 @@ public class QuotaManagerTest {
public void testCheckQuota() throws Exception {
CoordinatorConf conf = new CoordinatorConf();
conf.set(CoordinatorConf.COORDINATOR_QUOTA_DEFAULT_PATH, quotaFile);
+ conf.setInteger(CoordinatorConf.COORDINATOR_QUOTA_DEFAULT_APP_NUM, 5);
final ApplicationManager applicationManager = new ApplicationManager(conf);
final AtomicInteger uuid = new AtomicInteger();
Map<String, Long> uuidAndTime = new ConcurrentHashMap<>();
@@ -123,6 +125,8 @@ public class QuotaManagerTest {
@Test
public void testCheckQuotaMetrics() {
+ CoordinatorMetrics.clear();
+ CoordinatorMetrics.register();
CoordinatorConf conf = new CoordinatorConf();
conf.set(CoordinatorConf.COORDINATOR_QUOTA_DEFAULT_PATH, quotaFile);
conf.setLong(CoordinatorConf.COORDINATOR_APP_EXPIRED, 1500);
@@ -150,6 +154,7 @@ public class QuotaManagerTest {
assertFalse(icCheck2);
// The default number of tasks submitted is 2, and the third will be
rejected
assertTrue(icCheck3);
+ assertFalse(icCheck4);
assertEquals(
applicationManager.getQuotaManager().getCurrentUserAndApp().get("user4").size(),
2);
assertEquals(CoordinatorMetrics.gaugeRunningAppNumToUser.labels("user4").get(),
2);
@@ -164,4 +169,48 @@ public class QuotaManagerTest {
&&
CoordinatorMetrics.gaugeRunningAppNumToUser.labels("user3").get() == 0;
});
}
+
+ @Test
+ public void testCheckQuotaWithDefault() {
+ CoordinatorConf conf = new CoordinatorConf();
+ conf.set(CoordinatorConf.COORDINATOR_QUOTA_DEFAULT_PATH, quotaFile);
+ final ApplicationManager applicationManager = new ApplicationManager(conf);
+ Awaitility.await()
+ .timeout(5, TimeUnit.SECONDS)
+ .until(() -> applicationManager.getDefaultUserApps().size() > 2);
+
+ QuotaManager quotaManager = applicationManager.getQuotaManager();
+ Map<String, Map<String, Long>> currentUserAndApp =
quotaManager.getCurrentUserAndApp();
+
+ currentUserAndApp.computeIfAbsent("user1", x -> mockUUidAppAndTime(30));
+ currentUserAndApp.computeIfAbsent("user2", x -> mockUUidAppAndTime(20));
+ currentUserAndApp.computeIfAbsent("user3", x -> mockUUidAppAndTime(29));
+ currentUserAndApp.computeIfAbsent("disable_quota_user1", x ->
mockUUidAppAndTime(100));
+ currentUserAndApp.computeIfAbsent("blank_user1", x ->
mockUUidAppAndTime(0));
+
+ assertEquals(currentUserAndApp.get("user1").size(), 30);
+ assertEquals(currentUserAndApp.get("user2").size(), 20);
+ assertEquals(currentUserAndApp.get("user3").size(), 29);
+ assertEquals(currentUserAndApp.get("disable_quota_user1").size(), 100);
+ assertEquals(currentUserAndApp.get("blank_user1").size(), 0);
+
+ assertTrue(quotaManager.checkQuota("user1", mockUUidAppId()));
+ assertTrue(quotaManager.checkQuota("user2", mockUUidAppId()));
+ assertFalse(quotaManager.checkQuota("user3", mockUUidAppId()));
+ assertTrue(quotaManager.checkQuota("user3", mockUUidAppId()));
+ assertFalse(quotaManager.checkQuota("disable_quota_user1",
mockUUidAppId()));
+ assertTrue(quotaManager.checkQuota("blank_user1", mockUUidAppId()));
+ }
+
+ private String mockUUidAppId() {
+ return String.valueOf(uuid.incrementAndGet());
+ }
+
+ private Map<String, Long> mockUUidAppAndTime(int mockAppNum) {
+ Map<String, Long> uuidAndTime = new ConcurrentHashMap<>();
+ for (int i = 0; i < mockAppNum; i++) {
+ uuidAndTime.put(mockUUidAppId(), System.currentTimeMillis());
+ }
+ return uuidAndTime;
+ }
}
diff --git a/coordinator/src/test/resources/quotaFile.properties
b/coordinator/src/test/resources/quotaFile.properties
index 7d4110285..a67c07f38 100644
--- a/coordinator/src/test/resources/quotaFile.properties
+++ b/coordinator/src/test/resources/quotaFile.properties
@@ -18,3 +18,5 @@
user1 =10
user2= 20
user3 = 30
+disable_quota_user1=-1
+blank_user1=0
