(celeborn) branch main updated: [CELEBORN-1516][FOLLOWUP] Support reset method for DynamicConfigServiceFactory

[rexxiong]

This is an automated email from the ASF dual-hosted git repository.

rexxiong pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git


The following commit(s) were added to refs/heads/main by this push:
     new 752a0d945 [CELEBORN-1516][FOLLOWUP] Support reset method for 
DynamicConfigServiceFactory
752a0d945 is described below

commit 752a0d94599dffc965b48801c7cf941a0d7fc4f1
Author: Sanskar Modi <sanskarmod...@gmail.com>
AuthorDate: Wed Oct 30 17:18:31 2024 +0800

    [CELEBORN-1516][FOLLOWUP] Support reset method for 
DynamicConfigServiceFactory
    
    ### What changes were proposed in this pull request?
    - Added a reset method for DynamicConfigServiceFactory
    - Cleaned up QuotaManagerSuite
    
    ### Why are the changes needed?
    Without this change we can not initialize new configService in any other 
tests.
    Ex: test for this PR https://github.com/apache/celeborn/pull/2844 are 
failing because of this issue.
    
    ### Does this PR introduce _any_ user-facing change?
    NA
    
    ### How was this patch tested?
    NA
    
    Closes #2848 from s0nskar/fix_quotatest.
    
    Authored-by: Sanskar Modi <sanskarmod...@gmail.com>
    Signed-off-by: Shuang <lvshuang....@alibaba-inc.com>
---
 .../service/deploy/master/quota/QuotaManager.scala    |  2 +-
 .../deploy/master/quota/QuotaManagerSuite.scala       | 19 +++++--------------
 .../service/config/DynamicConfigServiceFactory.java   | 10 ++++++++++
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git 
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManager.scala
 
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManager.scala
index a5d446368..07906b7d3 100644
--- 
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManager.scala
+++ 
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManager.scala
@@ -24,7 +24,7 @@ import org.apache.celeborn.common.util.Utils
 import org.apache.celeborn.server.common.service.config.ConfigService
 
 class QuotaManager(celebornConf: CelebornConf, configService: ConfigService) 
extends Logging {
-  val DEFAULT_QUOTA = Quota(
+  private val DEFAULT_QUOTA = Quota(
     celebornConf.get(CelebornConf.QUOTA_DISK_BYTES_WRITTEN),
     celebornConf.get(CelebornConf.QUOTA_DISK_FILE_COUNT),
     celebornConf.get(CelebornConf.QUOTA_HDFS_BYTES_WRITTEN),
diff --git 
a/master/src/test/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManagerSuite.scala
 
b/master/src/test/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManagerSuite.scala
index 5e5e93017..90da1cfd1 100644
--- 
a/master/src/test/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManagerSuite.scala
+++ 
b/master/src/test/scala/org/apache/celeborn/service/deploy/master/quota/QuotaManagerSuite.scala
@@ -17,31 +17,22 @@
 
 package org.apache.celeborn.service.deploy.master.quota
 
-import java.io.File
-
 import org.junit.Assert.assertEquals
-import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
-import org.scalatest.funsuite.AnyFunSuite
 
+import org.apache.celeborn.CelebornFunSuite
 import org.apache.celeborn.common.CelebornConf
 import org.apache.celeborn.common.identity.UserIdentifier
-import org.apache.celeborn.common.internal.Logging
 import org.apache.celeborn.common.quota.{Quota, ResourceConsumption}
 import org.apache.celeborn.common.util.Utils
 import 
org.apache.celeborn.server.common.service.config.DynamicConfigServiceFactory
 
-class QuotaManagerSuite extends AnyFunSuite
-  with BeforeAndAfterAll
-  with BeforeAndAfterEach
-  with Logging {
+class QuotaManagerSuite extends CelebornFunSuite {
   protected var quotaManager: QuotaManager = _
 
-  // helper function
-  final protected def getTestResourceFile(file: String): File = {
-    new File(getClass.getClassLoader.getResource(file).getFile)
-  }
-
   override def beforeAll(): Unit = {
+    super.beforeAll()
+    DynamicConfigServiceFactory.reset()
+
     val conf = new CelebornConf()
     conf.set(CelebornConf.DYNAMIC_CONFIG_STORE_BACKEND, "FS")
     conf.set(
diff --git 
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
 
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
index 592a7e17a..57049a994 100644
--- 
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
+++ 
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
@@ -19,6 +19,8 @@ package org.apache.celeborn.server.common.service.config;
 
 import java.util.ServiceLoader;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import org.apache.celeborn.common.CelebornConf;
 import org.apache.celeborn.common.util.Utils;
 
@@ -47,4 +49,12 @@ public class DynamicConfigServiceFactory {
 
     return _INSTANCE;
   }
+
+  @VisibleForTesting
+  public static void reset() {
+    if (_INSTANCE != null) {
+      _INSTANCE.shutdown();
+      _INSTANCE = null;
+    }
+  }
 }