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 18:23:08 UTC
[GitHub] [ozone] yuangu002 opened a new pull request #2366: HDDS-5332. Add a new column family and a service provider in Recon DB for Namespace Summaries
yuangu002 opened a new pull request #2366:
URL: https://github.com/apache/ozone/pull/2366
## What changes were proposed in this pull request?
Table schema:
objectId --> class NSSummary
New class NSSummary should at least include these fields:
1. Total number of files directly[1] under this directory,
2. Total size of files directly under this directory,
3. File Size Buckets of files directly under this directory.
Note: [1] directly means not counting files under any subdirectories, for now. We could further optimize the speed later by trading more writes (propagating the statistics any layers deep upwards) with speed. This is for reducing potential write-amplification for now. The stats could be extended into arbitrary layers deep later.
[06/21] Renamed ReconContainerDBProvider to ReconDBProvider
[06/21] Expanded the scope of this issue by designing a service provider interface for namespace summary, which wraps up all operations on the new schema.
[06/23] Merge with HDDS-5379
Because we are intending to add a new CF to the Recon container DB, the existing ContainerDBServiceProvider must be refactored to resolve conflict with a new service provider sharing the same RDB as the current one directly operates on the DB not just the CF/tables.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5332
## How was this patch tested?
Unit test: TestReconNamespaceSummaryManagerImpl
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658279158
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -458,7 +458,6 @@ public void testGetDatanodes() throws Exception {
.findFirst().orElse(null);
return (datanodeMetadata1 != null &&
datanodeMetadata1.getContainers() == 1 &&
- datanodeMetadata1.getOpenContainers() == 1 &&
Review comment:
why delete this?
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658281819
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -458,7 +458,6 @@ public void testGetDatanodes() throws Exception {
.findFirst().orElse(null);
return (datanodeMetadata1 != null &&
datanodeMetadata1.getContainers() == 1 &&
- datanodeMetadata1.getOpenContainers() == 1 &&
Review comment:
Original file doesn't have this line. I added it back when I didn't figure out a way to write test case for multiple containers
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658795762
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
Even we expand or shrink the num_of_bins in the future, the current approach should have no problem with it. In `NSSummaryCodec`, I calculated the number of ints stored in NSSummary:
`private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;`
then, I calculated the number of bytes needed for (de)serialization:
`final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;`
and
`assert(rawData.length == NUM_OF_INTS * Integer.BYTES);`,
so even if in the future we change the num of bins, we don't need any modification in `NSSummaryCodec`
(also take a look at my new commits, now we only need to change max/min file size upper bound. The num_of_bins is computed based on them)
--
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
[GitHub] [ozone] smengcl commented on pull request #2366: HDDS-5332. Add a new column family and a service provider in Recon DB for Namespace Summaries
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#issuecomment-867898437
pls check the [findbugs](https://github.com/apache/ozone/runs/2907805505) warning:
```
M V EI: org.apache.hadoop.ozone.recon.api.types.NSSummary.getFileSizeBucket() may expose internal representation by returning NSSummary.fileSizeBucket At NSSummary.java:[line 57]
```
--
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
[GitHub] [ozone] avijayanhwx merged pull request #2366: HDDS-5332. Add a new column family and a service provider in Recon DB for Namespace Summaries
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #2366:
URL: https://github.com/apache/ozone/pull/2366
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658270637
##########
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();
+ }
Review comment:
Dupes `initializeTable`? If no other operations need to be performed here we can just remove this method
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658997710
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
hmm that wasn't my point. My point is the future upgradability. Consider this: if we expanded the bins by 1 slot in the future. NUM_OF_INTS in the executed binary will be 41+1=42. If we are still reading from the **old** RDB, there won't be a 42-th entry in the byte array. So it'll likely throw an exception at this point.
A worse case is, if we also added a new int field to the NSSummary class at the same time indicating a different metric. Then the new code won't know if the 42-th entry is the new bin or new field. Because the new code in `fromPersistedFormat` is using the new `NUM_OF_INTS=42`, not the old `NUM_OF_INTS=41`.
The solution is very simple. Just writing an extra int (or `short`, to save space) indicating _the length of array_ right before the content (list of ints) of the array. Even if we don't do much in `fromPersistedFormat` right now, it will help later on that we add logic to get the correct array length. For example, just read the length-indicating int and throw it away. It will help when expand the class.
That's also part of the reason why we use protobuf to deal with all those compatibility details.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658971480
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconConstants.java
##########
@@ -58,4 +58,9 @@ private ReconConstants() {
+ PIPELINE_DB_SUFFIX;
public static final String RECON_SCM_NODE_DB =
"recon-node.db";
+ public static final long MAX_FILE_SIZE_UPPER_BOUND = 1125899906842624L;
Review comment:
```suggestion
// 1125899906842624L = 1PB
public static final long MAX_FILE_SIZE_UPPER_BOUND = 1125899906842624L;
```
move the original comment here 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.
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658243379
##########
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)
Review comment:
A new method?
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658220687
##########
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:
yeah sure, but the next step will be adding a new -du task to query info from OM and write into RDB, so maybe it could be used for testing that. But for now, I am good with removing it.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658246825
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
##########
@@ -110,25 +94,9 @@ public void close() throws Exception {
public void initNewContainerDB(Map<ContainerKeyPrefix, Integer>
Review comment:
TODO: Rename this
```suggestion
public void reinitContainerTable(Map<ContainerKeyPrefix, Integer>
```
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658434144
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
##########
@@ -110,25 +94,9 @@ public void close() throws Exception {
public void initNewContainerDB(Map<ContainerKeyPrefix, Integer>
Review comment:
Followed Aravindan's suggestion: `reinitWithNewContainerDataFromOm`
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658240303
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
One suggestion here: we should probably write NUM_OF_BINS before writing the array. Just in case we expand or shrink the array in the future. Accordingly we need to handle this in `fromPersistedFormat`.
An even better approach would be to have an `IntegerArrayCodec` that handles the dynamic length of int array?
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658224563
##########
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:
I intentionally broke the codec and the test failed
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r659005833
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
Here's an example of how ArrayList is serialized: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ArrayList.java#L862
We could also use built-in ArrayList here to not deal with the length of array ourselves. But that isn't necessary as the length of array is fixed for the same Recon version.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658985372
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
##########
@@ -234,8 +241,8 @@ public StorageContainerServiceProvider getStorageContainerServiceProvider() {
}
@VisibleForTesting
- public ContainerDBServiceProvider getContainerDBServiceProvider() {
- return containerDBServiceProvider;
+ public ReconContainerMetadataManager getRCMM() {
Review comment:
```suggestion
public ReconContainerMetadataManager getReconContainerMetadataManager() {
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658236785
##########
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:
nit
```suggestion
public class NSSummaryCodec implements Codec<NSSummary> {
```
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658228082
##########
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:
Is there a case when `nsSummaryTable` is null? (like the first time initializing the table)
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658243379
##########
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)
Review comment:
A new method?
```suggestion
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658997710
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
hmm that wasn't my point. My point is the future upgradability. Consider this: if we expanded the bins by 1 slot in the future. NUM_OF_INTS in the executed binary will be 41+1=42. If we are still reading from the **old** RDB, there won't be a 42-th entry in the byte array. So it'll likely throw an exception at this point.
The worse case is, if we also added a new int field to the NSSummary class at the same time indicating a different metric. Then the new code won't know if the 42-th entry is the new bin or new field. Because the new code in `fromPersistedFormat` is using the new `NUM_OF_INTS=42`, not the old `NUM_OF_INTS=41`.
The solution is very simple. Just writing an extra int indicating _the length of array_ right before the content (list of ints) of the array. Even if we don't do much in `fromPersistedFormat` right now, it will help later on that we add logic to get the correct array length. For example, just read the length-indicating int and throw it away. It will help when expand the class.
That's also part of the reason why we use protobuf to deal with all those compatibility details.
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
hmm that wasn't my point. My point is the future upgradability. Consider this: if we expanded the bins by 1 slot in the future. NUM_OF_INTS in the executed binary will be 41+1=42. If we are still reading from the **old** RDB, there won't be a 42-th entry in the byte array. So it'll likely throw an exception at this point.
A worse case is, if we also added a new int field to the NSSummary class at the same time indicating a different metric. Then the new code won't know if the 42-th entry is the new bin or new field. Because the new code in `fromPersistedFormat` is using the new `NUM_OF_INTS=42`, not the old `NUM_OF_INTS=41`.
The solution is very simple. Just writing an extra int indicating _the length of array_ right before the content (list of ints) of the array. Even if we don't do much in `fromPersistedFormat` right now, it will help later on that we add logic to get the correct array length. For example, just read the length-indicating int and throw it away. It will help when expand the class.
That's also part of the reason why we use protobuf to deal with all those compatibility details.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658997710
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
hmm that wasn't my point. I'm point is the future upgradability. Consider this: if we expanded the bins by 1 slot in the future. NUM_OF_INTS in the executed binary will be 41+1=42. If we are still reading from the **old** RDB, there won't be a 42-th entry in the byte array. So it'll likely throw an exception at this point.
The worse case is, if we also added a new int field to the NSSummary class at the same time indicating a different metric. Then the new code won't know if the 42-th entry is the new bin or new field. Because the new code in `fromPersistedFormat` is using the new `NUM_OF_INTS=42`, not the old `NUM_OF_INTS=41`.
The solution is very simple. Just writing an extra int indicating _the length of array_ right before the content (list of ints) of the array. Even if we don't do much in `fromPersistedFormat` right now, it will help later on that we add logic to get the correct array length. For example, just read the length-indicating int and throw it away. It will help when expand the class.
That's also part of the reason why we use protobuf to deal with all those compatibility details.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658246825
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
##########
@@ -110,25 +94,9 @@ public void close() throws Exception {
public void initNewContainerDB(Map<ContainerKeyPrefix, Integer>
Review comment:
TODO: Rename this
```suggestion
public void initContainerTable(Map<ContainerKeyPrefix, Integer>
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658997710
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
hmm that wasn't my point. I'm point is the future upgradability. Consider this: if we expanded the bins by 1 slot in the future. NUM_OF_INTS in the executed binary will be 41+1=42. If we are still reading from the **old** RDB, there won't be a 42-th entry in the byte array. So it'll likely throw an exception at this point.
The worse case is, if we also added a new int field to the NSSummary class at the same time indicating a different metric. Then the new code won't know if the 42-th entry is the new bin or new field. Because the new code in `fromPersistedFormat` is using the new `NUM_OF_INTS=42`, not the old `NUM_OF_INTS=41`.
The solution is very simple. Just writing an extra int indicating _the length of array_ right before the content (list of ints) of the array. Even if we don't do much in `fromPersistedFormat` right now, it will help later on that we add logic to get the correct array length. For example, just read the length-indicating int and throw it away. It will help when expand the DB.
That's also part of the reason why we use protobuf to deal with all those compatibility details.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658275666
##########
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 {
+ 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 {
+ if (table == null) {
+ return;
+ }
+ TableIterator<Object, ? extends KeyValue<Object, Object>>
+ tableIterator = table.iterator();
+ while (tableIterator.hasNext()) {
+ KeyValue<Object, Object> entry = tableIterator.next();
+ table.delete(entry.getKey());
+ }
Review comment:
We could use constant time drop then create CF with RocksDB:
```
RocksDB db;
db.dropColumnFamily();
db.createColumnFamily();
```
but right now these are not exposed in RDBStore on the Ozone side so maybe later.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658982732
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconConstants.java
##########
@@ -58,4 +58,9 @@ private ReconConstants() {
+ PIPELINE_DB_SUFFIX;
public static final String RECON_SCM_NODE_DB =
"recon-node.db";
+ public static final long MAX_FILE_SIZE_UPPER_BOUND = 1125899906842624L;
+ public static final long MIN_FILE_SIZE_UPPER_BOUND = 1024L;
Review comment:
nvm. should be upper bound.
--
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
[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
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658905323
##########
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:
If the table exists in the ReconDBDefinition, then it should never be null after init. We can move _this.nsSummaryTable = NAMESPACE_SUMMARY.getTable(namespaceDbStore);_ to the constructor, and change initNSSummaryTable to do just the truncate operation?
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658795762
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
Even we expand or shrink the num_of_bins in the future, the current approach should have no problem with it. In `NSSummaryCodec`, I calculated the number of ints stored in NSSummary:
`private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;`
then, I calculated the number of bytes needed for (de)serialization:
`final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;`
and
`assert(rawData.length == NUM_OF_INTS * Integer.BYTES);`,
so even if in the future we change the num of bins, we don't need any modification in `NSSummaryCodec`
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r659064408
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/codec/NSSummaryCodec.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.codec;
+
+import org.apache.hadoop.hdds.utils.db.IntegerCodec;
+import org.apache.hadoop.hdds.utils.db.ShortCodec;
+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> {
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ private final Codec<Short> shortCodec = new ShortCodec();
+ // 2 int fields + 41-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES + Short.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ out.write(shortCodec.toPersistedFormat((short)ReconConstants.NUM_OF_BINS));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
+ return out.toByteArray();
+ }
+
+ @Override
+ public NSSummary fromPersistedFormat(byte[] rawData) throws IOException {
+ assert(rawData.length == NUM_OF_INTS * Integer.BYTES + Short.BYTES);
+ DataInputStream in = new DataInputStream(new ByteArrayInputStream(rawData));
+ NSSummary res = new NSSummary();
+ res.setNumOfFiles(in.readInt());
+ res.setSizeOfFiles(in.readInt());
+ short len = in.readShort();
Review comment:
assert here?
```suggestion
short len = in.readShort();
assert(len == (short) NUM_OF_BINS);
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658235816
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/NSSummary.java
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.api.types;
+
+import org.apache.hadoop.ozone.recon.ReconConstants;
+
+import java.util.Arrays;
+
+/**
+ * Class to encapsulate namespace metadata summaries from OM.
+ */
+
+public class NSSummary {
+ private int numOfFiles;
+ private int sizeOfFiles;
+ private int[] fileSizeBucket;
+
+ public NSSummary() {
+ this.numOfFiles = 0;
+ this.sizeOfFiles = 0;
+ // TODO: I read the min is 1024(2^10), max is 1PB(2^50),
+ // so the number of buckets should be 40?
+ this.fileSizeBucket = new int[ReconConstants.NUM_OF_BINS];
Review comment:
Or, we can move `FileSizeCountTask.MAX_FILE_SIZE_UPPER_BOUND` to `ReconConstants` and calculate number of bins from it in `ReconConstants`.
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658928398
##########
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:
Fixed.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r659005833
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
Here's an example of how ArrayList is serialized: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ArrayList.java#L862
We could also use built-in ArrayList here to not deal with the length of array ourselves. But that isn't necessary as the length of array is fixed for one Recon version.
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658216220
##########
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:
I copied the code from the original `initNewContainerDB`, but yeah I can remove it if DB doesn't need re-init
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658248416
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconDBDefinition.java
##########
@@ -18,14 +18,13 @@
*/
package org.apache.hadoop.ozone.recon.spi.impl;
-import org.apache.hadoop.hdds.utils.db.DBColumnFamilyDefinition;
-import org.apache.hadoop.hdds.utils.db.DBDefinition;
-import org.apache.hadoop.hdds.utils.db.IntegerCodec;
-import org.apache.hadoop.hdds.utils.db.LongCodec;
+import org.apache.hadoop.hdds.utils.db.*;
Review comment:
Same here
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658285757
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -458,7 +458,6 @@ public void testGetDatanodes() throws Exception {
.findFirst().orElse(null);
return (datanodeMetadata1 != null &&
datanodeMetadata1.getContainers() == 1 &&
- datanodeMetadata1.getOpenContainers() == 1 &&
Review comment:
sure
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658987533
##########
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> {
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
Review comment:
```suggestion
// 2 int fields + 41-length int array
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r659064408
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/codec/NSSummaryCodec.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.codec;
+
+import org.apache.hadoop.hdds.utils.db.IntegerCodec;
+import org.apache.hadoop.hdds.utils.db.ShortCodec;
+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> {
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ private final Codec<Short> shortCodec = new ShortCodec();
+ // 2 int fields + 41-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES + Short.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ out.write(shortCodec.toPersistedFormat((short)ReconConstants.NUM_OF_BINS));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
+ return out.toByteArray();
+ }
+
+ @Override
+ public NSSummary fromPersistedFormat(byte[] rawData) throws IOException {
+ assert(rawData.length == NUM_OF_INTS * Integer.BYTES + Short.BYTES);
+ DataInputStream in = new DataInputStream(new ByteArrayInputStream(rawData));
+ NSSummary res = new NSSummary();
+ res.setNumOfFiles(in.readInt());
+ res.setSizeOfFiles(in.readInt());
+ short len = in.readShort();
Review comment:
assert here?
```suggestion
short len = in.readShort();
assert(len == NUM_OF_INTS);
```
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/codec/NSSummaryCodec.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.codec;
+
+import org.apache.hadoop.hdds.utils.db.IntegerCodec;
+import org.apache.hadoop.hdds.utils.db.ShortCodec;
+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> {
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ private final Codec<Short> shortCodec = new ShortCodec();
+ // 2 int fields + 41-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES + Short.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ out.write(shortCodec.toPersistedFormat((short)ReconConstants.NUM_OF_BINS));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
+ return out.toByteArray();
+ }
+
+ @Override
+ public NSSummary fromPersistedFormat(byte[] rawData) throws IOException {
+ assert(rawData.length == NUM_OF_INTS * Integer.BYTES + Short.BYTES);
+ DataInputStream in = new DataInputStream(new ByteArrayInputStream(rawData));
+ NSSummary res = new NSSummary();
+ res.setNumOfFiles(in.readInt());
+ res.setSizeOfFiles(in.readInt());
+ short len = in.readShort();
Review comment:
assert here?
```suggestion
short len = in.readShort();
assert(len == (short) NUM_OF_INTS);
```
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658280993
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -458,7 +458,6 @@ public void testGetDatanodes() throws Exception {
.findFirst().orElse(null);
return (datanodeMetadata1 != null &&
datanodeMetadata1.getContainers() == 1 &&
- datanodeMetadata1.getOpenContainers() == 1 &&
Review comment:
I think open container has been tested by TestOpenContainerCount
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658276299
##########
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 {
+ 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 {
+ if (table == null) {
+ return;
+ }
+ TableIterator<Object, ? extends KeyValue<Object, Object>>
+ tableIterator = table.iterator();
+ while (tableIterator.hasNext()) {
+ KeyValue<Object, Object> entry = tableIterator.next();
+ table.delete(entry.getKey());
+ }
+ }
+
+ static DBStore getNewDBStore(OzoneConfiguration configuration) {
+ String dbName = RECON_CONTAINER_KEY_DB + "_" + System.currentTimeMillis();
+ return initializeDBStore(configuration, dbName);
+ }
+
private static DBStore initializeDBStore(OzoneConfiguration configuration,
String dbName) {
DBStore dbStore = null;
try {
dbStore = DBStoreBuilder.createDBStore(configuration,
- new ReconDBDefinition(dbName));
+ new ReconDBDefinition(dbName));
} catch (Exception ex) {
LOG.error("Unable to initialize Recon container metadata store.", ex);
}
return dbStore;
}
- static DBStore getNewDBStore(OzoneConfiguration configuration) {
- String dbName = RECON_CONTAINER_KEY_DB + "_" + System.currentTimeMillis();
- return initializeDBStore(configuration, dbName);
+ public void close() throws Exception {
+ if (this.dbStore != null) {
+ dbStore.close();
+ dbStore = null;
+ }
}
-}
+}
Review comment:
nit: a new line at the end of file
```suggestion
}
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658263783
##########
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?
Review comment:
We will get disk usage on upper API levels.
```suggestion
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658272888
##########
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 : {}",
Review comment:
nit
```suggestion
LOG.info("Last known Recon DB : {}",
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r659064408
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/codec/NSSummaryCodec.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.codec;
+
+import org.apache.hadoop.hdds.utils.db.IntegerCodec;
+import org.apache.hadoop.hdds.utils.db.ShortCodec;
+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> {
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ private final Codec<Short> shortCodec = new ShortCodec();
+ // 2 int fields + 41-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES + Short.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ out.write(shortCodec.toPersistedFormat((short)ReconConstants.NUM_OF_BINS));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
+ return out.toByteArray();
+ }
+
+ @Override
+ public NSSummary fromPersistedFormat(byte[] rawData) throws IOException {
+ assert(rawData.length == NUM_OF_INTS * Integer.BYTES + Short.BYTES);
+ DataInputStream in = new DataInputStream(new ByteArrayInputStream(rawData));
+ NSSummary res = new NSSummary();
+ res.setNumOfFiles(in.readInt());
+ res.setSizeOfFiles(in.readInt());
+ short len = in.readShort();
Review comment:
assert here, maybe?
```suggestion
short len = in.readShort();
assert(len == (short) NUM_OF_BINS);
```
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658213395
##########
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:
I think the `testStoreAndGet` in `org.apache.hadoop.ozone.recon.spi.impl.TestReconNamespaceSummaryManagerImpl` has already covered the codec. Otherwise the test will fail.
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658437420
##########
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();
+ }
Review comment:
`initializeTable` was also called inside constructor.
Currently, `initNSSummaryTable` doesn't have any reference other than testing. But when we do FileSizeCountTask update, there may be a necessity to reinit the namespace table. (Just as `reinitWithNewContainerDataFromOm` did for container-related tables)
--
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
[GitHub] [ozone] yuangu002 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
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658989740
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconConstants.java
##########
@@ -58,4 +58,9 @@ private ReconConstants() {
+ PIPELINE_DB_SUFFIX;
public static final String RECON_SCM_NODE_DB =
"recon-node.db";
+ public static final long MAX_FILE_SIZE_UPPER_BOUND = 1125899906842624L;
+ public static final long MIN_FILE_SIZE_UPPER_BOUND = 1024L;
Review comment:
No. I think it's still upper bound, but the minimum upper bound. Let's say the file size is 10L, which will be rounded to 1024L instead of 16L since 1024L is the minimum upper bound for file size.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658230305
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconControllerModule.java
##########
@@ -38,13 +37,11 @@
import org.apache.hadoop.ozone.recon.recovery.ReconOMMetadataManager;
import org.apache.hadoop.ozone.recon.recovery.ReconOmMetadataManagerImpl;
import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade;
-import org.apache.hadoop.ozone.recon.spi.ContainerDBServiceProvider;
+import org.apache.hadoop.ozone.recon.spi.ReconContainerMetadataManager;
import org.apache.hadoop.ozone.recon.spi.OzoneManagerServiceProvider;
+import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
-import org.apache.hadoop.ozone.recon.spi.impl.ContainerDBServiceProviderImpl;
-import org.apache.hadoop.ozone.recon.spi.impl.OzoneManagerServiceProviderImpl;
-import org.apache.hadoop.ozone.recon.spi.impl.ReconContainerDBProvider;
-import org.apache.hadoop.ozone.recon.spi.impl.StorageContainerServiceProviderImpl;
Review comment:
This is likely done by the IDE, but let's refrain from using wildcard imports.
--
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
[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
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658240303
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
One suggestion here: we should probably write NUM_OF_BINS before writing the array. Just in case we expand or shrink the array in the future. Accordingly we need to handle this in `fromPersistedFormat`.
An even better approach would be to have an `IntegerArrayCodec` that handles the dynamic length of int array?
Or we can use protobuf.
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r659005833
##########
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>{
+
+ private final Codec<Integer> integerCodec = new IntegerCodec();
+ // 2 int fields + 40-length int array
+ private static final int NUM_OF_INTS = 2 + ReconConstants.NUM_OF_BINS;
+
+ @Override
+ public byte[] toPersistedFormat(NSSummary object) throws IOException {
+ final int sizeOfRes = NUM_OF_INTS * Integer.BYTES;
+ ByteArrayOutputStream out = new ByteArrayOutputStream(sizeOfRes);
+ out.write(integerCodec.toPersistedFormat(object.getNumOfFiles()));
+ out.write(integerCodec.toPersistedFormat(object.getSizeOfFiles()));
+ int[] fileSizeBucket = object.getFileSizeBucket();
+ for (int i = 0; i < ReconConstants.NUM_OF_BINS; ++i) {
+ out.write(integerCodec.toPersistedFormat(fileSizeBucket[i]));
+ }
Review comment:
We could also use built-in ArrayList here to not deal with the length of array ourselves. https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ArrayList.java#L862
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658242754
##########
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.
Review comment:
nit
```suggestion
* Interface for DB operations on NSSummary.
```
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658246825
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
##########
@@ -110,25 +94,9 @@ public void close() throws Exception {
public void initNewContainerDB(Map<ContainerKeyPrefix, Integer>
Review comment:
TODO: Rename this
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658284088
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -458,7 +458,6 @@ public void testGetDatanodes() throws Exception {
.findFirst().orElse(null);
return (datanodeMetadata1 != null &&
datanodeMetadata1.getContainers() == 1 &&
- datanodeMetadata1.getOpenContainers() == 1 &&
Review comment:
IMO this can serve as another check, why not just keep it?
--
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
[GitHub] [ozone] smengcl 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
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2366:
URL: https://github.com/apache/ozone/pull/2366#discussion_r658978858
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconConstants.java
##########
@@ -58,4 +58,9 @@ private ReconConstants() {
+ PIPELINE_DB_SUFFIX;
public static final String RECON_SCM_NODE_DB =
"recon-node.db";
+ public static final long MAX_FILE_SIZE_UPPER_BOUND = 1125899906842624L;
+ public static final long MIN_FILE_SIZE_UPPER_BOUND = 1024L;
+ public static final int NUM_OF_BINS = (int)(Math.log(
+ MAX_FILE_SIZE_UPPER_BOUND) / Math.log(2) - Math.log(
+ MIN_FILE_SIZE_UPPER_BOUND) / Math.log(2)) + 1;
Review comment:
```suggestion
// 41 bins
public static final int NUM_OF_BINS = (int) Math.ceil(Math.log(
(double) MAX_FILE_SIZE_UPPER_BOUND / MIN_FILE_SIZE_UPPER_BOUND) /
Math.log(2)) + 1;
```
Add a Math.ceil() just in case
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconConstants.java
##########
@@ -58,4 +58,9 @@ private ReconConstants() {
+ PIPELINE_DB_SUFFIX;
public static final String RECON_SCM_NODE_DB =
"recon-node.db";
+ public static final long MAX_FILE_SIZE_UPPER_BOUND = 1125899906842624L;
+ public static final long MIN_FILE_SIZE_UPPER_BOUND = 1024L;
Review comment:
Do you mean lower bound?
```suggestion
public static final long MIN_FILE_SIZE_LOWER_BOUND = 1024L;
```
--
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