smengcl commented on code in PR #10862:
URL: https://github.com/apache/ozone/pull/10862#discussion_r3716362073
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMDBCheckpointServletInodeBasedXferNonLeader.java:
##########
@@ -77,4 +88,46 @@ void
processMetadataSnapshotRequestSetsStatusWhenSendErrorFails() throws Excepti
verify(response).setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
}
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void processMetadataSnapshotRequestDoesNotReturn503WhenLeader(boolean
isLeaderReady) throws Exception {
+ OMDBCheckpointServletInodeBasedXfer servlet =
+ spy(new OMDBCheckpointServletInodeBasedXfer());
+ OzoneManager om = mock(OzoneManager.class);
+ when(om.isLeader()).thenReturn(true);
+ when(om.isLeaderReady()).thenReturn(isLeaderReady);
+
+ ServletContext ctx = mock(ServletContext.class);
+ when(ctx.getAttribute(OzoneConsts.OM_CONTEXT_ATTRIBUTE)).thenReturn(om);
+ doReturn(ctx).when(servlet).getServletContext();
+ // Force a failure after leader check so this unit test can stay
lightweight
+ // (no full servlet/bootstrap setup) while still proving that leader
requests
+ // are not rejected with 503.
+ doThrow(new IOException("test collect failure"))
+ .when(servlet).collectDbDataToTransfer(any(), anySet(), any());
Review Comment:
This test does not initialize the bootstrap lock. Therefore, the request
throws a NullPointerException before collectDbDataToTransfer() runs. The catch
block sets status 500, so the test passes for an unintended reason. Pls provide
a mock lock that can be acquired. Then verify that collectDbDataToTransfer()
runs and throws the intended test exception.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMDBCheckpointServletInodeBasedXferNonLeader.java:
##########
@@ -77,4 +88,46 @@ void
processMetadataSnapshotRequestSetsStatusWhenSendErrorFails() throws Excepti
verify(response).setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
}
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void processMetadataSnapshotRequestDoesNotReturn503WhenLeader(boolean
isLeaderReady) throws Exception {
+ OMDBCheckpointServletInodeBasedXfer servlet =
+ spy(new OMDBCheckpointServletInodeBasedXfer());
+ OzoneManager om = mock(OzoneManager.class);
+ when(om.isLeader()).thenReturn(true);
+ when(om.isLeaderReady()).thenReturn(isLeaderReady);
+
+ ServletContext ctx = mock(ServletContext.class);
+ when(ctx.getAttribute(OzoneConsts.OM_CONTEXT_ATTRIBUTE)).thenReturn(om);
+ doReturn(ctx).when(servlet).getServletContext();
+ // Force a failure after leader check so this unit test can stay
lightweight
+ // (no full servlet/bootstrap setup) while still proving that leader
requests
+ // are not rejected with 503.
+ doThrow(new IOException("test collect failure"))
+ .when(servlet).collectDbDataToTransfer(any(), anySet(), any());
Review Comment:
diff
```diff
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMDBCheckpointServletInodeBasedXferNonLeader.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMDBCheckpointServletInodeBasedXferNonLeader.java
index 978dbc6e829..00000000000 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMDBCheckpointServletInodeBasedXferNonLeader.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMDBCheckpointServletInodeBasedXferNonLeader.java
@@ -36,8 +36,10 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.hadoop.hdds.scm.HddsWhiteboxTestUtils;
import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.lock.BootstrapStateHandler;
import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer;
import
org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer.RaftServerStatus;
+import org.apache.ratis.util.UncheckedAutoCloseable;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
@@ -101,6 +103,10 @@ class TestOMDBCheckpointServletInodeBasedXferNonLeader {
ServletContext ctx = mock(ServletContext.class);
when(ctx.getAttribute(OzoneConsts.OM_CONTEXT_ATTRIBUTE)).thenReturn(om);
doReturn(ctx).when(servlet).getServletContext();
+ BootstrapStateHandler.Lock lock =
mock(BootstrapStateHandler.Lock.class);
+ when(lock.acquireWriteLock())
+ .thenReturn(mock(UncheckedAutoCloseable.class));
+ doReturn(lock).when(servlet).getBootstrapStateLock();
// Force a failure after leader check so this unit test can stay
lightweight
// (no full servlet/bootstrap setup) while still proving that leader
requests
// are not rejected with 503.
@@ -112,6 +118,7 @@ class TestOMDBCheckpointServletInodeBasedXferNonLeader {
servlet.processMetadataSnapshotRequest(request, response, false, true);
+ verify(servlet).collectDbDataToTransfer(eq(request), anySet(), any());
verify(response, never())
.sendError(eq(HttpServletResponse.SC_SERVICE_UNAVAILABLE),
anyString());
verify(om).isLeader();
```
--
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]
