Skip to content

HDDS-11797. Remove cyclic dependency between SCMSafeModeManager and SafeModeRules #8782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nandakumar131 merged 1 commit into from
Jul 14, 2025
Merged

HDDS-11797. Remove cyclic dependency between SCMSafeModeManager and SafeModeRules #8782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SCMSafeModeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ public SCMSafeModeManager(final ConfigurationSource conf,
this.scmContext = scmContext;
this.safeModeMetrics = SafeModeMetrics.create();

// TODO: Remove the cyclic ("this") dependency (HDDS-11797)
SafeModeRuleFactory.initialize(conf, scmContext, eventQueue, this,
SafeModeRuleFactory.initialize(conf, scmContext, eventQueue,
pipelineManager, containerManager, nodeManager);
SafeModeRuleFactory factory = SafeModeRuleFactory.getInstance();
factory.addSafeModeManager(this);
factory.getSafeModeRules().forEach(rule -> exitRules.put(rule.getRuleName(), rule));
factory.getPreCheckRules().forEach(rule -> preCheckRules.add(rule.getRuleName()));

Expand Down
14 changes: 6 additions & 8 deletions ...dds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SafeModeRuleFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ public final class SafeModeRuleFactory {
private final SCMContext scmContext;
private final EventQueue eventQueue;

// TODO: Remove dependency on safeModeManager (HDDS-11797)
private final SCMSafeModeManager safeModeManager;
private final PipelineManager pipelineManager;
private final ContainerManager containerManager;
private final NodeManager nodeManager;
Expand All @@ -49,23 +47,24 @@ public final class SafeModeRuleFactory {
private SafeModeRuleFactory(final ConfigurationSource config,
final SCMContext scmContext,
final EventQueue eventQueue,
final SCMSafeModeManager safeModeManager,
final PipelineManager pipelineManager,
final ContainerManager containerManager,
final NodeManager nodeManager) {
this.config = config;
this.scmContext = scmContext;
this.eventQueue = eventQueue;
this.safeModeManager = safeModeManager;
this.pipelineManager = pipelineManager;
this.containerManager = containerManager;
this.nodeManager = nodeManager;
this.safeModeRules = new ArrayList<>();
this.preCheckRules = new ArrayList<>();
loadRules();
}

private void loadRules() {
public void addSafeModeManager(SCMSafeModeManager safeModeManager) {
loadRules(safeModeManager);
}

private void loadRules(SCMSafeModeManager safeModeManager) {
// TODO: Use annotation to load the rules. (HDDS-11730)
SafeModeExitRule<?> ratisContainerRule = new RatisContainerSafeModeRule(eventQueue,
config, containerManager, safeModeManager);
Expand Down Expand Up @@ -102,12 +101,11 @@ public static synchronized void initialize(
final ConfigurationSource config,
final SCMContext scmContext,
final EventQueue eventQueue,
final SCMSafeModeManager safeModeManager,
final PipelineManager pipelineManager,
final ContainerManager containerManager,
final NodeManager nodeManager) {
instance = new SafeModeRuleFactory(config, scmContext, eventQueue,
safeModeManager, pipelineManager, containerManager, nodeManager);
pipelineManager, containerManager, nodeManager);
}

public List<SafeModeExitRule<?>> getSafeModeRules() {
Expand Down
11 changes: 7 additions & 4 deletions ...server-scm/src/test/java/org/apache/hadoop/hdds/scm/safemode/TestSafeModeRuleFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ public void testIllegalState() {

@Test
public void testLoadedSafeModeRules() {
initializeSafeModeRuleFactory();
SCMSafeModeManager safeModeManager = initializeSafeModeRuleFactory();
final SafeModeRuleFactory factory = SafeModeRuleFactory.getInstance();
factory.addSafeModeManager(safeModeManager);

// Currently we assert the total count against hardcoded value
// as the rules are hardcoded in SafeModeRuleFactory.
Expand All @@ -62,8 +63,9 @@ public void testLoadedSafeModeRules() {

@Test
public void testLoadedPreCheckRules() {
initializeSafeModeRuleFactory();
SCMSafeModeManager safeModeManager = initializeSafeModeRuleFactory();
final SafeModeRuleFactory factory = SafeModeRuleFactory.getInstance();
factory.addSafeModeManager(safeModeManager);

// Currently we assert the total count against hardcoded value
// as the rules are hardcoded in SafeModeRuleFactory.
Expand All @@ -74,13 +76,14 @@ public void testLoadedPreCheckRules() {

}

private void initializeSafeModeRuleFactory() {
private SCMSafeModeManager initializeSafeModeRuleFactory() {
final SCMSafeModeManager safeModeManager = mock(SCMSafeModeManager.class);
when(safeModeManager.getSafeModeMetrics()).thenReturn(mock(SafeModeMetrics.class));
SafeModeRuleFactory.initialize(new OzoneConfiguration(),
SCMContext.emptyContext(), new EventQueue(), safeModeManager, mock(
SCMContext.emptyContext(), new EventQueue(), mock(
PipelineManager.class),
mock(ContainerManager.class), mock(NodeManager.class));
return safeModeManager;
}

}