smengcl commented on code in PR #11659:
URL: https://github.com/apache/ozone/pull/11659#discussion_r4333369071
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java:
##########
@@ -978,6 +979,72 @@ public void
testICRsOnContainerClose(ContainerLayoutVersion containerLayoutVersi
}
}
+ @Test
+ public void testCreateContainerReleasesSpaceOnUncheckedFailure() throws
Exception {
+ final long containerID = 1L;
+ HandlerWithVolumeSet handlerWithVolume =
+ createCreateContainerHandlerWithVolumeSet(tempDir, true);
+ HddsVolume hddsVolume = (HddsVolume)
handlerWithVolume.getVolumeSet().getVolumesList().get(0);
+ String datanodeId = handlerWithVolume.getHandler().getDatanodeId();
+
+ ContainerCommandResponseProto response =
handlerWithVolume.getHandler().handle(
+ createContainerRequest(datanodeId, containerID), null, null);
+
+ assertEquals(CONTAINER_INTERNAL_ERROR, response.getResult());
+ assertThat(response.getMessage()).contains("dbParentDir == null");
+ assertEquals(0, hddsVolume.getCommittedBytes());
+ assertNull(handlerWithVolume.getContainerSet().getContainer(containerID));
+ }
+
+ @Test
+ public void testCreateContainerKeepsSpaceOfCreatedContainer() throws
Exception {
+ final long containerID = 1L;
+ HandlerWithVolumeSet handlerWithVolume =
+ createCreateContainerHandlerWithVolumeSet(tempDir, false);
+ HddsVolume hddsVolume = (HddsVolume)
handlerWithVolume.getVolumeSet().getVolumesList().get(0);
+ String datanodeId = handlerWithVolume.getHandler().getDatanodeId();
+ KeyValueHandler kvHandler = handlerWithVolume.getHandler();
+
+ ContainerCommandResponseProto response =
+ kvHandler.handleCreateContainer(createContainerRequest(datanodeId,
containerID), null);
+ assertEquals(ContainerProtos.Result.SUCCESS, response.getResult());
+ KeyValueContainer container =
+ (KeyValueContainer)
handlerWithVolume.getContainerSet().getContainer(containerID);
+ assertNotNull(container);
+ assertEquals(maxContainerSize, hddsVolume.getCommittedBytes());
+ assertTrue(container.getContainerData().isCommittedSpace());
+
+ long committedAfterFirst = hddsVolume.getCommittedBytes();
+ ContainerCommandResponseProto response2 =
+ kvHandler.handleCreateContainer(createContainerRequest(datanodeId,
containerID), null);
+ assertEquals(ContainerProtos.Result.SUCCESS, response2.getResult());
+ assertEquals(committedAfterFirst, hddsVolume.getCommittedBytes());
+ assertTrue(container.getContainerData().isCommittedSpace());
+ }
+
+ private HandlerWithVolumeSet createCreateContainerHandlerWithVolumeSet(
Review Comment:
nit: this mostly duplicates the existing
`createKeyValueHandlerWithVolumeSet(Path)`, but skips its cleanup
(`ContainerMetrics.remove()` and unregistering the volume stats). The tests can
reuse it and drop this helper:
```diff
- HandlerWithVolumeSet handlerWithVolume =
- createCreateContainerHandlerWithVolumeSet(tempDir, true);
+ HandlerWithVolumeSet handlerWithVolume =
createKeyValueHandlerWithVolumeSet(tempDir);
HddsVolume hddsVolume = (HddsVolume)
handlerWithVolume.getVolumeSet().getVolumesList().get(0);
+ hddsVolume.setDbParentDir(null);
```
and `createKeyValueHandlerWithVolumeSet(tempDir)` in
`testCreateContainerKeepsSpaceOfCreatedContainer`. `TestKeyValueHandler` still
passes locally with that change.
--
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]
