devmadhuu commented on code in PR #10384:
URL: https://github.com/apache/ozone/pull/10384#discussion_r3386615911
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconStorageContainerSyncHelper.java:
##########
@@ -47,16 +54,51 @@ class TestReconStorageContainerSyncHelper {
private final ReconContainerManager mockContainerManager =
mock(ReconContainerManager.class);
- private final ReconStorageContainerSyncHelper syncHelper;
+ private ReconScmContainerSyncMetrics metrics;
+ private ReconStorageContainerSyncHelper syncHelper;
- TestReconStorageContainerSyncHelper() {
+ @BeforeEach
+ void setUp() {
+ metrics = ReconScmContainerSyncMetrics.create();
syncHelper = new ReconStorageContainerSyncHelper(
mockScmServiceProvider,
new OzoneConfiguration(),
- mockContainerManager
+ mockContainerManager,
+ metrics
);
}
+ @AfterEach
+ void tearDown() {
+ metrics.unRegister();
+ }
+
+ @Test
+ void testContainerSyncMetricsTrackPreSyncDriftAndDuration() throws Exception
{
+ when(mockScmServiceProvider.getContainerCount(OPEN)).thenReturn(5L);
+
when(mockScmServiceProvider.getContainerCount(QUASI_CLOSED)).thenReturn(1L);
+ when(mockScmServiceProvider.getContainerCount(CLOSED)).thenReturn(8L);
+ when(mockScmServiceProvider.getContainerCount(DELETED)).thenReturn(9L);
+ when(mockContainerManager.getContainerStateCount(OPEN)).thenReturn(3);
+
when(mockContainerManager.getContainerStateCount(QUASI_CLOSED)).thenReturn(1);
+ when(mockContainerManager.getContainerStateCount(CLOSED)).thenReturn(10);
+ when(mockContainerManager.getContainerStateCount(DELETED)).thenReturn(7);
+ when(mockScmServiceProvider.getListOfContainerIDs(
+ any(), any(Integer.class), any())).thenReturn(Collections.emptyList());
+
+ boolean result = syncHelper.syncWithSCMContainerInfo();
+
+ assertTrue(result);
+ assertEquals(2L, metrics.getContainerCountDrift(OPEN));
Review Comment:
Tests are updated in `TestReconStorageContainerManagerFacade` so they
validate the high-level metric path rather than only the helper/per-state
metrics.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/metrics/ReconScmContainerSyncMetrics.java:
##########
@@ -75,19 +103,115 @@ public void unRegister() {
ms.unregisterSource(SOURCE_NAME);
}
- public void setTargetedSyncStatus(int status) {
- targetedSyncStatus.set(status);
+ public void setScmContainerSyncStatus(int status) {
+ scmContainerSyncStatus.set(status);
+ }
+
+ public void setScmContainerSyncDurationMs(long durationMs) {
+ scmContainerSyncDurationMs.set(durationMs);
+ }
+
+ public void setContainerSyncDurationMs(
+ HddsProtos.LifeCycleState state, long durationMs) {
+ setStateGauge(containerSyncDurationMs, state, durationMs);
+ }
+
+ public void setContainerCountDrift(
+ HddsProtos.LifeCycleState state, long drift) {
+ setStateGauge(containerCountDrift, state, drift);
+ }
+
+ public int getScmContainerSyncStatus() {
+ return scmContainerSyncStatus.get();
+ }
+
+ public long getScmContainerSyncDurationMs() {
+ return scmContainerSyncDurationMs.get();
+ }
+
+ public long getContainerSyncDurationMs(
+ HddsProtos.LifeCycleState state) {
+ return getStateGauge(containerSyncDurationMs, state);
+ }
+
+ public long getContainerCountDrift(
+ HddsProtos.LifeCycleState state) {
+ return getStateGauge(containerCountDrift, state);
+ }
+
+ @Override
+ public void getMetrics(MetricsCollector collector, boolean all) {
+ MetricsRecordBuilder builder = collector.addRecord(SOURCE_NAME);
+ builder.addGauge(SCM_CONTAINER_SYNC_STATUS, getScmContainerSyncStatus());
+ builder.addGauge(SCM_CONTAINER_SYNC_DURATION_MS,
+ getScmContainerSyncDurationMs());
+ for (HddsProtos.LifeCycleState state : SYNC_STATES) {
+ builder.addGauge(containerSyncDurationMetricInfo.get(state),
+ getContainerSyncDurationMs(state));
+ builder.addGauge(containerCountDriftMetricInfo.get(state),
+ getContainerCountDrift(state));
+ }
+ }
+
+ private static Map<HddsProtos.LifeCycleState, AtomicLong>
+ initStateGaugeValues() {
+ Map<HddsProtos.LifeCycleState, AtomicLong> gauges =
+ new EnumMap<>(HddsProtos.LifeCycleState.class);
+ for (HddsProtos.LifeCycleState state : SYNC_STATES) {
+ gauges.put(state, new AtomicLong());
+ }
+ return Collections.unmodifiableMap(gauges);
+ }
+
+ private static Map<HddsProtos.LifeCycleState, MetricsInfo>
+ initSyncDurationMetricInfo() {
+ Map<HddsProtos.LifeCycleState, MetricsInfo> metrics =
+ new EnumMap<>(HddsProtos.LifeCycleState.class);
+ for (HddsProtos.LifeCycleState state : SYNC_STATES) {
+ String stateName = metricStateName(state);
+ metrics.put(state, Interns.info(
+ CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, stateName)
+ + "ContainerSyncDurationMs",
+ "Time taken by the " + stateName
+ + " container sync pass in milliseconds"));
+ }
+ return Collections.unmodifiableMap(metrics);
+ }
+
+ private static Map<HddsProtos.LifeCycleState, MetricsInfo>
+ initCountDriftMetricInfo() {
+ Map<HddsProtos.LifeCycleState, MetricsInfo> metrics =
+ new EnumMap<>(HddsProtos.LifeCycleState.class);
+ for (HddsProtos.LifeCycleState state : SYNC_STATES) {
+ String stateName = metricStateName(state);
+ metrics.put(state, Interns.info(
+ CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, stateName)
+ + "ContainerCountDrift",
+ "Container count drift observed at start of sync pass "
Review Comment:
I agree with your point. The exposed metric is effectively “last
successfully observed drift”, not necessarily drift from the latest attempted
sync. Changed the metric description.
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconStorageContainerSyncHelper.java:
##########
@@ -47,16 +54,51 @@ class TestReconStorageContainerSyncHelper {
private final ReconContainerManager mockContainerManager =
mock(ReconContainerManager.class);
- private final ReconStorageContainerSyncHelper syncHelper;
+ private ReconScmContainerSyncMetrics metrics;
+ private ReconStorageContainerSyncHelper syncHelper;
- TestReconStorageContainerSyncHelper() {
+ @BeforeEach
+ void setUp() {
+ metrics = ReconScmContainerSyncMetrics.create();
syncHelper = new ReconStorageContainerSyncHelper(
mockScmServiceProvider,
new OzoneConfiguration(),
- mockContainerManager
+ mockContainerManager,
+ metrics
);
}
+ @AfterEach
+ void tearDown() {
+ metrics.unRegister();
+ }
+
+ @Test
+ void testContainerSyncMetricsTrackPreSyncDriftAndDuration() throws Exception
{
+ when(mockScmServiceProvider.getContainerCount(OPEN)).thenReturn(5L);
+
when(mockScmServiceProvider.getContainerCount(QUASI_CLOSED)).thenReturn(1L);
+ when(mockScmServiceProvider.getContainerCount(CLOSED)).thenReturn(8L);
+ when(mockScmServiceProvider.getContainerCount(DELETED)).thenReturn(9L);
+ when(mockContainerManager.getContainerStateCount(OPEN)).thenReturn(3);
+
when(mockContainerManager.getContainerStateCount(QUASI_CLOSED)).thenReturn(1);
+ when(mockContainerManager.getContainerStateCount(CLOSED)).thenReturn(10);
+ when(mockContainerManager.getContainerStateCount(DELETED)).thenReturn(7);
+ when(mockScmServiceProvider.getListOfContainerIDs(
+ any(), any(Integer.class), any())).thenReturn(Collections.emptyList());
+
+ boolean result = syncHelper.syncWithSCMContainerInfo();
+
+ assertTrue(result);
+ assertEquals(2L, metrics.getContainerCountDrift(OPEN));
+ assertEquals(0L, metrics.getContainerCountDrift(QUASI_CLOSED));
+ assertEquals(-2L, metrics.getContainerCountDrift(CLOSED));
+ assertEquals(2L, metrics.getContainerCountDrift(DELETED));
+ assertTrue(metrics.getContainerSyncDurationMs(OPEN) >= 0);
Review Comment:
Have added more tests.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
