From c766182691a934f4b5b9ee793ecca7ce06f022dc Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Mon, 26 Oct 2020 07:43:40 -0400 Subject: [PATCH 01/17] Outline how static column family options cna be used for datanode --- .../container/metadata/AbstractDatanodeStore.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java index 6c258eda7cd0..025de95ae054 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java @@ -26,9 +26,14 @@ import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfoList; import org.apache.hadoop.ozone.container.common.interfaces.BlockIterator; +import org.rocksdb.BlockBasedTableConfig; +import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.DBOptions; +import org.rocksdb.LRUCache; +import org.rocksdb.Options; import org.rocksdb.Statistics; import org.rocksdb.StatsLevel; +import org.rocksdb.util.SizeUnit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +64,15 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { private DBStore store; private final AbstractDatanodeDBDefinition dbDef; private final long containerID; + private static final ColumnFamilyOptions cfOptions; + + static { + BlockBasedTableConfig table_config = new BlockBasedTableConfig(); + table_config.setBlockCache(new LRUCache(8 * SizeUnit.MB)); + Options options = new Options(); + options.setTableFormatConfig(table_config); + cfOptions = new ColumnFamilyOptions(options); + } /** * Constructs the metadata store and starts the DB services. From 0a7cce24feccc443debb33b11c9129d684411de1 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Mon, 26 Oct 2020 07:44:30 -0400 Subject: [PATCH 02/17] Remove unused method and make methods not used outside private --- .../hadoop/hdds/utils/db/DBStoreBuilder.java | 71 +++++++++---------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 6a202993bdca..8ab633686683 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -106,13 +106,43 @@ public static DBStoreBuilder newBuilder(OzoneConfiguration configuration, } public static DBStoreBuilder newBuilder(ConfigurationSource configuration, - DBDefinition definition) { + DBDefinition definition) { DBStoreBuilder builder = createDBStoreBuilder(configuration, definition); builder.registerTables(definition); return builder; } + /** + * Create DBStoreBuilder from a generic DBDefinition. + */ + public static DBStore createDBStore(ConfigurationSource configuration, + DBDefinition definition) throws IOException { + DBStoreBuilder builder = createDBStoreBuilder(configuration, definition); + builder.registerTables(definition); + + return builder.build(); + } + + private static DBStoreBuilder createDBStoreBuilder( + ConfigurationSource configuration, DBDefinition definition) { + + File metadataDir = definition.getDBLocation(configuration); + + if (metadataDir == null) { + + LOG.warn("{} is not configured. We recommend adding this setting. " + + "Falling back to {} instead.", + definition.getLocationConfigKey(), + HddsConfigKeys.OZONE_METADATA_DIRS); + metadataDir = getOzoneMetaDirPath(configuration); + } + + return DBStoreBuilder.newBuilder(configuration) + .setName(definition.getName()) + .setPath(Paths.get(metadataDir.getPath())); + } + public DBStoreBuilder setProfile(DBProfile profile) { dbProfile = profile; return this; @@ -133,12 +163,6 @@ public DBStoreBuilder addCodec(Class type, Codec codec) { return this; } - public DBStoreBuilder addTable(String tableName, ColumnFamilyOptions option) - throws IOException { - LOG.debug("using custom profile for table: {}", tableName); - return addTableDefinition(tableName, option); - } - private DBStoreBuilder addTableDefinition(String tableName, ColumnFamilyOptions option) throws IOException { TableConfig tableConfig = new TableConfig(tableName, option); @@ -273,38 +297,7 @@ private File getDBFile() throws IOException { return Paths.get(dbPath.toString(), dbname).toFile(); } - private static DBStoreBuilder createDBStoreBuilder( - ConfigurationSource configuration, DBDefinition definition) { - - File metadataDir = definition.getDBLocation(configuration); - - if (metadataDir == null) { - - LOG.warn("{} is not configured. We recommend adding this setting. " + - "Falling back to {} instead.", - definition.getLocationConfigKey(), - HddsConfigKeys.OZONE_METADATA_DIRS); - metadataDir = getOzoneMetaDirPath(configuration); - } - - return DBStoreBuilder.newBuilder(configuration) - .setName(definition.getName()) - .setPath(Paths.get(metadataDir.getPath())); - } - - /** - * Create DBStoreBuilder from a generic DBDefinition. - */ - public static DBStore createDBStore(ConfigurationSource configuration, - DBDefinition definition) - throws IOException { - DBStoreBuilder builder = createDBStoreBuilder(configuration, definition); - builder.registerTables(definition); - - return builder.build(); - } - - public DBStoreBuilder registerTables(DBDefinition definition) { + private DBStoreBuilder registerTables(DBDefinition definition) { for (DBColumnFamilyDefinition columnFamily : definition.getColumnFamilies()) { From 338aa070ded59a2a6fd40d3ef8be09134dbacad4 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Mon, 26 Oct 2020 08:36:52 -0400 Subject: [PATCH 03/17] Refactor DBStoreBuilder constructors Removal of DBProfile also got mixed in here, but not finished. --- .../hadoop/hdds/utils/db/DBStoreBuilder.java | 88 ++++++++----------- 1 file changed, 39 insertions(+), 49 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 8ab633686683..471e367dde6a 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -52,6 +52,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.security.auth.login.Configuration; + /** * DBStore Builder. */ @@ -69,9 +71,9 @@ public final class DBStoreBuilder { // DB PKIProfile used by ROCKDB instances. public static final DBProfile HDDS_DEFAULT_DB_PROFILE = DBProfile.DISK; - private Set tables; - private DBProfile dbProfile; + private Set tableConfigs; private DBOptions rocksDBOption; + private ColumnFamilyOptions columnFamilyOptions; private String dbname; private Path dbPath; private List tableNames; @@ -80,57 +82,49 @@ public final class DBStoreBuilder { private String rocksDbStat; private RocksDBConfiguration rocksDBConfiguration; - private DBStoreBuilder(ConfigurationSource configuration) { - this(configuration, configuration.getObject(RocksDBConfiguration.class)); + /** + * Create DBStoreBuilder from a generic DBDefinition. + */ + public static DBStore createDBStore(ConfigurationSource configuration, + DBDefinition definition) throws IOException { + return newBuilder(configuration, definition).build(); } - private DBStoreBuilder(ConfigurationSource configuration, - RocksDBConfiguration rocksDBConfiguration) { - tables = new HashSet<>(); - tableNames = new LinkedList<>(); - this.configuration = configuration; - this.registry = new CodecRegistry(); - this.rocksDbStat = configuration.getTrimmed( - OZONE_METADATA_STORE_ROCKSDB_STATISTICS, - OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT); - this.rocksDBConfiguration = rocksDBConfiguration; + public static DBStoreBuilder newBuilder(ConfigurationSource configuration, + DBDefinition definition) { + + DBStoreBuilder builder = newBuilder(configuration); + builder.applyDBDefinition(definition); + + return builder; } public static DBStoreBuilder newBuilder(ConfigurationSource configuration) { - return new DBStoreBuilder(configuration); + return newBuilder(configuration, + configuration.getObject(RocksDBConfiguration.class)); } - public static DBStoreBuilder newBuilder(OzoneConfiguration configuration, + public static DBStoreBuilder newBuilder(ConfigurationSource configuration, RocksDBConfiguration rocksDBConfiguration) { return new DBStoreBuilder(configuration, rocksDBConfiguration); } - public static DBStoreBuilder newBuilder(ConfigurationSource configuration, - DBDefinition definition) { - DBStoreBuilder builder = createDBStoreBuilder(configuration, definition); - builder.registerTables(definition); - - return builder; - } - - /** - * Create DBStoreBuilder from a generic DBDefinition. - */ - public static DBStore createDBStore(ConfigurationSource configuration, - DBDefinition definition) throws IOException { - DBStoreBuilder builder = createDBStoreBuilder(configuration, definition); - builder.registerTables(definition); - - return builder.build(); + private DBStoreBuilder(ConfigurationSource configuration, + RocksDBConfiguration rocksDBConfiguration) { + tableConfigs = new HashSet<>(); + tableNames = new LinkedList<>(); + this.configuration = configuration; + this.registry = new CodecRegistry(); + this.rocksDbStat = configuration.getTrimmed( + OZONE_METADATA_STORE_ROCKSDB_STATISTICS, + OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT); + this.rocksDBConfiguration = rocksDBConfiguration; } - private static DBStoreBuilder createDBStoreBuilder( - ConfigurationSource configuration, DBDefinition definition) { - + private DBStoreBuilder applyDBDefinition(DBDefinition definition) { File metadataDir = definition.getDBLocation(configuration); if (metadataDir == null) { - LOG.warn("{} is not configured. We recommend adding this setting. " + "Falling back to {} instead.", definition.getLocationConfigKey(), @@ -138,13 +132,10 @@ private static DBStoreBuilder createDBStoreBuilder( metadataDir = getOzoneMetaDirPath(configuration); } - return DBStoreBuilder.newBuilder(configuration) - .setName(definition.getName()) - .setPath(Paths.get(metadataDir.getPath())); - } + registerTables(definition); + setName(definition.getName()); + setPath(Paths.get(metadataDir.getPath())); - public DBStoreBuilder setProfile(DBProfile profile) { - dbProfile = profile; return this; } @@ -166,7 +157,7 @@ public DBStoreBuilder addCodec(Class type, Codec codec) { private DBStoreBuilder addTableDefinition(String tableName, ColumnFamilyOptions option) throws IOException { TableConfig tableConfig = new TableConfig(tableName, option); - if (!tables.add(tableConfig)) { + if (!tableConfigs.add(tableConfig)) { String message = "Unable to add the table: " + tableName + ". Please check if this table name is already in use."; LOG.error(message); @@ -197,26 +188,25 @@ public DBStore build() throws IOException { throw new IOException("Required parameter is missing. Please make sure " + "sure Path and DB name is provided."); } - processDBProfile(); + setDefaultDBProfile(); processTables(); DBOptions options = getDbProfile(); WriteOptions writeOptions = new WriteOptions(); writeOptions.setSync(rocksDBConfiguration.getSyncOption()); - File dbFile = getDBFile(); if (!dbFile.getParentFile().exists()) { throw new IOException("The DB destination directory should exist."); } - return new RDBStore(dbFile, options, writeOptions, tables, registry); + return new RDBStore(dbFile, options, writeOptions, tableConfigs, registry); } /** * if the DBProfile is not set, we will default to using default from the * config file. */ - private void processDBProfile() { + private void setDefaultDBProfile() { if (dbProfile == null) { dbProfile = this.configuration.getEnum(HDDS_DB_PROFILE, HDDS_DEFAULT_DB_PROFILE); @@ -241,7 +231,7 @@ private DBOptions getDbProfile() { if (StringUtil.isNotBlank(dbname)) { List columnFamilyDescriptors = new LinkedList<>(); - for (TableConfig tc : tables) { + for (TableConfig tc : tableConfigs) { columnFamilyDescriptors.add(tc.getDescriptor()); } From 5c481f891623b70e787227da82715050b8d42155 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Wed, 28 Oct 2020 08:05:21 -0400 Subject: [PATCH 04/17] Finish reorganizing methods --- .../metadata/AbstractDatanodeStore.java | 2 +- .../hadoop/hdds/utils/db/DBStoreBuilder.java | 220 +++++++++--------- 2 files changed, 116 insertions(+), 106 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java index 025de95ae054..dda5f6b4d7ad 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java @@ -107,7 +107,7 @@ public void start(ConfigurationSource config) } this.store = DBStoreBuilder.newBuilder(config, dbDef) - .setDBOption(options) + .setDBOptions(options) .build(); // Use the DatanodeTable wrapper to disable the table iterator on diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 471e367dde6a..8619d16cf2d7 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -24,15 +24,17 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Set; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.StringUtils; import org.apache.hadoop.hdds.conf.ConfigurationSource; -import org.apache.hadoop.hdds.conf.OzoneConfiguration; import com.google.common.base.Preconditions; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DB_PROFILE; @@ -52,8 +54,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.security.auth.login.Configuration; - /** * DBStore Builder. */ @@ -71,12 +71,12 @@ public final class DBStoreBuilder { // DB PKIProfile used by ROCKDB instances. public static final DBProfile HDDS_DEFAULT_DB_PROFILE = DBProfile.DISK; - private Set tableConfigs; private DBOptions rocksDBOption; - private ColumnFamilyOptions columnFamilyOptions; + private DBOptions defaultDBOptions; + private ColumnFamilyOptions defaultColumnFamilyOptions; private String dbname; private Path dbPath; - private List tableNames; + private Map tableOptions; private ConfigurationSource configuration; private CodecRegistry registry; private String rocksDbStat; @@ -111,17 +111,24 @@ public static DBStoreBuilder newBuilder(ConfigurationSource configuration, private DBStoreBuilder(ConfigurationSource configuration, RocksDBConfiguration rocksDBConfiguration) { - tableConfigs = new HashSet<>(); - tableNames = new LinkedList<>(); + tableOptions = new HashMap<>(); this.configuration = configuration; this.registry = new CodecRegistry(); this.rocksDbStat = configuration.getTrimmed( OZONE_METADATA_STORE_ROCKSDB_STATISTICS, OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT); this.rocksDBConfiguration = rocksDBConfiguration; + + DBProfile dbProfile = this.configuration.getEnum(HDDS_DB_PROFILE, + HDDS_DEFAULT_DB_PROFILE); + LOG.debug("default profile:{}", dbProfile); + + defaultDBOptions = dbProfile.getDBOptions(); + setDefaultColumnFamilyOptions(dbProfile.getColumnFamilyOptions()); } private DBStoreBuilder applyDBDefinition(DBDefinition definition) { + // Set metadata dirs. File metadataDir = definition.getDBLocation(configuration); if (metadataDir == null) { @@ -132,20 +139,63 @@ private DBStoreBuilder applyDBDefinition(DBDefinition definition) { metadataDir = getOzoneMetaDirPath(configuration); } - registerTables(definition); setName(definition.getName()); setPath(Paths.get(metadataDir.getPath())); + // Add column family names and codecs. + for (DBColumnFamilyDefinition columnFamily : + definition.getColumnFamilies()) { + + addTable(columnFamily.getName()); + addCodec(columnFamily.getKeyType(), columnFamily.getKeyCodec()); + addCodec(columnFamily.getValueType(), columnFamily.getValueCodec()); + } + return this; } + /** + * Builds a DBStore instance and returns that. + * + * @return DBStore + */ + public DBStore build() throws IOException { + if(StringUtil.isBlank(dbname) || (dbPath == null)) { + LOG.error("Required Parameter missing."); + throw new IOException("Required parameter is missing. Please make sure " + + "Path and DB name is provided."); + } + + Set tableConfigs = makeTableConfigs(); + + if (rocksDBOption == null) { + rocksDBOption = getDefaultDBOptions(tableConfigs); + } + + WriteOptions writeOptions = new WriteOptions(); + writeOptions.setSync(rocksDBConfiguration.getSyncOption()); + + File dbFile = getDBFile(); + if (!dbFile.getParentFile().exists()) { + throw new IOException("The DB destination directory should exist."); + } + + return new RDBStore(dbFile, rocksDBOption, writeOptions, tableConfigs, + registry); + } + public DBStoreBuilder setName(String name) { dbname = name; return this; } public DBStoreBuilder addTable(String tableName) { - tableNames.add(tableName); + return addTable(tableName, null); + } + + public DBStoreBuilder addTable(String tableName, + ColumnFamilyOptions options) { + tableOptions.put(tableName, options); return this; } @@ -154,20 +204,14 @@ public DBStoreBuilder addCodec(Class type, Codec codec) { return this; } - private DBStoreBuilder addTableDefinition(String tableName, - ColumnFamilyOptions option) throws IOException { - TableConfig tableConfig = new TableConfig(tableName, option); - if (!tableConfigs.add(tableConfig)) { - String message = "Unable to add the table: " + tableName + - ". Please check if this table name is already in use."; - LOG.error(message); - throw new IOException(message); - } + public DBStoreBuilder setDBOptions(DBOptions option) { + rocksDBOption = option; return this; } - public DBStoreBuilder setDBOption(DBOptions option) { - rocksDBOption = option; + public DBStoreBuilder setDefaultColumnFamilyOptions( + ColumnFamilyOptions cfOptions) { + defaultColumnFamilyOptions = cfOptions; return this; } @@ -177,60 +221,68 @@ public DBStoreBuilder setPath(Path path) { return this; } - /** - * Builds a DBStore instance and returns that. - * - * @return DBStore - */ - public DBStore build() throws IOException { - if(StringUtil.isBlank(dbname) || (dbPath == null)) { - LOG.error("Required Parameter missing."); - throw new IOException("Required parameter is missing. Please make sure " - + "sure Path and DB name is provided."); - } - setDefaultDBProfile(); - processTables(); - DBOptions options = getDbProfile(); + private Set makeTableConfigs() throws IOException { + Set tableConfigs = new HashSet<>(); - WriteOptions writeOptions = new WriteOptions(); - writeOptions.setSync(rocksDBConfiguration.getSyncOption()); + tableOptions.putIfAbsent(DEFAULT_COLUMN_FAMILY_NAME, + defaultColumnFamilyOptions); - File dbFile = getDBFile(); - if (!dbFile.getParentFile().exists()) { - throw new IOException("The DB destination directory should exist."); + for (Map.Entry entry: + tableOptions.entrySet()) { + String name = entry.getKey(); + ColumnFamilyOptions options = entry.getValue(); + + if (options == null) { + LOG.debug("using default profile for table:{}", name); + tableConfigs.add(new TableConfig(name, defaultColumnFamilyOptions)); + } + else { + tableConfigs.add(new TableConfig(name, options)); + } } - return new RDBStore(dbFile, options, writeOptions, tableConfigs, registry); + + return tableConfigs; } - /** - * if the DBProfile is not set, we will default to using default from the - * config file. - */ - private void setDefaultDBProfile() { - if (dbProfile == null) { - dbProfile = this.configuration.getEnum(HDDS_DB_PROFILE, - HDDS_DEFAULT_DB_PROFILE); + private DBOptions getDefaultDBOptions(Collection tableConfigs) { + DBOptions dbOptions = getDBOptionsFromFile(tableConfigs); + + if (dbOptions == null) { + dbOptions = defaultDBOptions; + + LOG.debug("Using default options: {}", dbProfile); + } + + // Apply logging settings. + if (rocksDBConfiguration.isRocksdbLoggingEnabled()) { + org.rocksdb.Logger logger = new org.rocksdb.Logger(dbOptions) { + @Override + protected void log(InfoLogLevel infoLogLevel, String s) { + ROCKS_DB_LOGGER.info(s); + } + }; + InfoLogLevel level = InfoLogLevel.valueOf(rocksDBConfiguration + .getRocksdbLogLevel() + "_LEVEL"); + logger.setInfoLogLevel(level); + dbOptions.setLogger(logger); } - LOG.debug("default profile:{}", dbProfile); - } - private void processTables() throws IOException { - List list = new ArrayList<>(tableNames); - list.add(DEFAULT_COLUMN_FAMILY_NAME); - for (String name : list) { - LOG.debug("using default profile for table:{}", name); - addTableDefinition(name, dbProfile.getColumnFamilyOptions()); + // Create statistics. + if (!rocksDbStat.equals(OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF)) { + Statistics statistics = new Statistics(); + statistics.setStatsLevel(StatsLevel.valueOf(rocksDbStat)); + dbOptions = dbOptions.setStatistics(statistics); } + + return dbOptions; } - private DBOptions getDbProfile() { - if (rocksDBOption != null) { - return rocksDBOption; - } + private DBOptions getDBOptionsFromFile(Collection tableConfigs) { DBOptions option = null; - if (StringUtil.isNotBlank(dbname)) { - List columnFamilyDescriptors = new LinkedList<>(); + List columnFamilyDescriptors = new ArrayList<>(); + + if (StringUtil.isNotBlank(dbname)) { for (TableConfig tc : tableConfigs) { columnFamilyDescriptors.add(tc.getDescriptor()); } @@ -248,29 +300,6 @@ private DBOptions getDbProfile() { } } - if (option == null) { - LOG.debug("Using default options: {}", dbProfile); - option = dbProfile.getDBOptions(); - } - - if (rocksDBConfiguration.isRocksdbLoggingEnabled()) { - org.rocksdb.Logger logger = new org.rocksdb.Logger(option) { - @Override - protected void log(InfoLogLevel infoLogLevel, String s) { - ROCKS_DB_LOGGER.info(s); - } - }; - InfoLogLevel level = InfoLogLevel.valueOf(rocksDBConfiguration - .getRocksdbLogLevel() + "_LEVEL"); - logger.setInfoLogLevel(level); - option.setLogger(logger); - } - - if (!rocksDbStat.equals(OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF)) { - Statistics statistics = new Statistics(); - statistics.setStatsLevel(StatsLevel.valueOf(rocksDbStat)); - option = option.setStatistics(statistics); - } return option; } @@ -286,23 +315,4 @@ private File getDBFile() throws IOException { } return Paths.get(dbPath.toString(), dbname).toFile(); } - - private DBStoreBuilder registerTables(DBDefinition definition) { - for (DBColumnFamilyDefinition columnFamily : - definition.getColumnFamilies()) { - - if (!columnFamily.getTableName().equals(DEFAULT_COLUMN_FAMILY_NAME)) { - // The default column family is always added. - // If it is present in the DB Definition, ignore it so we don't get an - // error about adding it twice. - addTable(columnFamily.getName()); - } - // Add new codecs specified for the table, which may be an alias for a - // table that was already added with different codecs. - addCodec(columnFamily.getKeyType(), columnFamily.getKeyCodec()); - addCodec(columnFamily.getValueType(), columnFamily.getValueCodec()); - } - - return this; - } } From 7b780ccc312b679e2e2c79c5ad9e6b3701151da9 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Thu, 29 Oct 2020 17:44:19 -0400 Subject: [PATCH 05/17] Code and comment clean up --- .../hadoop/hdds/utils/db/DBStoreBuilder.java | 85 ++++++++++++++----- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 8619d16cf2d7..5a0834590620 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -27,7 +27,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -71,12 +70,19 @@ public final class DBStoreBuilder { // DB PKIProfile used by ROCKDB instances. public static final DBProfile HDDS_DEFAULT_DB_PROFILE = DBProfile.DISK; + // the DBOptions used if the caller does not specify. + private final DBOptions defaultDBOptions; + // The DBOptions specified by the caller. private DBOptions rocksDBOption; - private DBOptions defaultDBOptions; - private ColumnFamilyOptions defaultColumnFamilyOptions; + // The column family options that will be used for any column families + // added by name only (without specifying options). + private ColumnFamilyOptions defaultCfOptions; private String dbname; private Path dbPath; - private Map tableOptions; + // Maps added column family names to the column family options they were + // added with. Value will be null if the column family was not added with + // any options. On build, this will be replaced with defaultCfOptions. + private Map cfOptions; private ConfigurationSource configuration; private CodecRegistry registry; private String rocksDbStat; @@ -111,7 +117,7 @@ public static DBStoreBuilder newBuilder(ConfigurationSource configuration, private DBStoreBuilder(ConfigurationSource configuration, RocksDBConfiguration rocksDBConfiguration) { - tableOptions = new HashMap<>(); + cfOptions = new HashMap<>(); this.configuration = configuration; this.registry = new CodecRegistry(); this.rocksDbStat = configuration.getTrimmed( @@ -119,15 +125,17 @@ private DBStoreBuilder(ConfigurationSource configuration, OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT); this.rocksDBConfiguration = rocksDBConfiguration; + // Get default DBOptions and ColumnFamilyOptions from the default DB + // profile. DBProfile dbProfile = this.configuration.getEnum(HDDS_DB_PROFILE, HDDS_DEFAULT_DB_PROFILE); - LOG.debug("default profile:{}", dbProfile); + LOG.debug("Default DB profile:{}", dbProfile); defaultDBOptions = dbProfile.getDBOptions(); - setDefaultColumnFamilyOptions(dbProfile.getColumnFamilyOptions()); + setDefaultCfOptions(dbProfile.getColumnFamilyOptions()); } - private DBStoreBuilder applyDBDefinition(DBDefinition definition) { + private void applyDBDefinition(DBDefinition definition) { // Set metadata dirs. File metadataDir = definition.getDBLocation(configuration); @@ -150,8 +158,6 @@ private DBStoreBuilder applyDBDefinition(DBDefinition definition) { addCodec(columnFamily.getKeyType(), columnFamily.getKeyCodec()); addCodec(columnFamily.getValueType(), columnFamily.getValueCodec()); } - - return this; } /** @@ -195,7 +201,7 @@ public DBStoreBuilder addTable(String tableName) { public DBStoreBuilder addTable(String tableName, ColumnFamilyOptions options) { - tableOptions.put(tableName, options); + cfOptions.put(tableName, options); return this; } @@ -209,9 +215,9 @@ public DBStoreBuilder setDBOptions(DBOptions option) { return this; } - public DBStoreBuilder setDefaultColumnFamilyOptions( + public DBStoreBuilder setDefaultCfOptions( ColumnFamilyOptions cfOptions) { - defaultColumnFamilyOptions = cfOptions; + defaultCfOptions = cfOptions; return this; } @@ -221,20 +227,33 @@ public DBStoreBuilder setPath(Path path) { return this; } - private Set makeTableConfigs() throws IOException { + public DBStoreBuilder setProfile(DBProfile prof) { + setDBOptions(prof.getDBOptions()); + setDefaultCfOptions(prof.getColumnFamilyOptions()); + return this; + } + + /** + * Converts column families and their corresponding options that have been + * registered with the builder to a set of {@link TableConfig} objects. + * Column families with no options specified will have the default column + * family options for this builder applied. + */ + private Set makeTableConfigs() { Set tableConfigs = new HashSet<>(); - tableOptions.putIfAbsent(DEFAULT_COLUMN_FAMILY_NAME, - defaultColumnFamilyOptions); + // If default column family was not added, add it with the default options. + cfOptions.putIfAbsent(DEFAULT_COLUMN_FAMILY_NAME, + defaultCfOptions); for (Map.Entry entry: - tableOptions.entrySet()) { + cfOptions.entrySet()) { String name = entry.getKey(); ColumnFamilyOptions options = entry.getValue(); if (options == null) { - LOG.debug("using default profile for table:{}", name); - tableConfigs.add(new TableConfig(name, defaultColumnFamilyOptions)); + LOG.debug("using default column family options for table: {}", name); + tableConfigs.add(new TableConfig(name, defaultCfOptions)); } else { tableConfigs.add(new TableConfig(name, options)); @@ -244,13 +263,27 @@ private Set makeTableConfigs() throws IOException { return tableConfigs; } + /** + * Attempts to get RocksDB {@link DBOptions} from an ini config file. If + * that file does not exist, the value of {@code defaultDBOptions} is used + * instead. + * After an {@link DBOptions} is chosen, it will have the logging level + * specified by builder's {@link RocksDBConfiguration} applied to it. It + * will also have statistics added if they are not turned off in the + * builder's {@link ConfigurationSource}. + * + * @param tableConfigs Configurations for each column family, used when + * reading DB options from the ini file. + * + * @return The {@link DBOptions} that should be used as the default value + * for this builder if one is not specified by the caller. + */ private DBOptions getDefaultDBOptions(Collection tableConfigs) { DBOptions dbOptions = getDBOptionsFromFile(tableConfigs); if (dbOptions == null) { dbOptions = defaultDBOptions; - - LOG.debug("Using default options: {}", dbProfile); + LOG.debug("Using RocksDB DBOptions from default profile."); } // Apply logging settings. @@ -277,6 +310,12 @@ protected void log(InfoLogLevel infoLogLevel, String s) { return dbOptions; } + /** + * Attempts to construct a {@link DBOptions} object from the configuration + * directory with name equal to {@code database name}.ini, where {@code + * database name} is the property set by + * {@link DBStoreBuilder#setName(String)}. + */ private DBOptions getDBOptionsFromFile(Collection tableConfigs) { DBOptions option = null; @@ -292,10 +331,10 @@ private DBOptions getDBOptionsFromFile(Collection tableConfigs) { option = DBConfigFromFile.readFromFile(dbname, columnFamilyDescriptors); if(option != null) { - LOG.info("Using Configs from {}.ini file", dbname); + LOG.info("Using RocksDB DBOptions from {}.ini file", dbname); } } catch (IOException ex) { - LOG.info("Unable to read RocksDB config from {}", dbname, ex); + LOG.info("Unable to read RocksDB DBOptions from {}", dbname, ex); } } } From 290b5ec96b8b6b58cb50bbcbff09280e5ee580f5 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Thu, 29 Oct 2020 18:02:07 -0400 Subject: [PATCH 06/17] Add documentation for profile setter --- .../java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 5a0834590620..0156d0f8f435 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -227,6 +227,10 @@ public DBStoreBuilder setPath(Path path) { return this; } + /** + * Set the {@link DBOptions} and default {@link ColumnFamilyOptions} based + * on {@code prof}. + */ public DBStoreBuilder setProfile(DBProfile prof) { setDBOptions(prof.getDBOptions()); setDefaultCfOptions(prof.getColumnFamilyOptions()); From cba325251787f298948f68dc3fa86a146be4aa3e Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Fri, 30 Oct 2020 11:06:16 -0400 Subject: [PATCH 07/17] Rename column fmaily setting options --- .../org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 0156d0f8f435..7f56a35b3030 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -132,7 +132,7 @@ private DBStoreBuilder(ConfigurationSource configuration, LOG.debug("Default DB profile:{}", dbProfile); defaultDBOptions = dbProfile.getDBOptions(); - setDefaultCfOptions(dbProfile.getColumnFamilyOptions()); + setDefaultCFOptions(dbProfile.getColumnFamilyOptions()); } private void applyDBDefinition(DBDefinition definition) { @@ -215,7 +215,7 @@ public DBStoreBuilder setDBOptions(DBOptions option) { return this; } - public DBStoreBuilder setDefaultCfOptions( + public DBStoreBuilder setDefaultCFOptions( ColumnFamilyOptions cfOptions) { defaultCfOptions = cfOptions; return this; @@ -233,7 +233,7 @@ public DBStoreBuilder setPath(Path path) { */ public DBStoreBuilder setProfile(DBProfile prof) { setDBOptions(prof.getDBOptions()); - setDefaultCfOptions(prof.getColumnFamilyOptions()); + setDefaultCFOptions(prof.getColumnFamilyOptions()); return this; } From 85b0a505fb2afb8cac9ce7d8cc88e0aa7f6edc0f Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Fri, 30 Oct 2020 11:06:39 -0400 Subject: [PATCH 08/17] Add simple test for registering the same table name multiple times --- .../hdds/utils/db/TestDBStoreBuilder.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestDBStoreBuilder.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestDBStoreBuilder.java index d406060165f3..99fcbae80452 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestDBStoreBuilder.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestDBStoreBuilder.java @@ -27,6 +27,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; +import org.rocksdb.ColumnFamilyOptions; import java.io.File; import java.io.IOException; @@ -99,15 +100,27 @@ public void builderWithDoubleTableName() throws Exception { if(!newFolder.exists()) { Assert.assertTrue(newFolder.mkdirs()); } - thrown.expect(IOException.class); - DBStoreBuilder.newBuilder(conf) + // Registering a new table with the same name should replace the previous + // one. + DBStore dbStore = DBStoreBuilder.newBuilder(conf) .setName("Test.db") .setPath(newFolder.toPath()) .addTable("FIRST") - .addTable("FIRST") + .addTable("FIRST", new ColumnFamilyOptions()) .build(); - // Nothing to do , This will throw so we do not have to close. + // Building should succeed without error. + + try (Table firstTable = dbStore.getTable("FIRST")) { + byte[] key = + RandomStringUtils.random(9).getBytes(StandardCharsets.UTF_8); + byte[] value = + RandomStringUtils.random(9).getBytes(StandardCharsets.UTF_8); + firstTable.put(key, value); + byte[] temp = firstTable.get(key); + Assert.assertArrayEquals(value, temp); + } + dbStore.close(); } @Test From 86a25cfa2f848eda69a6b7895597b699a97a1de4 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Fri, 30 Oct 2020 11:07:17 -0400 Subject: [PATCH 09/17] Fix errors when sharing column family options and cache --- .../metadata/AbstractDatanodeStore.java | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java index dda5f6b4d7ad..95a004bead86 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java @@ -27,10 +27,11 @@ import org.apache.hadoop.ozone.container.common.helpers.ChunkInfoList; import org.apache.hadoop.ozone.container.common.interfaces.BlockIterator; import org.rocksdb.BlockBasedTableConfig; +import org.rocksdb.BloomFilter; import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.DBOptions; import org.rocksdb.LRUCache; -import org.rocksdb.Options; +import org.rocksdb.RocksDB; import org.rocksdb.Statistics; import org.rocksdb.StatsLevel; import org.rocksdb.util.SizeUnit; @@ -60,18 +61,26 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { private Table deletedBlocksTable; private static final Logger LOG = - LoggerFactory.getLogger(AbstractDatanodeStore.class); + LoggerFactory.getLogger(AbstractDatanodeStore.class); private DBStore store; private final AbstractDatanodeDBDefinition dbDef; private final long containerID; private static final ColumnFamilyOptions cfOptions; + private static final DBProfile DEFAULT_PROFILE = DBProfile.DISK; + private static final long CACHE_SIZE = 8 * SizeUnit.MB; + static { - BlockBasedTableConfig table_config = new BlockBasedTableConfig(); - table_config.setBlockCache(new LRUCache(8 * SizeUnit.MB)); - Options options = new Options(); - options.setTableFormatConfig(table_config); - cfOptions = new ColumnFamilyOptions(options); + // Enables static construction of RocksDB objects. + RocksDB.loadLibrary(); + + BlockBasedTableConfig tableConfig = new BlockBasedTableConfig(); + tableConfig.setBlockCache(new LRUCache(CACHE_SIZE)) + .setPinL0FilterAndIndexBlocksInCache(true) + .setFilterPolicy(new BloomFilter()); + + cfOptions = DEFAULT_PROFILE.getColumnFamilyOptions(); + cfOptions.setTableFormatConfig(tableConfig); } /** @@ -81,8 +90,7 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { * @throws IOException - on Failure. */ protected AbstractDatanodeStore(ConfigurationSource config, long containerID, - AbstractDatanodeDBDefinition dbDef) - throws IOException { + AbstractDatanodeDBDefinition dbDef) throws IOException { this.dbDef = dbDef; this.containerID = containerID; start(config); @@ -90,9 +98,9 @@ protected AbstractDatanodeStore(ConfigurationSource config, long containerID, @Override public void start(ConfigurationSource config) - throws IOException { + throws IOException { if (this.store == null) { - DBOptions options = new DBOptions(); + DBOptions options = DEFAULT_PROFILE.getDBOptions(); options.setCreateIfMissing(true); options.setCreateMissingColumnFamilies(true); @@ -108,6 +116,7 @@ public void start(ConfigurationSource config) this.store = DBStoreBuilder.newBuilder(config, dbDef) .setDBOptions(options) + .setDefaultCFOptions(cfOptions) .build(); // Use the DatanodeTable wrapper to disable the table iterator on From eab4a124b10dd682646b2bdc8542907e6b543a4d Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Mon, 2 Nov 2020 17:58:48 -0500 Subject: [PATCH 10/17] Rename static column family options variable --- .../ozone/container/metadata/AbstractDatanodeStore.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java index 95a004bead86..eadf09fd76ee 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java @@ -65,8 +65,8 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { private DBStore store; private final AbstractDatanodeDBDefinition dbDef; private final long containerID; - private static final ColumnFamilyOptions cfOptions; + private static final ColumnFamilyOptions COLUMN_FAMILY_OPTIONS; private static final DBProfile DEFAULT_PROFILE = DBProfile.DISK; private static final long CACHE_SIZE = 8 * SizeUnit.MB; @@ -79,8 +79,8 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { .setPinL0FilterAndIndexBlocksInCache(true) .setFilterPolicy(new BloomFilter()); - cfOptions = DEFAULT_PROFILE.getColumnFamilyOptions(); - cfOptions.setTableFormatConfig(tableConfig); + COLUMN_FAMILY_OPTIONS = DEFAULT_PROFILE.getColumnFamilyOptions(); + COLUMN_FAMILY_OPTIONS.setTableFormatConfig(tableConfig); } /** @@ -116,7 +116,7 @@ public void start(ConfigurationSource config) this.store = DBStoreBuilder.newBuilder(config, dbDef) .setDBOptions(options) - .setDefaultCFOptions(cfOptions) + .setDefaultCFOptions(COLUMN_FAMILY_OPTIONS) .build(); // Use the DatanodeTable wrapper to disable the table iterator on From 02b81897aebbcf7fea0db69ab17bc97afebcfe15 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 3 Nov 2020 07:32:21 -0500 Subject: [PATCH 11/17] Fix checkstyle violations --- .../org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java index 7f56a35b3030..5b907afd9f82 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java @@ -216,8 +216,8 @@ public DBStoreBuilder setDBOptions(DBOptions option) { } public DBStoreBuilder setDefaultCFOptions( - ColumnFamilyOptions cfOptions) { - defaultCfOptions = cfOptions; + ColumnFamilyOptions options) { + defaultCfOptions = options; return this; } @@ -258,8 +258,7 @@ private Set makeTableConfigs() { if (options == null) { LOG.debug("using default column family options for table: {}", name); tableConfigs.add(new TableConfig(name, defaultCfOptions)); - } - else { + } else { tableConfigs.add(new TableConfig(name, options)); } } From 35a6fe102a5c68afc97f93909dd66078159e262e Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 3 Nov 2020 09:09:58 -0500 Subject: [PATCH 12/17] Retrigger CI From 18d90f717e55b76c8695a8cdcd5a7604e7d02c3e Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 10 Nov 2020 14:56:47 -0500 Subject: [PATCH 13/17] Allow cache size to be specified from xml configuration --- .../apache/hadoop/ozone/OzoneConfigKeys.java | 5 ++ .../src/main/resources/ozone-default.xml | 10 +++ .../metadata/AbstractDatanodeStore.java | 61 ++++++++++++++----- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index 482ac88f366c..024b8dc6b3ba 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -343,6 +343,11 @@ public final class OzoneConfigKeys { public static final double HDDS_DATANODE_STORAGE_UTILIZATION_CRITICAL_THRESHOLD_DEFAULT = 0.95; + public static final String + HDDS_DATANODE_METADATA_CACHE_SIZE = "hdds.datanode.metadata.cache.size"; + public static final String + HDDS_DATANODE_CACHE_SIZE_DEFAULT = "64MB"; + public static final String OZONE_SECURITY_ENABLED_KEY = "ozone.security.enabled"; public static final boolean OZONE_SECURITY_ENABLED_DEFAULT = false; diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 485397819fcd..d63dbbec64fc 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -1278,6 +1278,16 @@ + + hdds.datanode.metadata.cache.size + 64MB + OZONE, DATANODE, MANAGEMENT + + Size of the shared block metadata cache on each datanode. All + containers on a datanode will share this cache. + + + hdds.command.status.report.interval 30s diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java index eadf09fd76ee..49ad7b98476c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java @@ -17,12 +17,15 @@ */ package org.apache.hadoop.ozone.container.metadata; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.StringUtils; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.hdds.conf.StorageUnit; import org.apache.hadoop.hdds.utils.MetadataKeyFilters; import org.apache.hadoop.hdds.utils.MetadataKeyFilters.KeyPrefixFilter; import org.apache.hadoop.hdds.utils.db.*; +import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfoList; import org.apache.hadoop.ozone.container.common.interfaces.BlockIterator; @@ -40,7 +43,10 @@ import java.io.Closeable; import java.io.IOException; +import java.util.Collections; +import java.util.Map; import java.util.NoSuchElementException; +import java.util.concurrent.ConcurrentHashMap; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_STORE_ROCKSDB_STATISTICS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT; @@ -65,23 +71,11 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { private DBStore store; private final AbstractDatanodeDBDefinition dbDef; private final long containerID; + private final ColumnFamilyOptions cfOptions; - private static final ColumnFamilyOptions COLUMN_FAMILY_OPTIONS; private static final DBProfile DEFAULT_PROFILE = DBProfile.DISK; - private static final long CACHE_SIZE = 8 * SizeUnit.MB; - - static { - // Enables static construction of RocksDB objects. - RocksDB.loadLibrary(); - - BlockBasedTableConfig tableConfig = new BlockBasedTableConfig(); - tableConfig.setBlockCache(new LRUCache(CACHE_SIZE)) - .setPinL0FilterAndIndexBlocksInCache(true) - .setFilterPolicy(new BloomFilter()); - - COLUMN_FAMILY_OPTIONS = DEFAULT_PROFILE.getColumnFamilyOptions(); - COLUMN_FAMILY_OPTIONS.setTableFormatConfig(tableConfig); - } + private static final Map + OPTIONS_CACHE = new ConcurrentHashMap<>(); /** * Constructs the metadata store and starts the DB services. @@ -91,6 +85,15 @@ public abstract class AbstractDatanodeStore implements DatanodeStore { */ protected AbstractDatanodeStore(ConfigurationSource config, long containerID, AbstractDatanodeDBDefinition dbDef) throws IOException { + + // The same config instance is used on each datanode, so we can share the + // corresponding column family options, providing a single shared cache + // for all containers on a datanode. + if (!OPTIONS_CACHE.containsKey(config)) { + OPTIONS_CACHE.put(config, buildColumnFamilyOptions(config)); + } + cfOptions = OPTIONS_CACHE.get(config); + this.dbDef = dbDef; this.containerID = containerID; start(config); @@ -116,7 +119,7 @@ public void start(ConfigurationSource config) this.store = DBStoreBuilder.newBuilder(config, dbDef) .setDBOptions(options) - .setDefaultCFOptions(COLUMN_FAMILY_OPTIONS) + .setDefaultCFOptions(cfOptions) .build(); // Use the DatanodeTable wrapper to disable the table iterator on @@ -202,6 +205,12 @@ public void compactDB() throws IOException { store.compactDB(); } + @VisibleForTesting + public static Map + getColumnFamilyOptionsCache() { + return Collections.unmodifiableMap(OPTIONS_CACHE); + } + private static void checkTableStatus(Table table, String name) throws IOException { String logMessage = "Unable to get a reference to %s table. Cannot " + @@ -214,6 +223,26 @@ private static void checkTableStatus(Table table, String name) } } + private static ColumnFamilyOptions buildColumnFamilyOptions( + ConfigurationSource config) { + long cacheSize = (long) config.getStorageSize( + OzoneConfigKeys.HDDS_DATANODE_METADATA_CACHE_SIZE, + OzoneConfigKeys.HDDS_DATANODE_CACHE_SIZE_DEFAULT, + StorageUnit.BYTES); + + // Enables static creation of RocksDB objects. + RocksDB.loadLibrary(); + + BlockBasedTableConfig tableConfig = new BlockBasedTableConfig(); + tableConfig.setBlockCache(new LRUCache(cacheSize * SizeUnit.MB)) + .setPinL0FilterAndIndexBlocksInCache(true) + .setFilterPolicy(new BloomFilter()); + + return DEFAULT_PROFILE + .getColumnFamilyOptions() + .setTableFormatConfig(tableConfig); + } + /** * Block Iterator for KeyValue Container. This block iterator returns blocks * which match with the {@link MetadataKeyFilters.KeyPrefixFilter}. If no From ff1c93a261da055f240b067c2d00bcc0e4b8d600 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 10 Nov 2020 14:57:09 -0500 Subject: [PATCH 14/17] Add unit test for column family options sharing among containers --- .../keyvalue/TestKeyValueContainer.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index 79cf6a7e1ecc..b72f2b559c2a 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -20,6 +20,7 @@ import org.apache.hadoop.conf.StorageUnit; import org.apache.hadoop.hdds.client.BlockID; +import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; @@ -37,6 +38,7 @@ import org.apache.hadoop.ozone.container.common.volume.VolumeSet; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; +import org.apache.hadoop.ozone.container.metadata.AbstractDatanodeStore; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.util.DiskChecker; import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB; @@ -50,6 +52,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.mockito.Mockito; +import org.rocksdb.ColumnFamilyOptions; import java.io.File; @@ -383,4 +386,32 @@ public void testUpdateContainerUnsupportedRequest() throws Exception { .getResult()); } } + + @Test + public void testContainersShareColumnFamilyOptions() throws Exception { + // Get a read only view (not a copy) of the options cache. + Map cachedOptions = + AbstractDatanodeStore.getColumnFamilyOptionsCache(); + Assert.assertTrue(cachedOptions.isEmpty()); + + // Create Container 1 + keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); + Assert.assertEquals(1, cachedOptions.size()); + ColumnFamilyOptions options1 = cachedOptions.get(conf); + Assert.assertNotNull(options1); + + // Create Container 2 + keyValueContainerData = new KeyValueContainerData(2L, + layout, + (long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), + datanodeId.toString()); + keyValueContainer = new KeyValueContainer(keyValueContainerData, conf); + keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); + + Assert.assertEquals(1, cachedOptions.size()); + ColumnFamilyOptions options2 = cachedOptions.get(conf); + + // Column family options object should be reused. + Assert.assertSame(options1, options2); + } } From 1e68293a5d67e91af8f3c1bf0d33e597be27e9e4 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 10 Nov 2020 17:27:41 -0500 Subject: [PATCH 15/17] Fix failing unit test by using one static configuration in unit tests --- .../ozone/container/keyvalue/TestKeyValueContainer.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index b72f2b559c2a..628cb7d60083 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -82,7 +82,6 @@ public class TestKeyValueContainer { @Rule public TemporaryFolder folder = new TemporaryFolder(); - private OzoneConfiguration conf; private String scmId = UUID.randomUUID().toString(); private VolumeSet volumeSet; private RoundRobinVolumeChoosingPolicy volumeChoosingPolicy; @@ -92,6 +91,11 @@ public class TestKeyValueContainer { private final ChunkLayOutVersion layout; + // Use one configuration object across parameterized runs of tests. + // This preserves the column family options in the container options + // cache for testContainersShareColumnFamilyOptions. + private static final OzoneConfiguration conf = new OzoneConfiguration(); + public TestKeyValueContainer(ChunkLayOutVersion layout) { this.layout = layout; } @@ -103,7 +107,6 @@ public static Iterable parameters() { @Before public void setUp() throws Exception { - conf = new OzoneConfiguration(); datanodeId = UUID.randomUUID(); HddsVolume hddsVolume = new HddsVolume.Builder(folder.getRoot() .getAbsolutePath()).conf(conf).datanodeUuid(datanodeId @@ -392,7 +395,6 @@ public void testContainersShareColumnFamilyOptions() throws Exception { // Get a read only view (not a copy) of the options cache. Map cachedOptions = AbstractDatanodeStore.getColumnFamilyOptionsCache(); - Assert.assertTrue(cachedOptions.isEmpty()); // Create Container 1 keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); @@ -410,6 +412,7 @@ public void testContainersShareColumnFamilyOptions() throws Exception { Assert.assertEquals(1, cachedOptions.size()); ColumnFamilyOptions options2 = cachedOptions.get(conf); + Assert.assertNotNull(options2); // Column family options object should be reused. Assert.assertSame(options1, options2); From f0e65baaf875349b943eb920bab6fbfb78a53294 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 10 Nov 2020 17:40:58 -0500 Subject: [PATCH 16/17] Fix checkstyle violation --- .../keyvalue/TestKeyValueContainer.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index 628cb7d60083..c2b487be2933 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -94,7 +94,7 @@ public class TestKeyValueContainer { // Use one configuration object across parameterized runs of tests. // This preserves the column family options in the container options // cache for testContainersShareColumnFamilyOptions. - private static final OzoneConfiguration conf = new OzoneConfiguration(); + private static final OzoneConfiguration CONF = new OzoneConfiguration(); public TestKeyValueContainer(ChunkLayOutVersion layout) { this.layout = layout; @@ -109,7 +109,7 @@ public static Iterable parameters() { public void setUp() throws Exception { datanodeId = UUID.randomUUID(); HddsVolume hddsVolume = new HddsVolume.Builder(folder.getRoot() - .getAbsolutePath()).conf(conf).datanodeUuid(datanodeId + .getAbsolutePath()).conf(CONF).datanodeUuid(datanodeId .toString()).build(); volumeSet = mock(MutableVolumeSet.class); @@ -122,14 +122,14 @@ public void setUp() throws Exception { (long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), datanodeId.toString()); - keyValueContainer = new KeyValueContainer(keyValueContainerData, conf); + keyValueContainer = new KeyValueContainer(keyValueContainerData, CONF); } private void addBlocks(int count) throws Exception { long containerId = keyValueContainerData.getContainerID(); try(ReferenceCountedDB metadataStore = BlockUtils.getDB(keyValueContainer - .getContainerData(), conf)) { + .getContainerData(), CONF)) { for (int i = 0; i < count; i++) { // Creating BlockData BlockID blockID = new BlockID(containerId, i); @@ -186,7 +186,7 @@ public void testContainerImportExport() throws Exception { long numberOfKeysToWrite = 12; //write one few keys to check the key count after import try(ReferenceCountedDB metadataStore = - BlockUtils.getDB(keyValueContainerData, conf)) { + BlockUtils.getDB(keyValueContainerData, CONF)) { Table blockDataTable = metadataStore.getStore().getBlockDataTable(); @@ -199,7 +199,7 @@ public void testContainerImportExport() throws Exception { metadataStore.getStore().getMetadataTable() .put(OzoneConsts.BLOCK_COUNT, numberOfKeysToWrite); } - BlockUtils.removeDB(keyValueContainerData, conf); + BlockUtils.removeDB(keyValueContainerData, CONF); Map metadata = new HashMap<>(); metadata.put("key1", "value1"); @@ -225,7 +225,7 @@ public void testContainerImportExport() throws Exception { keyValueContainerData.getLayOutVersion(), keyValueContainerData.getMaxSize(), UUID.randomUUID().toString(), datanodeId.toString()); - KeyValueContainer container = new KeyValueContainer(containerData, conf); + KeyValueContainer container = new KeyValueContainer(containerData, CONF); HddsVolume containerVolume = volumeChoosingPolicy.chooseVolume(volumeSet .getVolumesList(), 1); @@ -297,7 +297,7 @@ public void testDeleteContainer() throws Exception { keyValueContainerData.setState(ContainerProtos.ContainerDataProto.State .CLOSED); keyValueContainer = new KeyValueContainer( - keyValueContainerData, conf); + keyValueContainerData, CONF); keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); keyValueContainer.delete(); @@ -376,7 +376,7 @@ public void testUpdateContainerUnsupportedRequest() throws Exception { try { keyValueContainerData.setState( ContainerProtos.ContainerDataProto.State.CLOSED); - keyValueContainer = new KeyValueContainer(keyValueContainerData, conf); + keyValueContainer = new KeyValueContainer(keyValueContainerData, CONF); keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); Map metadata = new HashMap<>(); metadata.put(OzoneConsts.VOLUME, OzoneConsts.OZONE); @@ -399,7 +399,7 @@ public void testContainersShareColumnFamilyOptions() throws Exception { // Create Container 1 keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); Assert.assertEquals(1, cachedOptions.size()); - ColumnFamilyOptions options1 = cachedOptions.get(conf); + ColumnFamilyOptions options1 = cachedOptions.get(CONF); Assert.assertNotNull(options1); // Create Container 2 @@ -407,11 +407,11 @@ public void testContainersShareColumnFamilyOptions() throws Exception { layout, (long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), datanodeId.toString()); - keyValueContainer = new KeyValueContainer(keyValueContainerData, conf); + keyValueContainer = new KeyValueContainer(keyValueContainerData, CONF); keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId); Assert.assertEquals(1, cachedOptions.size()); - ColumnFamilyOptions options2 = cachedOptions.get(conf); + ColumnFamilyOptions options2 = cachedOptions.get(CONF); Assert.assertNotNull(options2); // Column family options object should be reused. From 6d99e8c9c7639d2c3c48d7473dfd7ea9a468b0b7 Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Thu, 12 Nov 2020 15:14:43 -0500 Subject: [PATCH 17/17] Rename cache size configuration to hdds.datanode.metadata.rocksdb.cache.size --- .../main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java | 6 +++--- hadoop-hdds/common/src/main/resources/ozone-default.xml | 6 +++--- .../ozone/container/metadata/AbstractDatanodeStore.java | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index 024b8dc6b3ba..21157baa99ef 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -343,10 +343,10 @@ public final class OzoneConfigKeys { public static final double HDDS_DATANODE_STORAGE_UTILIZATION_CRITICAL_THRESHOLD_DEFAULT = 0.95; + public static final String HDDS_DATANODE_METADATA_ROCKSDB_CACHE_SIZE = + "hdds.datanode.metadata.rocksdb.cache.size"; public static final String - HDDS_DATANODE_METADATA_CACHE_SIZE = "hdds.datanode.metadata.cache.size"; - public static final String - HDDS_DATANODE_CACHE_SIZE_DEFAULT = "64MB"; + HDDS_DATANODE_METADATA_ROCKSDB_CACHE_SIZE_DEFAULT = "64MB"; public static final String OZONE_SECURITY_ENABLED_KEY = "ozone.security.enabled"; diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index d63dbbec64fc..0f7c94913322 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -1279,12 +1279,12 @@ - hdds.datanode.metadata.cache.size + hdds.datanode.metadata.rocksdb.cache.size 64MB OZONE, DATANODE, MANAGEMENT - Size of the shared block metadata cache on each datanode. All - containers on a datanode will share this cache. + Size of the block metadata cache shared among RocksDB instances on each + datanode. All containers on a datanode will share this cache. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java index 49ad7b98476c..efbc24730af7 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java @@ -25,7 +25,6 @@ import org.apache.hadoop.hdds.utils.MetadataKeyFilters; import org.apache.hadoop.hdds.utils.MetadataKeyFilters.KeyPrefixFilter; import org.apache.hadoop.hdds.utils.db.*; -import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfoList; import org.apache.hadoop.ozone.container.common.interfaces.BlockIterator; @@ -51,6 +50,8 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_STORE_ROCKSDB_STATISTICS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF; +import static org.apache.hadoop.ozone.OzoneConfigKeys.HDDS_DATANODE_METADATA_ROCKSDB_CACHE_SIZE; +import static org.apache.hadoop.ozone.OzoneConfigKeys.HDDS_DATANODE_METADATA_ROCKSDB_CACHE_SIZE_DEFAULT; /** * Implementation of the {@link DatanodeStore} interface that contains @@ -226,8 +227,8 @@ private static void checkTableStatus(Table table, String name) private static ColumnFamilyOptions buildColumnFamilyOptions( ConfigurationSource config) { long cacheSize = (long) config.getStorageSize( - OzoneConfigKeys.HDDS_DATANODE_METADATA_CACHE_SIZE, - OzoneConfigKeys.HDDS_DATANODE_CACHE_SIZE_DEFAULT, + HDDS_DATANODE_METADATA_ROCKSDB_CACHE_SIZE, + HDDS_DATANODE_METADATA_ROCKSDB_CACHE_SIZE_DEFAULT, StorageUnit.BYTES); // Enables static creation of RocksDB objects.