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/06/13 08:36:45 UTC
[GitHub] [ozone] JyotinderSingh commented on a diff in pull request #3483: HDDS-6489 : Unit test to check cleanup tables for each OM response
JyotinderSingh commented on code in PR #3483:
URL: https://github.com/apache/ozone/pull/3483#discussion_r895454919
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfoResponses.java:
##########
@@ -0,0 +1,331 @@
+package org.apache.hadoop.ozone.om.response;
Review Comment:
Please add the Apache License comment to the top of any new files you create in the project.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfoResponses.java:
##########
@@ -0,0 +1,331 @@
+package org.apache.hadoop.ozone.om.response;
+
+import com.google.common.collect.Maps;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.fs.SaveSpaceUsageToFile;
+import org.apache.hadoop.hdds.scm.storage.RatisBlockOutputStream;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.response.s3.security.S3GetSecretResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+import org.reflections.Reflections;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Array;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.ozone.om.response.TestCleanupTableInfo.OM_RESPONSE_PACKAGE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.spy;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestCleanupTableInfoResponses{
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ RatisBlockOutputStream.class);
+
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+
+ /**
+ * Creates a spy object over an instantiated OMMetadataManager, giving the
+ * possibility to redefine behaviour. In the current implementation
+ * there isn't any behaviour which is redefined.
+ *
+ * @return the OMMetadataManager spy instance created.
+ * @throws IOException if I/O error occurs in setting up data store for the
+ * metadata manager.
+ */
+ private OMMetadataManager createOMMetadataManagerSpy(File folder) throws IOException {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ File newFolder = folder;
+ if (!newFolder.exists()) {
+ Assert.assertTrue(newFolder.mkdirs());
+ }
+ ServerUtils.setOzoneMetaDirPath(conf, newFolder.toString());
+ return spy(new OmMetadataManagerImpl(conf));
+ }
+
+ private void mockTables(OMMetadataManager omMetadataManager,
+ Set<String> updatedTables,
+ boolean isFSO)
+ throws IOException, IllegalAccessException {
+ Map<String, Table> tableMap = omMetadataManager.listTables();
+ Map<Table, Table> mockedTableMap = new HashMap<>();
+ List<String> tables = new ArrayList<>(tableMap.keySet());
+ for (String table:tables) {
+ Table mockedTable = Mockito.spy(tableMap.get(table));
+ Answer answer = invocationOnMock -> {
+ String t = table;
+ if (isFSO) {
+ if (table.equals(OmMetadataManagerImpl.OPEN_KEY_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_FILE_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.KEY_TABLE)) {
+ t = OmMetadataManagerImpl.FILE_TABLE;
+ }
+ } else {
+ if (table.equals(OmMetadataManagerImpl.OPEN_FILE_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_KEY_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.FILE_TABLE)) {
+ t = OmMetadataManagerImpl.KEY_TABLE;
+ }
+ }
+ updatedTables.add(t);
+ return null;
+ };
+ Mockito.lenient().doAnswer(answer).when(mockedTable).put(any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable)
+ .putWithBatch(any(), any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable).delete(any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable).deleteWithBatch(any(), any());
+ mockedTableMap.put(tableMap.get(table), mockedTable);
+ tableMap.put(table, mockedTable);
+ }
+ for (Field f:omMetadataManager.getClass().getDeclaredFields()) {
+ boolean isAccessible = f.isAccessible();
+ f.setAccessible(true);
+ try {
+ if (Table.class.isAssignableFrom(f.getType())) {
+ f.set(omMetadataManager,
+ mockedTableMap.getOrDefault(f.get(omMetadataManager),
+ (Table)f.get(omMetadataManager)));
+ }
+ } finally {
+ f.setAccessible(isAccessible);
+ }
+ }
+ }
+ private <T> T getMockObject(Class<T> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status,
+ boolean isFSO) {
+ T instance = Mockito.mock(type, invocationOnMock -> {
+ try {
+ return createInstance(invocationOnMock.getMethod().getReturnType(),
+ instanceMap, status, isFSO);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ }
+ });
+ instanceMap.put(type, instance);
+ return instance;
+ }
+ private List<Field> getFields(Class<?> type) {
+ List<Field> fields = new ArrayList<>();
+ while (type != Object.class) {
+ fields.addAll(Arrays.asList(type.getDeclaredFields()));
+ type = type.getSuperclass();
+ }
+ return fields;
+ }
+ private Object createInstance(Class<?> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status, boolean isFSO) {
+ if (instanceMap.containsKey(type)) {
+ return instanceMap.get(type);
+ }
+ Object instance = null;
+ if (type.isArray()) {
+ return Array.newInstance(type.getComponentType(), 0);
+ } else if (type.equals(Void.TYPE)) {
+ return null;
+ } else if (type.isPrimitive()) {
+ if (Boolean.TYPE.equals(type)) {
+ return true;
+ } else if (Character.TYPE.equals(type)) {
+ return 'a';
+ } else if (Byte.TYPE.equals(type)) {
+ return 0xFF;
+ } else if (Short.TYPE.equals(type)) {
+ return (short) 0;
+ } else if (Integer.TYPE.equals(type)) {
+ return 0;
+ } else if (Long.TYPE.equals(type)) {
+ return (long) 0;
+ } else if (Float.TYPE.equals(type)) {
+ return (float) 0.0;
+ } else if (Double.TYPE.equals(type)) {
+ return 0.0;
+ } else if (Void.TYPE.equals(type)) {
+ return null;
+ }
Review Comment:
A `switch` statement might be more readable here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMAllocateBlockResponseWithFSO.java:
##########
@@ -54,7 +54,7 @@ public OMAllocateBlockResponseWithFSO(@Nonnull OMResponse omResponse,
}
@Override
- public void addToDBBatch(OMMetadataManager omMetadataManager,
+ public void addToDBBatch(OMMetadataManager omMetadataManager,
Review Comment:
nit: remove indentation change.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfoResponses.java:
##########
@@ -0,0 +1,331 @@
+package org.apache.hadoop.ozone.om.response;
+
+import com.google.common.collect.Maps;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.fs.SaveSpaceUsageToFile;
+import org.apache.hadoop.hdds.scm.storage.RatisBlockOutputStream;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.response.s3.security.S3GetSecretResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+import org.reflections.Reflections;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Array;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.apache.hadoop.ozone.om.response.TestCleanupTableInfo.OM_RESPONSE_PACKAGE;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.spy;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestCleanupTableInfoResponses{
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ RatisBlockOutputStream.class);
+
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+
+ /**
+ * Creates a spy object over an instantiated OMMetadataManager, giving the
+ * possibility to redefine behaviour. In the current implementation
+ * there isn't any behaviour which is redefined.
+ *
+ * @return the OMMetadataManager spy instance created.
+ * @throws IOException if I/O error occurs in setting up data store for the
+ * metadata manager.
+ */
+ private OMMetadataManager createOMMetadataManagerSpy(File folder) throws IOException {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ File newFolder = folder;
+ if (!newFolder.exists()) {
+ Assert.assertTrue(newFolder.mkdirs());
+ }
+ ServerUtils.setOzoneMetaDirPath(conf, newFolder.toString());
+ return spy(new OmMetadataManagerImpl(conf));
+ }
+
+ private void mockTables(OMMetadataManager omMetadataManager,
+ Set<String> updatedTables,
+ boolean isFSO)
+ throws IOException, IllegalAccessException {
+ Map<String, Table> tableMap = omMetadataManager.listTables();
+ Map<Table, Table> mockedTableMap = new HashMap<>();
+ List<String> tables = new ArrayList<>(tableMap.keySet());
+ for (String table:tables) {
+ Table mockedTable = Mockito.spy(tableMap.get(table));
+ Answer answer = invocationOnMock -> {
+ String t = table;
+ if (isFSO) {
+ if (table.equals(OmMetadataManagerImpl.OPEN_KEY_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_FILE_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.KEY_TABLE)) {
+ t = OmMetadataManagerImpl.FILE_TABLE;
+ }
+ } else {
+ if (table.equals(OmMetadataManagerImpl.OPEN_FILE_TABLE)) {
+ t = OmMetadataManagerImpl.OPEN_KEY_TABLE;
+ }
+ if (table.equals(OmMetadataManagerImpl.FILE_TABLE)) {
+ t = OmMetadataManagerImpl.KEY_TABLE;
+ }
+ }
+ updatedTables.add(t);
+ return null;
+ };
+ Mockito.lenient().doAnswer(answer).when(mockedTable).put(any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable)
+ .putWithBatch(any(), any(), any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable).delete(any());
+ Mockito.lenient().doAnswer(answer).when(mockedTable).deleteWithBatch(any(), any());
+ mockedTableMap.put(tableMap.get(table), mockedTable);
+ tableMap.put(table, mockedTable);
+ }
+ for (Field f:omMetadataManager.getClass().getDeclaredFields()) {
+ boolean isAccessible = f.isAccessible();
+ f.setAccessible(true);
+ try {
+ if (Table.class.isAssignableFrom(f.getType())) {
+ f.set(omMetadataManager,
+ mockedTableMap.getOrDefault(f.get(omMetadataManager),
+ (Table)f.get(omMetadataManager)));
+ }
+ } finally {
+ f.setAccessible(isAccessible);
+ }
+ }
+ }
+ private <T> T getMockObject(Class<T> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status,
+ boolean isFSO) {
+ T instance = Mockito.mock(type, invocationOnMock -> {
+ try {
+ return createInstance(invocationOnMock.getMethod().getReturnType(),
+ instanceMap, status, isFSO);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ }
+ });
+ instanceMap.put(type, instance);
+ return instance;
+ }
+ private List<Field> getFields(Class<?> type) {
+ List<Field> fields = new ArrayList<>();
+ while (type != Object.class) {
+ fields.addAll(Arrays.asList(type.getDeclaredFields()));
+ type = type.getSuperclass();
+ }
+ return fields;
+ }
+ private Object createInstance(Class<?> type,
+ Map<Class, Object> instanceMap,
+ OzoneManagerProtocolProtos.Status status, boolean isFSO) {
+ if (instanceMap.containsKey(type)) {
+ return instanceMap.get(type);
+ }
+ Object instance = null;
+ if (type.isArray()) {
+ return Array.newInstance(type.getComponentType(), 0);
+ } else if (type.equals(Void.TYPE)) {
+ return null;
+ } else if (type.isPrimitive()) {
+ if (Boolean.TYPE.equals(type)) {
+ return true;
+ } else if (Character.TYPE.equals(type)) {
+ return 'a';
+ } else if (Byte.TYPE.equals(type)) {
+ return 0xFF;
+ } else if (Short.TYPE.equals(type)) {
+ return (short) 0;
+ } else if (Integer.TYPE.equals(type)) {
+ return 0;
+ } else if (Long.TYPE.equals(type)) {
+ return (long) 0;
+ } else if (Float.TYPE.equals(type)) {
+ return (float) 0.0;
+ } else if (Double.TYPE.equals(type)) {
+ return 0.0;
+ } else if (Void.TYPE.equals(type)) {
+ return null;
+ }
+ } else if (type.equals(OzoneManagerProtocolProtos.OMResponse.class)) {
+ return OzoneManagerProtocolProtos.OMResponse.newBuilder()
+ .setStatus(status).buildPartial();
+ } else if (type.equals(BucketLayout.class)) {
+ return isFSO ? BucketLayout.FILE_SYSTEM_OPTIMIZED
+ : BucketLayout.DEFAULT;
Review Comment:
It would be better to explicitly define the bucket layout here. For instance `LEGACY` or `OBJECT_STORE` instead of `DEFAULT`.
--
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