goiri commented on code in PR #5967:
URL: https://github.com/apache/hadoop/pull/5967#discussion_r1306269910
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/utils/FederationStateStoreFacade.java:
##########
@@ -195,11 +194,23 @@ public static RetryPolicy createRetryPolicy(Configuration
conf) {
/**
* Returns the singleton instance of the FederationStateStoreFacade object.
+ * @param conf Configuration.
*
* @return the singleton {@link FederationStateStoreFacade} instance
*/
- public static FederationStateStoreFacade getInstance() {
- return FACADE;
+ public static FederationStateStoreFacade getInstance(Configuration conf) {
+ if(facade == null) {
Review Comment:
Can we exit early?
```
if (facade != null) {
return facade;
}
```
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/utils/FederationPoliciesTestUtil.java:
##########
@@ -244,7 +245,7 @@ public static FederationStateStoreFacade initFacade(
List<SubClusterInfo> subClusterInfos, SubClusterPolicyConfiguration
policyConfiguration) throws YarnException {
FederationStateStoreFacade goodFacade = FederationStateStoreFacade
- .getInstance();
+ .getInstance(new Configuration());
Review Comment:
Can we have an empty getInstance() that does the new Configuration?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/rmadmin/TestFederationRMAdminInterceptor.java:
##########
@@ -120,7 +120,7 @@ public void setUp() {
// Initialize facade & stateSore
stateStore = new MemoryFederationStateStore();
stateStore.init(this.getConf());
- facade = FederationStateStoreFacade.getInstance();
+ facade = FederationStateStoreFacade.getInstance(this.getConf());
Review Comment:
Let's be consistent with using this or not.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/utils/FederationStateStoreFacade.java:
##########
@@ -195,11 +194,23 @@ public static RetryPolicy createRetryPolicy(Configuration
conf) {
/**
* Returns the singleton instance of the FederationStateStoreFacade object.
+ * @param conf Configuration.
*
* @return the singleton {@link FederationStateStoreFacade} instance
*/
- public static FederationStateStoreFacade getInstance() {
- return FACADE;
+ public static FederationStateStoreFacade getInstance(Configuration conf) {
+ if(facade == null) {
Review Comment:
Fix spaces too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
