[
https://issues.apache.org/jira/browse/HIVE-24828?focusedWorklogId=566801&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-566801
]
ASF GitHub Bot logged work on HIVE-24828:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 16/Mar/21 09:12
Start Date: 16/Mar/21 09:12
Worklog Time Spent: 10m
Work Description: kishendas commented on a change in pull request #2073:
URL: https://github.com/apache/hive/pull/2073#discussion_r594555075
##########
File path:
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
##########
@@ -3468,6 +3468,15 @@ CompactionResponse compact2(String dbname, String
tableName, String partitionNam
*/
ShowCompactResponse showCompactions() throws TException;
+ /**
+ * Get one latest record of completed compaction by given table/partition.
+ * @param request info on which compaction to retrieve
+ * @return one completed compaction record or null
+ * @throws TException
+ */
+ GetLatestCompactionResponse getLatestCompaction(GetLatestCompactionRequest
request)
Review comment:
Should it be more like getLatestCompactionRecord or
getLatestCompactionInfo ?
##########
File path:
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
##########
@@ -255,6 +235,109 @@ public void testShowCompactions() throws Exception {
});
}
+ @Test
+ public void testGetLatestCompaction() throws Exception {
+ final String dbName = "foo";
+ final String tableName = "bar";
+ final String errorMessage = "Dummy error";
+ addSucceededCompaction(dbName, tableName, CompactionType.MINOR, null,
errorMessage);
+ addFailedCompaction(dbName, tableName, CompactionType.MINOR, null,
errorMessage);
+ GetLatestCompactionRequest rqst = new GetLatestCompactionRequest();
+ rqst.setDbname(dbName);
+ rqst.setTablename(tableName);
+ GetLatestCompactionResponse response =
txnHandler.getLatestCompaction(rqst);
+
+ assertNotNull("Should got a response", response);
Review comment:
"Non-null response" sounds better
##########
File path:
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
##########
@@ -18,27 +18,7 @@
package org.apache.hadoop.hive.metastore.txn;
import org.apache.hadoop.hive.conf.HiveConf;
-import org.apache.hadoop.hive.metastore.api.AbortTxnRequest;
-import org.apache.hadoop.hive.metastore.api.AddDynamicPartitions;
-import org.apache.hadoop.hive.metastore.api.AllocateTableWriteIdsRequest;
-import org.apache.hadoop.hive.metastore.api.AllocateTableWriteIdsResponse;
-import org.apache.hadoop.hive.metastore.api.CommitTxnRequest;
-import org.apache.hadoop.hive.metastore.api.CompactionRequest;
-import org.apache.hadoop.hive.metastore.api.CompactionType;
-import org.apache.hadoop.hive.metastore.api.DataOperationType;
-import org.apache.hadoop.hive.metastore.api.GetOpenTxnsResponse;
-import org.apache.hadoop.hive.metastore.api.LockComponent;
-import org.apache.hadoop.hive.metastore.api.LockLevel;
-import org.apache.hadoop.hive.metastore.api.LockRequest;
-import org.apache.hadoop.hive.metastore.api.LockResponse;
-import org.apache.hadoop.hive.metastore.api.LockState;
-import org.apache.hadoop.hive.metastore.api.LockType;
-import org.apache.hadoop.hive.metastore.api.MetaException;
-import org.apache.hadoop.hive.metastore.api.OpenTxnRequest;
-import org.apache.hadoop.hive.metastore.api.OpenTxnsResponse;
-import org.apache.hadoop.hive.metastore.api.ShowCompactRequest;
-import org.apache.hadoop.hive.metastore.api.ShowCompactResponse;
-import org.apache.hadoop.hive.metastore.api.ShowCompactResponseElement;
+import org.apache.hadoop.hive.metastore.api.*;
Review comment:
We discourage use of .* . Please revert this change.
##########
File path:
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
##########
@@ -255,6 +235,109 @@ public void testShowCompactions() throws Exception {
});
}
+ @Test
+ public void testGetLatestCompaction() throws Exception {
+ final String dbName = "foo";
+ final String tableName = "bar";
+ final String errorMessage = "Dummy error";
+ addSucceededCompaction(dbName, tableName, CompactionType.MINOR, null,
errorMessage);
+ addFailedCompaction(dbName, tableName, CompactionType.MINOR, null,
errorMessage);
+ GetLatestCompactionRequest rqst = new GetLatestCompactionRequest();
+ rqst.setDbname(dbName);
+ rqst.setTablename(tableName);
+ GetLatestCompactionResponse response =
txnHandler.getLatestCompaction(rqst);
+
+ assertNotNull("Should got a response", response);
+ assertEquals("Should got only one compaction", 1,
response.getCompactionsSize());
Review comment:
"Expecting a single compaction record" ?
##########
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
##########
@@ -392,6 +387,39 @@ public void testGetValidWriteIds() throws TException {
Assert.assertEquals(writeIdList.getMinOpenWriteId().longValue(), 2);
}
+ @Test
+ public void testGetLatestCompaction() throws Exception {
+ final String dbName = "mydb";
+ final String tblName = "mytable";
+ Database db = new DatabaseBuilder().setName(dbName).build(conf);
+ db.unsetCatalogName();
+ client.createDatabase(db);
+
+ Table tbl = new TableBuilder().setDbName(dbName).setTableName(tblName)
+ .addCol("id", "int").addCol("name", "string")
+ .setType(TableType.MANAGED_TABLE.name()).build(conf);
+ client.createTable(tbl);
+ tbl = client.getTable(dbName, tblName);
+
+ client.compact2(tbl.getDbName(), tbl.getTableName(), null,
CompactionType.MINOR, new HashMap<>());
+ OptionalCompactionInfoStruct optionalCi =
client.findNextCompact("myworker");
+ client.markCleaned(optionalCi.getCi());
+
+ GetLatestCompactionRequest rqst = new GetLatestCompactionRequest();
+ rqst.setDbname(dbName);
+ rqst.setTablename(tblName);
+ GetLatestCompactionResponse response = client.getLatestCompaction(rqst);
+
+ Assert.assertNotNull("Response should not be null", response);
+ Assert.assertEquals(1, response.getCompactionsSize());
+ Assert.assertEquals(dbName, response.getDbname());
+ Assert.assertEquals(tblName, response.getTablename());
+ LatestCompactionInfo lci = response.getCompactions().get(0);
+ Assert.assertEquals("Compaction ID should be 1", 1, lci.getId());
Review comment:
Please remove all the redundant assert messages.
##########
File path:
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
##########
@@ -331,14 +414,38 @@ private void addFailedCompaction(String dbName, String
tableName, CompactionType
CompactionRequest rqst;
CompactionInfo ci;
rqst = new CompactionRequest(dbName, tableName, type);
- rqst.setPartitionname(partitionName);
+ if (partitionName != null) rqst.setPartitionname(partitionName);
Review comment:
We use flower brackets, even for simple if statements.
Hope you are using the Apache Hive Java template in your IDE ?
##########
File path:
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
##########
@@ -255,6 +235,109 @@ public void testShowCompactions() throws Exception {
});
}
+ @Test
+ public void testGetLatestCompaction() throws Exception {
+ final String dbName = "foo";
+ final String tableName = "bar";
+ final String errorMessage = "Dummy error";
+ addSucceededCompaction(dbName, tableName, CompactionType.MINOR, null,
errorMessage);
+ addFailedCompaction(dbName, tableName, CompactionType.MINOR, null,
errorMessage);
+ GetLatestCompactionRequest rqst = new GetLatestCompactionRequest();
+ rqst.setDbname(dbName);
+ rqst.setTablename(tableName);
+ GetLatestCompactionResponse response =
txnHandler.getLatestCompaction(rqst);
+
+ assertNotNull("Should got a response", response);
+ assertEquals("Should got only one compaction", 1,
response.getCompactionsSize());
+ LatestCompactionInfo lci = response.getCompactions().get(0);
+ assertEquals("Should return the first succeeded compaction", 1,
lci.getId());
+ assertNull("Partitionname should be null for non partitioned table",
lci.getPartitionname());
+ assertEquals(CompactionType.MINOR, lci.getType());
+ }
+
+ @Test
+ public void testGetLatestCompactionPartition() throws Exception {
+ final String dbName = "foo";
+ final String tableName = "bar";
+ final String partitionName = "ds=today";
+ final String errorMessage = "Dummy error";
+ addSucceededCompaction(dbName, tableName, CompactionType.MINOR,
partitionName, errorMessage);
+ addFailedCompaction(dbName, tableName, CompactionType.MINOR,
partitionName, errorMessage);
+ GetLatestCompactionRequest rqst = new GetLatestCompactionRequest();
+ rqst.setDbname(dbName);
+ rqst.setTablename(tableName);
+ rqst.addToPartitionnames(partitionName);
+ GetLatestCompactionResponse response =
txnHandler.getLatestCompaction(rqst);
+
+ assertNotNull("Should got a response", response);
+ assertEquals("Should got only one compaction", 1,
response.getCompactionsSize());
+ LatestCompactionInfo lci = response.getCompactions().get(0);
+ assertEquals("Should return the first succeeded compaction", 1,
lci.getId());
+ assertEquals("Should return same partition name as the request",
partitionName, lci.getPartitionname());
+ assertEquals(CompactionType.MINOR, lci.getType());
+
+ final String anotherPartitionName = "ds=yesterday";
+ addWaitingForCleaningCompaction(dbName, tableName, CompactionType.MINOR,
anotherPartitionName, errorMessage);
+ rqst.addToPartitionnames(anotherPartitionName);
+ response = txnHandler.getLatestCompaction(rqst);
+
+ assertNotNull("Should got a response", response);
+ assertEquals("Should got two compactions", 2,
response.getCompactionsSize());
Review comment:
"Expecting two compaction records"
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -3585,6 +3532,77 @@ public ShowCompactResponse
showCompact(ShowCompactRequest rqst) throws MetaExcep
}
}
+ @RetrySemantics.ReadOnly
+ public GetLatestCompactionResponse
getLatestCompaction(GetLatestCompactionRequest rqst) throws MetaException {
+ GetLatestCompactionResponse response = new GetLatestCompactionResponse();
+ response.setDbname(rqst.getDbname());
+ response.setTablename(rqst.getTablename());
+ Connection dbConn = null;
+ PreparedStatement pst = null;
+ try {
+ try {
+ dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
+
+ List<String> params = new ArrayList<>();
+ StringBuilder sb = new StringBuilder()
Review comment:
Can you please add a comment on what this query is doing ?
##########
File path:
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
##########
@@ -53,12 +33,12 @@
import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertFalse;
-import static junit.framework.Assert.assertNotNull;
-import static junit.framework.Assert.assertNull;
Review comment:
Are you sure these changes in import is required ?
##########
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
##########
@@ -392,6 +387,39 @@ public void testGetValidWriteIds() throws TException {
Assert.assertEquals(writeIdList.getMinOpenWriteId().longValue(), 2);
}
+ @Test
+ public void testGetLatestCompaction() throws Exception {
+ final String dbName = "mydb";
+ final String tblName = "mytable";
+ Database db = new DatabaseBuilder().setName(dbName).build(conf);
+ db.unsetCatalogName();
+ client.createDatabase(db);
+
+ Table tbl = new TableBuilder().setDbName(dbName).setTableName(tblName)
+ .addCol("id", "int").addCol("name", "string")
+ .setType(TableType.MANAGED_TABLE.name()).build(conf);
+ client.createTable(tbl);
+ tbl = client.getTable(dbName, tblName);
+
+ client.compact2(tbl.getDbName(), tbl.getTableName(), null,
CompactionType.MINOR, new HashMap<>());
+ OptionalCompactionInfoStruct optionalCi =
client.findNextCompact("myworker");
+ client.markCleaned(optionalCi.getCi());
+
+ GetLatestCompactionRequest rqst = new GetLatestCompactionRequest();
+ rqst.setDbname(dbName);
+ rqst.setTablename(tblName);
+ GetLatestCompactionResponse response = client.getLatestCompaction(rqst);
+
+ Assert.assertNotNull("Response should not be null", response);
Review comment:
You don't have add a message, if it's self explanatory.
##########
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##########
@@ -3585,6 +3532,77 @@ public ShowCompactResponse
showCompact(ShowCompactRequest rqst) throws MetaExcep
}
}
+ @RetrySemantics.ReadOnly
+ public GetLatestCompactionResponse
getLatestCompaction(GetLatestCompactionRequest rqst) throws MetaException {
+ GetLatestCompactionResponse response = new GetLatestCompactionResponse();
+ response.setDbname(rqst.getDbname());
+ response.setTablename(rqst.getTablename());
+ Connection dbConn = null;
+ PreparedStatement pst = null;
+ try {
+ try {
+ dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
+
+ List<String> params = new ArrayList<>();
+ StringBuilder sb = new StringBuilder()
+ .append("SELECT * FROM (")
+ .append(" SELECT")
+ .append(" \"CC_ID\", \"CC_DATABASE\", \"CC_TABLE\",
\"CC_PARTITION\", \"CC_TYPE\"")
+ .append(" FROM \"COMPLETED_COMPACTIONS\"")
+ .append(" WHERE \"CC_STATE\" = " + quoteChar(SUCCEEDED_STATE))
+ .append(" UNION ALL")
+ .append(" SELECT")
+ .append(" \"CQ_ID\" AS \"CC_ID\", \"CQ_DATABASE\" AS
\"CC_DATABASE\"")
+ .append(" ,\"CQ_TABLE\" AS \"CC_TABLE\", \"CQ_PARTITION\" AS
\"CC_PARTITION\"")
+ .append(" ,\"CQ_TYPE\" AS \"CC_TYPE\"")
+ .append(" FROM \"COMPACTION_QUEUE\"")
+ .append(" WHERE \"CQ_STATE\" = " +
quoteChar(READY_FOR_CLEANING))
+ .append(") AS compactions ")
+ .append(" WHERE \"CC_DATABASE\"=? AND \"CC_TABLE\"=?");
+ params.add(rqst.getDbname());
+ params.add(rqst.getTablename());
+ if (rqst.getPartitionnames() != null) {
+ sb.append(" AND \"CC_PARTITION\" IN (");
+ sb.append(String.join(",",
+ Collections.nCopies(rqst.getPartitionnamesSize(), "?")));
+ sb.append(")");
+ params.addAll(rqst.getPartitionnames());
+ } else {
+ sb.append(" AND \"CC_PARTITION\" IS NULL");
+ }
+ sb.append(" ORDER BY \"CC_ID\" DESC");
+
+ pst = sqlGenerator.prepareStmtWithParameters(dbConn, sb.toString(),
params);
+ LOG.debug("Going to execute query <" + sb.toString() + ">");
+ ResultSet rs = pst.executeQuery();
+ HashSet<String> partitionSet = new HashSet<>();
+ while (rs.next()) {
+ LatestCompactionInfo lci = new LatestCompactionInfo();
+ lci.setId(rs.getLong(1));
+ String partition = rs.getString(4);
+ if (!rs.wasNull()) lci.setPartitionname(partition);
Review comment:
Please use flower brackets, even for simple if statements.
##########
File path:
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
##########
@@ -22,21 +22,10 @@
import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.CATALOG_DB_SEPARATOR;
import org.apache.hadoop.hive.metastore.MetaStoreFilterHook;
-import org.apache.hadoop.hive.metastore.api.Catalog;
-import org.apache.hadoop.hive.metastore.api.Database;
-import org.apache.hadoop.hive.metastore.api.MetaException;
-import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
-import org.apache.hadoop.hive.metastore.api.Partition;
-import org.apache.hadoop.hive.metastore.api.PartitionSpec;
-import org.apache.hadoop.hive.metastore.api.Table;
-import org.apache.hadoop.hive.metastore.api.TableMeta;
-import org.apache.hadoop.hive.metastore.api.ShowCompactResponseElement;
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import org.apache.hadoop.hive.metastore.api.*;
+
+import java.util.*;
Review comment:
Please do not use .*;
----------------------------------------------------------------
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:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 566801)
Time Spent: 1h 50m (was: 1h 40m)
> [HMS] Provide new HMS API to return latest committed compaction record for a
> given table
> ----------------------------------------------------------------------------------------
>
> Key: HIVE-24828
> URL: https://issues.apache.org/jira/browse/HIVE-24828
> Project: Hive
> Issue Type: Sub-task
> Components: HiveServer2
> Reporter: Kishen Das
> Assignee: Yu-Wen Lai
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> We need a new HMS API to return the latest committed compaction record for a
> given table. This can be used by a remote cache to decide whether a given
> table's file metadata has been compacted or not, in order to decide whether
> file metadata has to be refreshed from the file system before serving or it
> can serve the current data from the cache.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
