-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-23996 Add metric for the split WAL procedure #8515
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,8 +58,11 @@ | |
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * ZooKeeper based implementation of {@link SplitLogManagerCoordination} | ||
| * ZooKeeper based implementation of {@link SplitLogManagerCoordination}. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we make the replacement a Javadoc link? SplitWALManager is in org.apache.hadoop.hbase.master, so use {@link org.apache.hadoop.hbase.master.SplitWALManager}. This makes the migration target navigable in generated API docs. |
||
| * @deprecated since 2.4.0 and in 3.0.0, to be removed in 4.0.0, replaced by procedure-based | ||
| * distributed WAL splitter; see SplitWALManager. | ||
| */ | ||
| @Deprecated | ||
| @InterfaceAudience.Private | ||
| public class ZKSplitLogManagerCoordination extends ZKListener | ||
| implements SplitLogManagerCoordination { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,9 +57,11 @@ | |
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * ZooKeeper based implementation of {@link SplitLogWorkerCoordination} It listen for changes in | ||
| * ZooKeeper and | ||
| * ZooKeeper based implementation of {@link SplitLogWorkerCoordination}. | ||
| * @deprecated since 2.4.0 and in 3.0.0, to be removed in 4.0.0, replaced by procedure-based | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same javadoc comment applies here |
||
| * distributed WAL splitter; see SplitWALManager. | ||
| */ | ||
| @Deprecated | ||
| @InterfaceAudience.Private | ||
| public class ZkSplitLogWorkerCoordination extends ZKListener implements SplitLogWorkerCoordination { | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ | |
| import org.apache.hadoop.fs.FileStatus; | ||
| import org.apache.hadoop.fs.FileSystem; | ||
| import org.apache.hadoop.fs.Path; | ||
| import org.apache.hadoop.hbase.CompatibilityFactory; | ||
| import org.apache.hadoop.hbase.HBaseTestingUtil; | ||
| import org.apache.hadoop.hbase.HConstants; | ||
| import org.apache.hadoop.hbase.ServerName; | ||
|
|
@@ -45,6 +46,7 @@ | |
| import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; | ||
| import org.apache.hadoop.hbase.procedure2.ProcedureYieldException; | ||
| import org.apache.hadoop.hbase.procedure2.StateMachineProcedure; | ||
| import org.apache.hadoop.hbase.test.MetricsAssertHelper; | ||
| import org.apache.hadoop.hbase.testclassification.LargeTests; | ||
| import org.apache.hadoop.hbase.testclassification.MasterTests; | ||
| import org.apache.hadoop.hbase.util.Bytes; | ||
|
|
@@ -69,6 +71,8 @@ | |
| public class TestSplitWALManager { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(TestSplitWALManager.class); | ||
| private static final MetricsAssertHelper METRICS_HELPER = | ||
| CompatibilityFactory.getInstance(MetricsAssertHelper.class); | ||
| private static HBaseTestingUtil TEST_UTIL; | ||
| private HMaster master; | ||
| private SplitWALManager splitWALManager; | ||
|
|
@@ -231,6 +235,9 @@ private void splitLogsTestHelper(HBaseTestingUtil testUtil) throws Exception { | |
| // load table | ||
| testUtil.loadTable(testUtil.getConnection().getTable(TABLE_NAME), FAMILY); | ||
| ProcedureExecutor<MasterProcedureEnv> masterPE = hmaster.getMasterProcedureExecutor(); | ||
| MetricsMasterSource masterSource = hmaster.getMasterMetrics().getMetricsSource(); | ||
| long splitWALSubmittedBase = METRICS_HELPER | ||
| .getCounter(MetricsMasterSource.SPLIT_WAL_METRIC_PREFIX + "SubmittedCount", masterSource); | ||
| ServerName metaServer = testUtil.getHBaseCluster().getServerHoldingMeta(); | ||
| ServerName testServer = testUtil.getHBaseCluster().getRegionServerThreads().stream() | ||
| .map(rs -> rs.getRegionServer().getServerName()).filter(rs -> rs != metaServer).findAny() | ||
|
|
@@ -239,6 +246,9 @@ private void splitLogsTestHelper(HBaseTestingUtil testUtil) throws Exception { | |
| assertEquals(1, procedures.size()); | ||
| ProcedureTestingUtility.submitAndWait(masterPE, procedures.get(0)); | ||
| assertEquals(0, splitWALManager.getWALsToSplit(testServer, false).size()); | ||
| // The SplitWALProcedure above should have been reported to the split WAL metric. | ||
| METRICS_HELPER.assertCounter(MetricsMasterSource.SPLIT_WAL_METRIC_PREFIX + "SubmittedCount", | ||
| splitWALSubmittedBase + 1, masterSource); | ||
|
|
||
| // Validate the old WAL file archive dir | ||
| Path walRootDir = hmaster.getMasterFileSystem().getWALRootDir(); | ||
|
|
@@ -251,6 +261,9 @@ private void splitLogsTestHelper(HBaseTestingUtil testUtil) throws Exception { | |
| ProcedureTestingUtility.submitAndWait(masterPE, procedures.get(0)); | ||
| assertEquals(0, splitWALManager.getWALsToSplit(metaServer, true).size()); | ||
| assertEquals(1, splitWALManager.getWALsToSplit(metaServer, false).size()); | ||
| // The meta SplitWALProcedure should also have been counted by the split WAL metric. | ||
| METRICS_HELPER.assertCounter(MetricsMasterSource.SPLIT_WAL_METRIC_PREFIX + "SubmittedCount", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This validates I do not see a deterministic terminal-failure path for SplitWALProcedure, so I would not require a FailedCount integration test in this PR. The generic procedure metrics test covers failure accounting; an assertion that FailedCount remains unchanged on this successful/retryable path would be a useful optional addition. |
||
| splitWALSubmittedBase + 2, masterSource); | ||
| // There should be archiveFileCount + 1 WALs after SplitWALProcedure finish | ||
| assertEquals(archiveFileCount + 1, walFS.listStatus(walArchivePath).length, | ||
| "Splitted WAL files should be archived"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same javadoc comment applies here