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 2021/06/24 19:02:41 UTC
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2366: HDDS-5332. Add a new column family and a service provider in Recon DB for Namespace Summaries
avijayanhwx commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658192670
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconRocksDB.java
##########
@@ -0,0 +1,119 @@
+package org.apache.hadoop.ozone.recon.spi.impl;
+import static org.apache.hadoop.ozone.recon.ReconConstants.RECON_CONTAINER_KEY_DB;
+import static org.apache.hadoop.ozone.recon.ReconServerConfigKeys.OZONE_RECON_DB_DIR;
+import java.io.File;
+import java.io.IOException;
+import javax.inject.Inject;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.db.DBStore;
+import org.apache.hadoop.hdds.utils.db.DBStoreBuilder;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import com.google.inject.ProvisionException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+
+/**
+ * Provider for Recon's RDB.
+ */
+public class ReconRocksDB {
+ private OzoneConfiguration configuration;
+ private ReconUtils reconUtils;
+ private DBStore dbStore;
+
+ @VisibleForTesting
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ReconRocksDB.class);
+
+ @Inject
+ ReconRocksDB(OzoneConfiguration configuration, ReconUtils reconUtils) {
+ this.configuration = configuration;
+ this.reconUtils = reconUtils;
+ this.dbStore = provideReconDB();
+ }
+
+ public DBStore provideReconDB() {
+ DBStore db;
+ File reconDbDir =
+ reconUtils.getReconDbDir(configuration, OZONE_RECON_DB_DIR);
+ File lastKnownContainerKeyDb =
+ reconUtils.getLastKnownDB(reconDbDir, RECON_CONTAINER_KEY_DB);
+ if (lastKnownContainerKeyDb != null) {
+ LOG.info("Last known DB : {}",
+ lastKnownContainerKeyDb.getAbsolutePath());
+ db = initializeDBStore(configuration,
+ lastKnownContainerKeyDb.getName());
+ } else {
+ db = getNewDBStore(configuration);
+ }
+ if (db == null) {
+ throw new ProvisionException("Unable to provide instance of DBStore " +
+ "store.");
+ }
+ return db;
+ }
+
+ public DBStore getDbStore() {
+ return dbStore;
+ }
+
+ public void reInit() throws IOException {
+ File oldDBLocation = dbStore.getDbLocation();
+ try {
+ dbStore.close();
+ } catch (Exception e) {
+ LOG.warn("Unable to close old Recon container key DB at {}.",
+ dbStore.getDbLocation().getAbsolutePath());
+ }
+ dbStore = getNewDBStore(configuration);
+ LOG.info("Creating new Recon Container DB at {}",
+ dbStore.getDbLocation().getAbsolutePath());
+
+ if (oldDBLocation.exists()) {
+ LOG.info("Cleaning up old Recon Container key DB at {}.",
+ oldDBLocation.getAbsolutePath());
+ FileUtils.deleteDirectory(oldDBLocation);
+ }
+ }
+
+ static void clearTable(Table table) throws IOException {
Review comment:
Suggested rename to '_truncateTable_' ?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
##########
@@ -234,8 +242,13 @@ public StorageContainerServiceProvider getStorageContainerServiceProvider() {
}
@VisibleForTesting
- public ContainerDBServiceProvider getContainerDBServiceProvider() {
- return containerDBServiceProvider;
+ public ReconContainerMetadataManager getContainerDBServiceProvider() {
Review comment:
Rename method to getRCMM?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconRocksDB.java
##########
@@ -0,0 +1,119 @@
+package org.apache.hadoop.ozone.recon.spi.impl;
+import static org.apache.hadoop.ozone.recon.ReconConstants.RECON_CONTAINER_KEY_DB;
+import static org.apache.hadoop.ozone.recon.ReconServerConfigKeys.OZONE_RECON_DB_DIR;
+import java.io.File;
+import java.io.IOException;
+import javax.inject.Inject;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.db.DBStore;
+import org.apache.hadoop.hdds.utils.db.DBStoreBuilder;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import com.google.inject.ProvisionException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+
+/**
+ * Provider for Recon's RDB.
+ */
+public class ReconRocksDB {
Review comment:
Suggested rename _ReconDBProvider_ or _ReconDBStore_ (Given that we have gotten rid of the older version of this class)
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconNamespaceSummaryManagerImpl.java
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.spi.impl;
+
+import org.apache.hadoop.hdds.utils.db.DBStore;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
+import static org.apache.hadoop.ozone.recon.spi.impl.ReconRocksDB.clearTable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import static org.apache.hadoop.ozone.recon.spi.impl.ReconDBDefinition.NAMESPACE_SUMMARY;
+
+import java.io.IOException;
+
+/**
+ * Wrapper functions for DB operations on recon namespace summary metadata.
+ */
+public class ReconNamespaceSummaryManagerImpl
+ implements ReconNamespaceSummaryManager {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ReconNamespaceSummaryManagerImpl.class);
+ private Table<Long, NSSummary> nsSummaryTable;
+ private DBStore namespaceDbStore;
+
+ // TODO: compute disk usage here?
+ @Inject
+ public ReconNamespaceSummaryManagerImpl(ReconRocksDB reconRocksDB) {
+ namespaceDbStore = reconRocksDB.getDbStore();
+ initializeTable();
+ }
+
+ @Override
+ public void initNSSummaryTable() throws IOException {
+ initializeTable();
+ }
+
+ @Override
+ public void storeNSSummary(long objectId, NSSummary nsSummary)
+ throws IOException {
+ nsSummaryTable.put(objectId, nsSummary);
+ }
+
+ @Override
+ public NSSummary getNSSummary(long objectId) throws IOException {
+ return nsSummaryTable.get(objectId);
+ }
+
+ private void initializeTable() {
+ try {
+ clearTable(this.nsSummaryTable);
+ this.nsSummaryTable = NAMESPACE_SUMMARY.getTable(namespaceDbStore);
Review comment:
Do we need to re-assign the table if all we do is deleting the entries in the table, instead of 'dropping' the column family?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconRocksDB.java
##########
@@ -17,79 +17,121 @@
*/
package org.apache.hadoop.ozone.recon.spi.impl;
-
import static org.apache.hadoop.ozone.recon.ReconConstants.RECON_CONTAINER_KEY_DB;
import static org.apache.hadoop.ozone.recon.ReconServerConfigKeys.OZONE_RECON_DB_DIR;
-
import java.io.File;
+import java.io.IOException;
+import javax.inject.Inject;
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.io.FileUtils;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
-import org.apache.hadoop.ozone.recon.ReconUtils;
import org.apache.hadoop.hdds.utils.db.DBStore;
import org.apache.hadoop.hdds.utils.db.DBStoreBuilder;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import com.google.inject.ProvisionException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.ProvisionException;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
/**
- * Provider for the Recon container DB (Metadata store).
+ * Provider for Recon's RDB.
*/
-public class ReconContainerDBProvider implements Provider<DBStore> {
+public class ReconRocksDB {
+ private OzoneConfiguration configuration;
+ private ReconUtils reconUtils;
+ private DBStore dbStore;
@VisibleForTesting
private static final Logger LOG =
- LoggerFactory.getLogger(ReconContainerDBProvider.class);
-
- private OzoneConfiguration configuration;
- private ReconUtils reconUtils;
+ LoggerFactory.getLogger(ReconRocksDB.class);
@Inject
- public ReconContainerDBProvider(OzoneConfiguration configuration,
- ReconUtils reconUtils) {
+ ReconRocksDB(OzoneConfiguration configuration, ReconUtils reconUtils) {
this.configuration = configuration;
this.reconUtils = reconUtils;
+ this.dbStore = provideReconDB();
}
- @Override
- public DBStore get() {
- DBStore dbStore;
+ public DBStore provideReconDB() {
+ DBStore db;
File reconDbDir =
- reconUtils.getReconDbDir(configuration, OZONE_RECON_DB_DIR);
+ reconUtils.getReconDbDir(configuration, OZONE_RECON_DB_DIR);
File lastKnownContainerKeyDb =
- reconUtils.getLastKnownDB(reconDbDir, RECON_CONTAINER_KEY_DB);
+ reconUtils.getLastKnownDB(reconDbDir, RECON_CONTAINER_KEY_DB);
if (lastKnownContainerKeyDb != null) {
- LOG.info("Last known container-key DB : {}",
- lastKnownContainerKeyDb.getAbsolutePath());
- dbStore = initializeDBStore(configuration,
- lastKnownContainerKeyDb.getName());
+ LOG.info("Last known DB : {}",
+ lastKnownContainerKeyDb.getAbsolutePath());
+ db = initializeDBStore(configuration,
+ lastKnownContainerKeyDb.getName());
} else {
- dbStore = getNewDBStore(configuration);
+ db = getNewDBStore(configuration);
}
- if (dbStore == null) {
+ if (db == null) {
throw new ProvisionException("Unable to provide instance of DBStore " +
- "store.");
+ "store.");
}
+ return db;
+ }
+
+ public DBStore getDbStore() {
return dbStore;
}
+ public void reInit() throws IOException {
Review comment:
We should never need to do a reInit on this DB now.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
##########
@@ -234,8 +242,13 @@ public StorageContainerServiceProvider getStorageContainerServiceProvider() {
}
@VisibleForTesting
- public ContainerDBServiceProvider getContainerDBServiceProvider() {
- return containerDBServiceProvider;
+ public ReconContainerMetadataManager getContainerDBServiceProvider() {
+ return reconContainerMetadataManager;
+ }
+
+ @VisibleForTesting
+ public ReconNamespaceSummaryManager getReconNamespaceSummaryManager() {
Review comment:
I don't see any usages for this getter. Can we remove it?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconNamespaceSummaryManager.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.spi;
+
+import org.apache.hadoop.hdds.annotation.InterfaceStability;
+import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+
+import java.io.IOException;
+
+/**
+ * Interface for DB operations on NS summary.
+ */
+@InterfaceStability.Unstable
+public interface ReconNamespaceSummaryManager {
+
+ // TODO: getDiskUsage? (w/w.o replication)
+ void initNSSummaryTable() throws IOException;
+
+ void storeNSSummary(long objectId, NSSummary nsSummary) throws IOException;
+
+ NSSummary getNSSummary(long objectId) throws IOException;
Review comment:
Taking in an object ID will be interesting :) Will revisit the interface when we do the subsequent patches which finishes up the computation & API layer.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
##########
@@ -145,6 +113,9 @@ public void initNewContainerDB(Map<ContainerKeyPrefix, Integer>
*/
private void initializeTables() {
try {
+ clearTable(this.containerKeyTable);
+ clearTable(this.containerKeyCountTable);
+ clearTable(this.containerReplicaHistoryTable);
Review comment:
Container Replica history table is not owned by the container-key mapper task. This is a regression that we are fixing now. Can we remove this truncation, and also rename '_initNewContainerDB_' to '_reinitWithNewContainerDataFromOm_' ?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
##########
@@ -93,8 +97,12 @@ protected void configureServlets() {
LOG.info("Initializing Recon server...");
try {
loginReconUserIfSecurityEnabled(configuration);
- this.containerDBServiceProvider =
- injector.getInstance(ContainerDBServiceProvider.class);
+ this.reconRocksDB = injector.getInstance(ReconRocksDB.class);
+ LOG.info("Recon Rocks DB.");
Review comment:
Debug LOG line left behind?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/codec/NSSummaryCodec.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.codec;
+
+import org.apache.hadoop.hdds.utils.db.IntegerCodec;
+import org.apache.hadoop.ozone.recon.ReconConstants;
+import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+import org.apache.hadoop.hdds.utils.db.Codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.IOException;
+
+/**
+ * Codec for Namespace Summary.
+ */
+public class NSSummaryCodec implements Codec<NSSummary>{
Review comment:
Can we add a unit test for this class?
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
##########
@@ -145,6 +113,9 @@ public void initNewContainerDB(Map<ContainerKeyPrefix, Integer>
*/
private void initializeTables() {
try {
+ clearTable(this.containerKeyTable);
+ clearTable(this.containerKeyCountTable);
+ clearTable(this.containerReplicaHistoryTable);
Review comment:
Instead of calling the "initializeTables", we can just truncate the 2 tables in the caller method. This initialize() method seems to be called from constructor as well.
--
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.
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