You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/04/26 08:53:30 UTC
[GitHub] [ozone] JacksonYao287 opened a new pull request, #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
JacksonYao287 opened a new pull request, #3352:
URL: https://github.com/apache/ozone/pull/3352
## What changes were proposed in this pull request?
this is a initial patch for refactoring RM, in this patch
1 Rename the existing RM class to LegacyReplicationManager, and expose the processContainer() method as protected or public. Remove the scheduling from this RM.
2 Create a new replication Manager, called ReplicationManager, then have it scheduled periodically like the current one.
3 When the new RM processes a container, if it is EC, it goes down the new path. If it is not EC, we call LegacyReplicationManager.processContainer(...).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6589
## How was this patch tested?
current UT
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r864420829
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -0,0 +1,448 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container.replication;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.hdds.HddsConfigKeys;
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.PlacementPolicy;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ContainerReplicaCount;
+import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport;
+import org.apache.hadoop.hdds.scm.container.common.helpers.MoveDataNodePair;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceManager;
+import org.apache.hadoop.hdds.scm.node.NodeManager;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.util.ExitUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Clock;
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Replication Manager (RM) is the one which is responsible for making sure
+ * that the containers are properly replicated. Replication Manager deals only
+ * with Quasi Closed / Closed container.
+ */
+public class ReplicationManager implements SCMService {
+
+ public static final Logger LOG =
+ LoggerFactory.getLogger(ReplicationManager.class);
+
+ /**
+ * Reference to the ContainerManager.
+ */
+ private final ContainerManager containerManager;
+
+
+ /**
+ * SCMContext from StorageContainerManager.
+ */
+ private final SCMContext scmContext;
+
+
+ /**
+ * ReplicationManager specific configuration.
+ */
+ private final ReplicationManagerConfiguration rmConf;
+
+ /**
+ * ReplicationMonitor thread is the one which wakes up at configured
+ * interval and processes all the containers.
+ */
+ private Thread replicationMonitor;
+
+ /**
+ * Flag used for checking if the ReplicationMonitor thread is running or
+ * not.
+ */
+ private volatile boolean running;
+
+ /**
+ * Report object that is refreshed each time replication Manager runs.
+ */
+ private ReplicationManagerReport containerReport;
+
+ /**
+ * Replication progress related metrics.
+ */
+ private ReplicationManagerMetrics metrics;
+
+
+ /**
+ * Legacy RM will hopefully be removed after completing refactor
+ * for now, it is used to process non-EC container.
+ */
+ private LegacyReplicationManager legacyReplicationManager;
+
+ /**
+ * SCMService related variables.
+ * After leaving safe mode, replicationMonitor needs to wait for a while
+ * before really take effect.
+ */
+ private final Lock serviceLock = new ReentrantLock();
+ private ServiceStatus serviceStatus = ServiceStatus.PAUSING;
+ private final long waitTimeInMillis;
+ private long lastTimeToBeReadyInMillis = 0;
+ private final Clock clock;
+
+ /**
+ * Constructs ReplicationManager instance with the given configuration.
+ *
+ * @param conf OzoneConfiguration
+ * @param containerManager ContainerManager
+ * @param containerPlacement PlacementPolicy
+ * @param eventPublisher EventPublisher
+ */
+ @SuppressWarnings("parameternumber")
+ public ReplicationManager(final ConfigurationSource conf,
+ final ContainerManager containerManager,
+ final PlacementPolicy containerPlacement,
+ final EventPublisher eventPublisher,
+ final SCMContext scmContext,
+ final SCMServiceManager serviceManager,
+ final NodeManager nodeManager,
+ final Clock clock,
+ final SCMHAManager scmhaManager,
+ final Table<ContainerID, MoveDataNodePair> moveTable)
+ throws IOException {
+ this.containerManager = containerManager;
+ this.scmContext = scmContext;
+ this.rmConf = conf.getObject(ReplicationManagerConfiguration.class);
+ this.running = false;
+ this.clock = clock;
+ this.containerReport = new ReplicationManagerReport();
+ this.metrics = null;
+ this.waitTimeInMillis = conf.getTimeDuration(
+ HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT,
+ HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT_DEFAULT,
+ TimeUnit.MILLISECONDS);
+ this.legacyReplicationManager = new LegacyReplicationManager(
+ conf, containerManager, containerPlacement, eventPublisher,
+ scmContext, nodeManager, scmhaManager, clock, moveTable);
+
+ // register ReplicationManager to SCMServiceManager.
+ serviceManager.register(this);
+
+ // start ReplicationManager.
+ start();
+ }
+
+ /**
+ * Starts Replication Monitor thread.
+ */
+ @Override
+ public synchronized void start() {
+ if (!isRunning()) {
+ LOG.info("Starting Replication Monitor Thread.");
+ running = true;
+ metrics = ReplicationManagerMetrics.create(this);
+ legacyReplicationManager.setMetrics(metrics);
+ replicationMonitor = new Thread(this::run);
+ replicationMonitor.setName("ReplicationMonitor");
+ replicationMonitor.setDaemon(true);
+ replicationMonitor.start();
+ } else {
+ LOG.info("Replication Monitor Thread is already running.");
+ }
+ }
+
+ /**
+ * Returns true if the Replication Monitor Thread is running.
+ *
+ * @return true if running, false otherwise
+ */
+ public boolean isRunning() {
+ if (!running) {
+ synchronized (this) {
+ return replicationMonitor != null
+ && replicationMonitor.isAlive();
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Stops Replication Monitor thread.
+ */
+ public synchronized void stop() {
+ if (running) {
+ LOG.info("Stopping Replication Monitor Thread.");
+ running = false;
+ legacyReplicationManager.clearInflightActions();
+ metrics.unRegister();
+ notifyAll();
Review Comment:
this is a good point , i have updated this patch , please take a look!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1110489222
i have force push this patch with a series of commits, PTAL! @kerneltime @sodonnel @kaijchen
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1118086028
thanks @sodonnel for the review and @umamaheswararao for the merge
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r862336284
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/InflightAction.java:
##########
@@ -17,12 +17,29 @@
*/
package org.apache.hadoop.hdds.scm.container.replication;
+import com.google.common.annotations.VisibleForTesting;
Review Comment:
yea , i notice this too.
as per our discussion , we will extract InFlightReplicaion and InFlightDeletion into a standalone class with associated test class, so i extract inflightAction a standalone class here.
i just delete ReplicationActivityStatusMXBean.java in the commit [b398eef](https://github.com/apache/ozone/pull/3352/commits/b398eef500a02867c42dbe8d613a37444137bd09)
and add InflightAction.java in another commit [f0c5034](https://github.com/apache/ozone/pull/3352/commits/f0c50340cddee2e476a6d3c918e4d86001529ff5), the options are clear in these two commits. but in the summary of Filie changed, it show `ReplicationActivityStatusMXBean.java` is moved to `InflightAction.java`, i am not sure what happens here, do you have any idea about this?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r863652873
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -0,0 +1,448 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container.replication;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.hdds.HddsConfigKeys;
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.PlacementPolicy;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ContainerReplicaCount;
+import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport;
+import org.apache.hadoop.hdds.scm.container.common.helpers.MoveDataNodePair;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceManager;
+import org.apache.hadoop.hdds.scm.node.NodeManager;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.util.ExitUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Clock;
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Replication Manager (RM) is the one which is responsible for making sure
+ * that the containers are properly replicated. Replication Manager deals only
+ * with Quasi Closed / Closed container.
+ */
+public class ReplicationManager implements SCMService {
+
+ public static final Logger LOG =
+ LoggerFactory.getLogger(ReplicationManager.class);
+
+ /**
+ * Reference to the ContainerManager.
+ */
+ private final ContainerManager containerManager;
+
+
+ /**
+ * SCMContext from StorageContainerManager.
+ */
+ private final SCMContext scmContext;
+
+
+ /**
+ * ReplicationManager specific configuration.
+ */
+ private final ReplicationManagerConfiguration rmConf;
+
+ /**
+ * ReplicationMonitor thread is the one which wakes up at configured
+ * interval and processes all the containers.
+ */
+ private Thread replicationMonitor;
+
+ /**
+ * Flag used for checking if the ReplicationMonitor thread is running or
+ * not.
+ */
+ private volatile boolean running;
+
+ /**
+ * Report object that is refreshed each time replication Manager runs.
+ */
+ private ReplicationManagerReport containerReport;
+
+ /**
+ * Replication progress related metrics.
+ */
+ private ReplicationManagerMetrics metrics;
+
+
+ /**
+ * Legacy RM will hopefully be removed after completing refactor
+ * for now, it is used to process non-EC container.
+ */
+ private LegacyReplicationManager legacyReplicationManager;
+
+ /**
+ * SCMService related variables.
+ * After leaving safe mode, replicationMonitor needs to wait for a while
+ * before really take effect.
+ */
+ private final Lock serviceLock = new ReentrantLock();
+ private ServiceStatus serviceStatus = ServiceStatus.PAUSING;
+ private final long waitTimeInMillis;
+ private long lastTimeToBeReadyInMillis = 0;
+ private final Clock clock;
+
+ /**
+ * Constructs ReplicationManager instance with the given configuration.
+ *
+ * @param conf OzoneConfiguration
+ * @param containerManager ContainerManager
+ * @param containerPlacement PlacementPolicy
+ * @param eventPublisher EventPublisher
+ */
+ @SuppressWarnings("parameternumber")
+ public ReplicationManager(final ConfigurationSource conf,
+ final ContainerManager containerManager,
+ final PlacementPolicy containerPlacement,
+ final EventPublisher eventPublisher,
+ final SCMContext scmContext,
+ final SCMServiceManager serviceManager,
+ final NodeManager nodeManager,
+ final Clock clock,
+ final SCMHAManager scmhaManager,
+ final Table<ContainerID, MoveDataNodePair> moveTable)
+ throws IOException {
+ this.containerManager = containerManager;
+ this.scmContext = scmContext;
+ this.rmConf = conf.getObject(ReplicationManagerConfiguration.class);
+ this.running = false;
+ this.clock = clock;
+ this.containerReport = new ReplicationManagerReport();
+ this.metrics = null;
+ this.waitTimeInMillis = conf.getTimeDuration(
+ HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT,
+ HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT_DEFAULT,
+ TimeUnit.MILLISECONDS);
+ this.legacyReplicationManager = new LegacyReplicationManager(
+ conf, containerManager, containerPlacement, eventPublisher,
+ scmContext, nodeManager, scmhaManager, clock, moveTable);
+
+ // register ReplicationManager to SCMServiceManager.
+ serviceManager.register(this);
+
+ // start ReplicationManager.
+ start();
+ }
+
+ /**
+ * Starts Replication Monitor thread.
+ */
+ @Override
+ public synchronized void start() {
+ if (!isRunning()) {
+ LOG.info("Starting Replication Monitor Thread.");
+ running = true;
+ metrics = ReplicationManagerMetrics.create(this);
+ legacyReplicationManager.setMetrics(metrics);
+ replicationMonitor = new Thread(this::run);
+ replicationMonitor.setName("ReplicationMonitor");
+ replicationMonitor.setDaemon(true);
+ replicationMonitor.start();
+ } else {
+ LOG.info("Replication Monitor Thread is already running.");
+ }
+ }
+
+ /**
+ * Returns true if the Replication Monitor Thread is running.
+ *
+ * @return true if running, false otherwise
+ */
+ public boolean isRunning() {
+ if (!running) {
+ synchronized (this) {
+ return replicationMonitor != null
+ && replicationMonitor.isAlive();
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Stops Replication Monitor thread.
+ */
+ public synchronized void stop() {
+ if (running) {
+ LOG.info("Stopping Replication Monitor Thread.");
+ running = false;
+ legacyReplicationManager.clearInflightActions();
+ metrics.unRegister();
+ notifyAll();
+ } else {
+ LOG.info("Replication Monitor Thread is not running.");
+ }
+ }
+
+ /**
+ * Process all the containers now, and wait for the processing to complete.
+ * This in intended to be used in tests.
+ */
+ public synchronized void processAll() {
+ if (!shouldRun()) {
+ LOG.info("Replication Manager is not ready to run until {}ms after " +
+ "safemode exit", waitTimeInMillis);
+ return;
+ }
+ final long start = clock.millis();
+ final List<ContainerInfo> containers =
+ containerManager.getContainers();
+ ReplicationManagerReport report = new ReplicationManagerReport();
+ for (ContainerInfo c : containers) {
+ if (!shouldRun()) {
+ break;
+ }
+ switch (c.getReplicationType()) {
+ case EC:
+ break;
+ default:
+ legacyReplicationManager.processContainer(c, report);
+ }
+ }
+ report.setComplete();
+ this.containerReport = report;
+ LOG.info("Replication Monitor Thread took {} milliseconds for" +
+ " processing {} containers.", clock.millis() - start,
+ containers.size());
+ }
+
+ public ReplicationManagerReport getContainerReport() {
+ return containerReport;
+ }
+
+ /**
+ * ReplicationMonitor thread runnable. This wakes up at configured
+ * interval and processes all the containers in the system.
+ */
+ private synchronized void run() {
+ try {
+ while (running) {
+ processAll();
+ wait(rmConf.getInterval());
+ }
+ } catch (Throwable t) {
+ // When we get runtime exception, we should terminate SCM.
+ LOG.error("Exception in Replication Monitor Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ ExitUtil.terminate(1, t);
Review Comment:
If we change the`stop()` method to use interrupt instead of wait, we probably need to ensure the ExitUtil is only called if the exception is not an `InterruptedException`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r862334706
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/LegacyReplicationManager.java:
##########
@@ -781,14 +645,10 @@ public CompletableFuture<MoveResult> move(ContainerID cid,
* @param cid Container to move
* @param mp MoveDataNodePair which contains source and target datanodes
*/
- public CompletableFuture<MoveResult> move(ContainerID cid,
+ private CompletableFuture<MoveResult> move(ContainerID cid,
MoveDataNodePair mp)
throws ContainerNotFoundException, NodeNotFoundException {
CompletableFuture<MoveResult> ret = new CompletableFuture<>();
- if (!isRunning()) {
Review Comment:
since the legacy RM does not have any scheduling logic, it is just a container processor now and does not know whether the new RM is running. the check is needed, and has been moved to the new RM , please see
[the check in new RM](https://github.com/apache/ozone/pull/3352/files/dcb2ce3c53bc01ed30aab1b27e494cddfda5a44a#diff-8be983672d581b5fdec0b1a7ce4fe4f0ee29a25684f2af3680604d51d3ebbb52R412)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1110252268
> This is a very big patch that moves the ReplicationManager class to LegacyReplicationManager, but there is also a lot of change inside the moved class. It would be better if this PR was a series of commits rather than one large one (this is true for any large PR), as it makes it much easier to review. Eg 1 commit for moving ReplicationManager -> LegacyReplicationManager (probably a refactor in Intellij), then a commit for each logical set of changes. It doesn't need to be a series of Jiras, just a series of commits in the PR that are not squashed until the end.
>
> Could you re-post this as a series of commits, or do you not have the changes like that on your local branch?
+1 Refactors without change in logic is best done as a set followed by any other code changes.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r861693439
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/InflightAction.java:
##########
@@ -17,12 +17,29 @@
*/
package org.apache.hadoop.hdds.scm.container.replication;
+import com.google.common.annotations.VisibleForTesting;
Review Comment:
This file is showing as a rename from `ReplicationActivityStatusMXBean.java → ...container/replication/InflightAction.java`, which is a bit strange. Should this InflightAction class not be a new file? Something strange has happened as you have moved things around I think.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1109764294
@JacksonYao287 thanks for the patch. Seems the CI failure is related to the change, please check.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] umamaheswararao commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1117562287
Great work @JacksonYao287 and thanks for @sodonnel for thorough reviews !!!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
sodonnel commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1110198778
This is a very big patch that moves the ReplicationManager class to LegacyReplicationManager, but there is also a lot of change inside the moved class. It would be better if this PR was a series of commits rather than one large one (this is true for any large PR), as it makes it much easier to review. Eg 1 commit for moving ReplicationManager -> LegacyReplicationManager (probably a refactor in Intellij), then a commit for each logical set of changes. It doesn't need to be a series of Jiras, just a series of commits in the PR that are not squashed until the end.
Could you re-post this as a series of commits, or do you not have the changes like that on your local branch?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r863652263
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -0,0 +1,448 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container.replication;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.hdds.HddsConfigKeys;
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.PlacementPolicy;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ContainerReplicaCount;
+import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport;
+import org.apache.hadoop.hdds.scm.container.common.helpers.MoveDataNodePair;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceManager;
+import org.apache.hadoop.hdds.scm.node.NodeManager;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.apache.hadoop.hdds.server.events.EventPublisher;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.util.ExitUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Clock;
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Replication Manager (RM) is the one which is responsible for making sure
+ * that the containers are properly replicated. Replication Manager deals only
+ * with Quasi Closed / Closed container.
+ */
+public class ReplicationManager implements SCMService {
+
+ public static final Logger LOG =
+ LoggerFactory.getLogger(ReplicationManager.class);
+
+ /**
+ * Reference to the ContainerManager.
+ */
+ private final ContainerManager containerManager;
+
+
+ /**
+ * SCMContext from StorageContainerManager.
+ */
+ private final SCMContext scmContext;
+
+
+ /**
+ * ReplicationManager specific configuration.
+ */
+ private final ReplicationManagerConfiguration rmConf;
+
+ /**
+ * ReplicationMonitor thread is the one which wakes up at configured
+ * interval and processes all the containers.
+ */
+ private Thread replicationMonitor;
+
+ /**
+ * Flag used for checking if the ReplicationMonitor thread is running or
+ * not.
+ */
+ private volatile boolean running;
+
+ /**
+ * Report object that is refreshed each time replication Manager runs.
+ */
+ private ReplicationManagerReport containerReport;
+
+ /**
+ * Replication progress related metrics.
+ */
+ private ReplicationManagerMetrics metrics;
+
+
+ /**
+ * Legacy RM will hopefully be removed after completing refactor
+ * for now, it is used to process non-EC container.
+ */
+ private LegacyReplicationManager legacyReplicationManager;
+
+ /**
+ * SCMService related variables.
+ * After leaving safe mode, replicationMonitor needs to wait for a while
+ * before really take effect.
+ */
+ private final Lock serviceLock = new ReentrantLock();
+ private ServiceStatus serviceStatus = ServiceStatus.PAUSING;
+ private final long waitTimeInMillis;
+ private long lastTimeToBeReadyInMillis = 0;
+ private final Clock clock;
+
+ /**
+ * Constructs ReplicationManager instance with the given configuration.
+ *
+ * @param conf OzoneConfiguration
+ * @param containerManager ContainerManager
+ * @param containerPlacement PlacementPolicy
+ * @param eventPublisher EventPublisher
+ */
+ @SuppressWarnings("parameternumber")
+ public ReplicationManager(final ConfigurationSource conf,
+ final ContainerManager containerManager,
+ final PlacementPolicy containerPlacement,
+ final EventPublisher eventPublisher,
+ final SCMContext scmContext,
+ final SCMServiceManager serviceManager,
+ final NodeManager nodeManager,
+ final Clock clock,
+ final SCMHAManager scmhaManager,
+ final Table<ContainerID, MoveDataNodePair> moveTable)
+ throws IOException {
+ this.containerManager = containerManager;
+ this.scmContext = scmContext;
+ this.rmConf = conf.getObject(ReplicationManagerConfiguration.class);
+ this.running = false;
+ this.clock = clock;
+ this.containerReport = new ReplicationManagerReport();
+ this.metrics = null;
+ this.waitTimeInMillis = conf.getTimeDuration(
+ HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT,
+ HddsConfigKeys.HDDS_SCM_WAIT_TIME_AFTER_SAFE_MODE_EXIT_DEFAULT,
+ TimeUnit.MILLISECONDS);
+ this.legacyReplicationManager = new LegacyReplicationManager(
+ conf, containerManager, containerPlacement, eventPublisher,
+ scmContext, nodeManager, scmhaManager, clock, moveTable);
+
+ // register ReplicationManager to SCMServiceManager.
+ serviceManager.register(this);
+
+ // start ReplicationManager.
+ start();
+ }
+
+ /**
+ * Starts Replication Monitor thread.
+ */
+ @Override
+ public synchronized void start() {
+ if (!isRunning()) {
+ LOG.info("Starting Replication Monitor Thread.");
+ running = true;
+ metrics = ReplicationManagerMetrics.create(this);
+ legacyReplicationManager.setMetrics(metrics);
+ replicationMonitor = new Thread(this::run);
+ replicationMonitor.setName("ReplicationMonitor");
+ replicationMonitor.setDaemon(true);
+ replicationMonitor.start();
+ } else {
+ LOG.info("Replication Monitor Thread is already running.");
+ }
+ }
+
+ /**
+ * Returns true if the Replication Monitor Thread is running.
+ *
+ * @return true if running, false otherwise
+ */
+ public boolean isRunning() {
+ if (!running) {
+ synchronized (this) {
+ return replicationMonitor != null
+ && replicationMonitor.isAlive();
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Stops Replication Monitor thread.
+ */
+ public synchronized void stop() {
+ if (running) {
+ LOG.info("Stopping Replication Monitor Thread.");
+ running = false;
+ legacyReplicationManager.clearInflightActions();
+ metrics.unRegister();
+ notifyAll();
Review Comment:
This `notifyAll()` should probably be `replicationMonitor.interrupt()`. I think it works fine like this, because of the synchronized, but I recently came across the `stop()` method in another service not stopping the service quickly. as the notify was issued when the thread was running and not waiting.
If we call interrupt, then it will interrupt the thread when it is sleeping, or it will set the interrupt flag and it will be interrupted when it stops running and tries to `wait()`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1117303926
@sodonnel thanks for the review and comments.
seems there are some flaky test case failed. pending for a clean CI.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r861691733
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/LegacyReplicationManager.java:
##########
@@ -781,14 +645,10 @@ public CompletableFuture<MoveResult> move(ContainerID cid,
* @param cid Container to move
* @param mp MoveDataNodePair which contains source and target datanodes
*/
- public CompletableFuture<MoveResult> move(ContainerID cid,
+ private CompletableFuture<MoveResult> move(ContainerID cid,
MoveDataNodePair mp)
throws ContainerNotFoundException, NodeNotFoundException {
CompletableFuture<MoveResult> ret = new CompletableFuture<>();
- if (!isRunning()) {
Review Comment:
Is the balancer going to keep working OK if this RM thread is not running any more? From this check, it must have needed / wanted it to be running before?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1109604705
@sodonnel PTAL, thanks!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r864609159
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -263,12 +263,13 @@ private synchronized void run() {
wait(rmConf.getInterval());
}
} catch (Throwable t) {
- // When we get runtime exception, we should terminate SCM.
- LOG.error("Exception in Replication Monitor Thread.", t);
if (t instanceof InterruptedException) {
- Thread.currentThread().interrupt();
Review Comment:
Even though we know the thread is exiting, I think you are still supposed to keep the line `Thread.currentThread().interrupt();` incase something else needs it before the thread exits.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] umamaheswararao merged pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3352:
URL: https://github.com/apache/ozone/pull/3352
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1110449124
thanks @sodonnel @kerneltime @kaijchen for the review!
> Could you re-post this as a series of commits, or do you not have the changes like that on your local branch?
i do have those commits in my local branch earlier, but I accidentally squash them before creating this patch, so it is a little hard to find them back now.
though this is a big patch , the logic in this patch seems clear and not complex to review. the steps in this patch are as follows:
1 after check again , i remove "ReplicationActivityStatus.java" and "ReplicationActivityStatusMXBean.java" from this project, since they seem not used anywhere.
2 move "ReplicationManager.java" to the directory "hdds.scm/container/replication", where "ReplicationActivityStatus.java" and "ReplicationActivityStatusMXBean.java" existed, so all the replication manager related work will be in this directory.
3 copy "ReplicationManager.java" to "LegacyReplicationManager.java", so we have to RM class now.
4 for "LegacyReplicationManager.java", i do not let it implement the "SCMService" interface, and remove all the scheduling logic from it , so now, it is only used to process non-EC container.
5 for "ReplicationManager.java" , i only keep the logic of scheduling and inject the instance of "LegacyReplicationManager " into it to process non-EC container.
6 for those public functions in ReplicatioManager class, which are called from other classes, i just keep them in the new ReplicatioManager class , and delegate the calls to LegacyReplicationManager class. these functions can be refactored or rewrite in a separate patch in the future.
7 fix all the influenced test case.
so , when reviewing , please pay more attention to "LegacyReplicationManager.java" and "ReplicationManager.java" , other parts of this patch almost have nothing to do with logic, they are all adaptive work.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3352:
URL: https://github.com/apache/ozone/pull/3352#issuecomment-1110454451
> i do have those commits in my local branch earlier, but I accidentally squash them before creating this patch, so it is a little hard to find them back now.
You can try `git reflog`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on a diff in pull request #3352: HDDS-6589. Add a new replication manager and change the existing one to legacy
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3352:
URL: https://github.com/apache/ozone/pull/3352#discussion_r862336284
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/InflightAction.java:
##########
@@ -17,12 +17,29 @@
*/
package org.apache.hadoop.hdds.scm.container.replication;
+import com.google.common.annotations.VisibleForTesting;
Review Comment:
yea , i notice this too.
as per our discussion , we will extract InFlightReplicaion and InFlightDeletion into a standalone class with associated test class, so i extract inflightAction a standalone class here for later refactor.
i just delete ReplicationActivityStatusMXBean.java in the commit [b398eef](https://github.com/apache/ozone/pull/3352/commits/b398eef500a02867c42dbe8d613a37444137bd09)
and add InflightAction.java in another commit [f0c5034](https://github.com/apache/ozone/pull/3352/commits/f0c50340cddee2e476a6d3c918e4d86001529ff5), the options are clear in these two commits. but in the summary of Filie changed, it show `ReplicationActivityStatusMXBean.java` is moved to `InflightAction.java`, i am not sure what happens here, do you have any idea about this?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org