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 2020/04/02 06:42:59 UTC
[GitHub] [hadoop-ozone] vivekratnavel opened a new pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
vivekratnavel opened a new pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753
…container was present in.
## What changes were proposed in this pull request?
- Create a new table in SQLite to keep track of container replica history
- Include the last known datanodes of a missing container in API (/api/v1/containers/missing)
- Add a new API (/api/v1/containers/{id}/replicaHistory) to get complete history of container replicas.
- Make changes in the UI to display datanode information in missing containers page.
- Add unit and acceptance tests.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3237
## How was this patch tested?
- Unit tests, acceptance tests and manual test via docker-compose.
To test the change,
`cd hadoop-ozone/dist/target/ozone-0.6.0-SNAPSHOT/compose/ozone`
Add the following lines to `docker-config` for faster testing
```
OZONE-SITE.XML_ozone.scm.dead.node.interval=2m
OZONE-SITE.XML_ozone.scm.stale.node.interval=1m
```
`docker-compose up -d`
`docker-compose exec om bash`
```
ozone freon rk --replicationType=RATIS --numOfVolumes=10 --numOfBuckets=10 --numOfKeys=10 --factor=ONE --numOfThreads=20
```
Now, exit the shell and bring down datanode
`docker ps`
`docker kill <containerid>`
Navigate to Recon UI and wait for a few minutes to see missing container alert in the Overview page. Clicking on the alert box should take you to the missing containers page:
<img width="1680" alt="Screen Shot 2020-04-01 at 10 07 06 PM" src="https://user-images.githubusercontent.com/1051198/78218221-44f19c00-7472-11ea-9ba2-e2b742bd6bff.png">
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx merged pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] vivekratnavel commented on issue #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on issue #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753#issuecomment-607913312
@avijayanhwx @elek Please review
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753#discussion_r402509841
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerSchemaManager.java
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.persistence;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerHistoryTable.CONTAINER_HISTORY;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import org.hadoop.ozone.recon.schema.ContainerSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerHistoryDao;
+import org.hadoop.ozone.recon.schema.tables.daos.MissingContainersDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerHistory;
+import org.hadoop.ozone.recon.schema.tables.pojos.MissingContainers;
+import org.jooq.DSLContext;
+import org.jooq.Record2;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Provide a high level API to access the Container Schema.
+ */
+@Singleton
+public class ContainerSchemaManager {
+ private ContainerHistoryDao containerHistoryDao;
+ private MissingContainersDao missingContainersDao;
+ private ContainerSchemaDefinition containerSchemaDefinition;
+
+ @Inject
+ public ContainerSchemaManager(ContainerHistoryDao containerHistoryDao,
+ ContainerSchemaDefinition containerSchemaDefinition,
+ MissingContainersDao missingContainersDao) {
+ this.containerHistoryDao = containerHistoryDao;
+ this.missingContainersDao = missingContainersDao;
+ this.containerSchemaDefinition = containerSchemaDefinition;
+ }
+
+ public void addMissingContainer(long containerID, long time) {
+ MissingContainers record = new MissingContainers(containerID, time);
+ missingContainersDao.insert(record);
+ }
+
+ public List<MissingContainers> getAllMissingContainers() {
+ return missingContainersDao.findAll();
+ }
+
+ public boolean isMissingContainer(long containerID) {
+ return missingContainersDao.existsById(containerID);
+ }
+
+ public void deleteMissingContainer(long containerID) {
+ missingContainersDao.deleteById(containerID);
+ }
+
+ public void upsertContainerHistory(long containerID, String datanode,
+ long time) {
+ DSLContext ctx = containerSchemaDefinition.getDSLContext();
+ Record2<Long, String> record =
+ ctx.newRecord(
+ CONTAINER_HISTORY.CONTAINER_ID,
+ CONTAINER_HISTORY.DATANODE_HOST).value1(containerID).value2(datanode);
+ ContainerHistory newRecord = new ContainerHistory();
+ newRecord.setContainerId(containerID);
+ newRecord.setDatanodeHost(datanode);
+ newRecord.setLastReportTimestamp(time);
+ if (containerHistoryDao.existsById(record)) {
+ newRecord.setFirstReportTimestamp(
+ containerHistoryDao.findById(record).getFirstReportTimestamp()
Review comment:
Here we are doing 2 lookups - first 'existsById' and then 'findById'. Can we refactor this by doing only one lookup instead?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753#discussion_r402516043
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerSchemaManager.java
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.persistence;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerHistoryTable.CONTAINER_HISTORY;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import org.hadoop.ozone.recon.schema.ContainerSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerHistoryDao;
+import org.hadoop.ozone.recon.schema.tables.daos.MissingContainersDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerHistory;
+import org.hadoop.ozone.recon.schema.tables.pojos.MissingContainers;
+import org.jooq.DSLContext;
+import org.jooq.Record2;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Provide a high level API to access the Container Schema.
+ */
+@Singleton
+public class ContainerSchemaManager {
+ private ContainerHistoryDao containerHistoryDao;
+ private MissingContainersDao missingContainersDao;
+ private ContainerSchemaDefinition containerSchemaDefinition;
+
+ @Inject
+ public ContainerSchemaManager(ContainerHistoryDao containerHistoryDao,
+ ContainerSchemaDefinition containerSchemaDefinition,
+ MissingContainersDao missingContainersDao) {
+ this.containerHistoryDao = containerHistoryDao;
+ this.missingContainersDao = missingContainersDao;
+ this.containerSchemaDefinition = containerSchemaDefinition;
+ }
+
+ public void addMissingContainer(long containerID, long time) {
+ MissingContainers record = new MissingContainers(containerID, time);
+ missingContainersDao.insert(record);
+ }
+
+ public List<MissingContainers> getAllMissingContainers() {
+ return missingContainersDao.findAll();
+ }
+
+ public boolean isMissingContainer(long containerID) {
+ return missingContainersDao.existsById(containerID);
+ }
+
+ public void deleteMissingContainer(long containerID) {
+ missingContainersDao.deleteById(containerID);
+ }
+
+ public void upsertContainerHistory(long containerID, String datanode,
+ long time) {
+ DSLContext ctx = containerSchemaDefinition.getDSLContext();
+ Record2<Long, String> record =
+ ctx.newRecord(
+ CONTAINER_HISTORY.CONTAINER_ID,
+ CONTAINER_HISTORY.DATANODE_HOST).value1(containerID).value2(datanode);
+ ContainerHistory newRecord = new ContainerHistory();
+ newRecord.setContainerId(containerID);
+ newRecord.setDatanodeHost(datanode);
+ newRecord.setLastReportTimestamp(time);
+ if (containerHistoryDao.existsById(record)) {
+ newRecord.setFirstReportTimestamp(
+ containerHistoryDao.findById(record).getFirstReportTimestamp()
+ );
+ containerHistoryDao.update(newRecord);
+ } else {
+ newRecord.setFirstReportTimestamp(time);
+ containerHistoryDao.insert(newRecord);
+ }
+ }
+
+ public List<ContainerHistory> getAllContainerHistory(long containerID) {
+ return containerHistoryDao.fetchByContainerId(containerID);
+ }
+
+ public List<ContainerHistory> getLatestContainerHistory(long containerID,
+ int limit) {
+ // Get container history sorted in descending order of timestamp
+ List<ContainerHistory> containerHistory =
Review comment:
Can we try and use ORDER BY and LIMIT on the DB side (using SELECT API maybe) instead of client side sorting and filtering?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753#discussion_r402370121
##########
File path: hadoop-ozone/recon-codegen/src/main/java/org/hadoop/ozone/recon/schema/ContainerSchemaDefinition.java
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.hadoop.ozone.recon.schema;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import org.jooq.DSLContext;
+import org.jooq.impl.DSL;
+import org.jooq.impl.SQLDataType;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.SQLException;
+
+/**
+ * Class used to create tables that are required for tracking containers.
+ */
+@Singleton
+public class ContainerSchemaDefinition implements ReconSchemaDefinition {
+
+ public static final String CONTAINER_HISTORY_TABLE_NAME =
+ "container_history";
+ public static final String MISSING_CONTAINERS_TABLE_NAME =
+ "missing_containers";
+ private static final String CONTAINER_ID = "container_id";
+ private final DataSource dataSource;
+ private DSLContext dslContext;
+
+ @Inject
+ ContainerSchemaDefinition(DataSource dataSource) {
+ this.dataSource = dataSource;
+ }
+
+ @Override
+ public void initializeSchema() throws SQLException {
+ Connection conn = dataSource.getConnection();
+ dslContext = DSL.using(conn);
+ createContainerHistoryTable();
+ createMissingContainersTable();
+ }
+
+ /**
+ * Create the Container History table.
+ */
+ private void createContainerHistoryTable() {
+ dslContext.createTableIfNotExists(CONTAINER_HISTORY_TABLE_NAME)
+ .column(CONTAINER_ID, SQLDataType.BIGINT)
+ .column("datanode_host", SQLDataType.VARCHAR(1024))
+ .column("first_report_timestamp", SQLDataType.BIGINT)
+ .column("last_report_timestamp", SQLDataType.BIGINT)
+ .constraint(DSL.constraint("pk_container_id_datanode_host")
+ .primaryKey(CONTAINER_ID, "datanode_host"))
+ .execute();
+ }
+
+ /**
+ * Create the Missing Containers table.
+ */
+ private void createMissingContainersTable() {
+ dslContext.createTableIfNotExists(MISSING_CONTAINERS_TABLE_NAME)
+ .column(CONTAINER_ID, SQLDataType.BIGINT)
+ .column("missing_since", SQLDataType.BIGINT)
+ .constraint(DSL.constraint("pk_container_id")
+ .primaryKey(CONTAINER_ID))
+ .execute();
+ }
+
+ public DSLContext getDSLContext() {
Review comment:
I believe DSLContext is shared across tables. We may be able to inject this. But, I am OK with having the getter inside just this schema definition.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #753: HDDS-3237. Recon should provide the list of datanodes that a missing …
URL: https://github.com/apache/hadoop-ozone/pull/753#discussion_r402502267
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestMissingContainerTask.java
##########
@@ -58,10 +60,16 @@ public void testRun() throws Exception {
ReconTaskSchemaDefinition.class);
taskSchemaDefinition.initializeSchema();
- UtilizationSchemaDefinition schemaDefinition =
- getInjector().getInstance(UtilizationSchemaDefinition.class);
+ ContainerSchemaDefinition schemaDefinition =
+ getInjector().getInstance(ContainerSchemaDefinition.class);
schemaDefinition.initializeSchema();
Review comment:
Instead of initializing a schema definition as and when needed in tests, we can create a test harness that creates a test sql DB, and provides access to any DAO that the test may need. This will remove the need for a unit test to know what DAO is needed somewhere deep inside the flow, and the code will be much cleaner. Of course, this will be a different JIRA work item.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org