You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/04/20 13:11:31 UTC
[GitHub] [hive] aasha opened a new pull request #987: HIVE-23235 Fix checkpointing for ORC Tables
aasha opened a new pull request #987:
URL: https://github.com/apache/hive/pull/987
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] github-actions[bot] closed pull request #987: HIVE-23235 Fix checkpointing for ORC Tables
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #987:
URL: https://github.com/apache/hive/pull/987
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pkumarsinha commented on a change in pull request #987: HIVE-23235 Fix checkpointing for ORC Tables
Posted by GitBox <gi...@apache.org>.
pkumarsinha commented on a change in pull request #987:
URL: https://github.com/apache/hive/pull/987#discussion_r411869774
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
##########
@@ -383,10 +385,17 @@ private void doRegularCopyOnce(FileSystem sourceFs, List<Path> srcList,
throw new IOException(e);
}
} else {
+ deleteSubDirs(destinationFs, destination);
FileUtil.copy(sourceFs, paths, destinationFs, destination, false, true, hiveConf);
}
}
+ private void deleteSubDirs(FileSystem destinationFs, Path destination) throws IOException {
+ for (FileStatus status : destinationFs.listStatus(destination)) {
Review comment:
Why can't we delete the destination and recreate the dest folder if required?
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcidTables.java
##########
@@ -761,6 +761,104 @@ public void testCheckPointingDataDumpFailureRegularCopy() throws Throwable {
.verifyResults(new String[]{"11", "21"});
}
+ @Test
+ public void testCheckPointingDataDumpFailureORCTableRegularCopy() throws Throwable {
+ WarehouseInstance.Tuple bootstrapDump = primary.run("use " + primaryDbName)
+ .run("CREATE TABLE t1(a int) clustered by (a) into 2 buckets" +
+ " stored as orc TBLPROPERTIES ('transactional'='true')")
+ .run("CREATE TABLE t2(a string) STORED AS TEXTFILE")
Review comment:
Add one empty table too.
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcidTables.java
##########
@@ -761,6 +761,104 @@ public void testCheckPointingDataDumpFailureRegularCopy() throws Throwable {
.verifyResults(new String[]{"11", "21"});
}
+ @Test
+ public void testCheckPointingDataDumpFailureORCTableRegularCopy() throws Throwable {
+ WarehouseInstance.Tuple bootstrapDump = primary.run("use " + primaryDbName)
+ .run("CREATE TABLE t1(a int) clustered by (a) into 2 buckets" +
+ " stored as orc TBLPROPERTIES ('transactional'='true')")
+ .run("CREATE TABLE t2(a string) STORED AS TEXTFILE")
+ .run("insert into t1 values (1)")
+ .run("insert into t1 values (2)")
+ .run("insert into t1 values (3)")
+ .run("insert into t2 values (11)")
+ .run("insert into t2 values (21)")
+ .dump(primaryDbName);
+ FileSystem fs = new Path(bootstrapDump.dumpLocation).getFileSystem(conf);
+ Path dumpPath = new Path(bootstrapDump.dumpLocation, ReplUtils.REPL_HIVE_BASE_DIR);
+ assertTrue(fs.exists(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString())));
+ Path metadataPath = new Path(dumpPath, EximUtil.METADATA_PATH_NAME);
+ long modifiedTimeMetadata = fs.getFileStatus(metadataPath).getModificationTime();
+ Path dataPath = new Path(dumpPath, EximUtil.DATA_PATH_NAME);
+ Path dbPath = new Path(dataPath, primaryDbName.toLowerCase());
+ Path tablet1Path = new Path(dbPath, "t1");
+ Path tablet2Path = new Path(dbPath, "t2");
+ //Delete dump ack and t2 data, metadata should be rewritten, data should be same for t1 but rewritten for t2
+ fs.delete(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString()), true);
+ assertFalse(fs.exists(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString())));
+ FileStatus[] statuses = fs.listStatus(tablet2Path);
+ //Delete t2 data
+ fs.delete(statuses[0].getPath(), true);
+ long modifiedTimeTable1 = fs.getFileStatus(tablet1Path).getModificationTime();
+ long modifiedTimeTable1CopyFile = fs.listStatus(tablet1Path)[0].getModificationTime();
+ long modifiedTimeTable2 = fs.getFileStatus(tablet2Path).getModificationTime();
+ //Do another dump. It should only dump table t2. Modification time of table t1 should be same while t2 is greater
+ WarehouseInstance.Tuple nextDump = primary.dump(primaryDbName);
+ assertEquals(nextDump.dumpLocation, bootstrapDump.dumpLocation);
+ assertTrue(fs.exists(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString())));
+ //File is copied again as we are using regular copy
+ assertTrue(modifiedTimeTable1 < fs.getFileStatus(tablet1Path).getModificationTime());
+ assertTrue(modifiedTimeTable1CopyFile < fs.listStatus(tablet1Path)[0].getModificationTime());
+ assertTrue(modifiedTimeTable2 < fs.getFileStatus(tablet2Path).getModificationTime());
+ assertTrue(modifiedTimeMetadata < fs.getFileStatus(metadataPath).getModificationTime());
+ replica.load(replicatedDbName, primaryDbName)
+ .run("select * from " + replicatedDbName + ".t1")
+ .verifyResults(new String[] {"1", "2", "3"})
+ .run("select * from " + replicatedDbName + ".t2")
+ .verifyResults(new String[]{"11", "21"});
+ }
+
+ @Test
+ public void testCheckPointingDataDumpFailureORCTableDistcpCopy() throws Throwable {
+ //Distcp copy
+ List<String> dumpClause = Arrays.asList(
+ "'" + HiveConf.ConfVars.HIVE_EXEC_COPYFILE_MAXSIZE.varname + "'='1'",
+ "'" + HiveConf.ConfVars.HIVE_IN_TEST.varname + "'='false'",
+ "'" + HiveConf.ConfVars.HIVE_EXEC_COPYFILE_MAXNUMFILES.varname + "'='0'",
+ "'" + HiveConf.ConfVars.HIVE_DISTCP_DOAS_USER.varname + "'='"
+ + UserGroupInformation.getCurrentUser().getUserName() + "'");
+ WarehouseInstance.Tuple bootstrapDump = primary.run("use " + primaryDbName)
+ .run("CREATE TABLE t1(a int) clustered by (a) into 2 buckets" +
+ " stored as orc TBLPROPERTIES ('transactional'='true')")
+ .run("CREATE TABLE t2(a string) STORED AS TEXTFILE")
Review comment:
t2 should be ORC table as the tests are mainly dealing with overwrite to it.
##########
File path: itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcidTables.java
##########
@@ -761,6 +761,104 @@ public void testCheckPointingDataDumpFailureRegularCopy() throws Throwable {
.verifyResults(new String[]{"11", "21"});
}
+ @Test
+ public void testCheckPointingDataDumpFailureORCTableRegularCopy() throws Throwable {
+ WarehouseInstance.Tuple bootstrapDump = primary.run("use " + primaryDbName)
+ .run("CREATE TABLE t1(a int) clustered by (a) into 2 buckets" +
+ " stored as orc TBLPROPERTIES ('transactional'='true')")
+ .run("CREATE TABLE t2(a string) STORED AS TEXTFILE")
+ .run("insert into t1 values (1)")
+ .run("insert into t1 values (2)")
+ .run("insert into t1 values (3)")
+ .run("insert into t2 values (11)")
+ .run("insert into t2 values (21)")
+ .dump(primaryDbName);
+ FileSystem fs = new Path(bootstrapDump.dumpLocation).getFileSystem(conf);
+ Path dumpPath = new Path(bootstrapDump.dumpLocation, ReplUtils.REPL_HIVE_BASE_DIR);
+ assertTrue(fs.exists(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString())));
+ Path metadataPath = new Path(dumpPath, EximUtil.METADATA_PATH_NAME);
+ long modifiedTimeMetadata = fs.getFileStatus(metadataPath).getModificationTime();
+ Path dataPath = new Path(dumpPath, EximUtil.DATA_PATH_NAME);
+ Path dbPath = new Path(dataPath, primaryDbName.toLowerCase());
+ Path tablet1Path = new Path(dbPath, "t1");
+ Path tablet2Path = new Path(dbPath, "t2");
+ //Delete dump ack and t2 data, metadata should be rewritten, data should be same for t1 but rewritten for t2
+ fs.delete(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString()), true);
+ assertFalse(fs.exists(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString())));
+ FileStatus[] statuses = fs.listStatus(tablet2Path);
+ //Delete t2 data
+ fs.delete(statuses[0].getPath(), true);
+ long modifiedTimeTable1 = fs.getFileStatus(tablet1Path).getModificationTime();
+ long modifiedTimeTable1CopyFile = fs.listStatus(tablet1Path)[0].getModificationTime();
+ long modifiedTimeTable2 = fs.getFileStatus(tablet2Path).getModificationTime();
+ //Do another dump. It should only dump table t2. Modification time of table t1 should be same while t2 is greater
+ WarehouseInstance.Tuple nextDump = primary.dump(primaryDbName);
+ assertEquals(nextDump.dumpLocation, bootstrapDump.dumpLocation);
+ assertTrue(fs.exists(new Path(dumpPath, DUMP_ACKNOWLEDGEMENT.toString())));
+ //File is copied again as we are using regular copy
+ assertTrue(modifiedTimeTable1 < fs.getFileStatus(tablet1Path).getModificationTime());
+ assertTrue(modifiedTimeTable1CopyFile < fs.listStatus(tablet1Path)[0].getModificationTime());
+ assertTrue(modifiedTimeTable2 < fs.getFileStatus(tablet2Path).getModificationTime());
Review comment:
In regular copy overwrite case, we are not deleting the table parent folder t2, how is this passing?
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] aasha commented on a change in pull request #987: HIVE-23235 Fix checkpointing for ORC Tables
Posted by GitBox <gi...@apache.org>.
aasha commented on a change in pull request #987:
URL: https://github.com/apache/hive/pull/987#discussion_r411930184
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
##########
@@ -383,10 +385,17 @@ private void doRegularCopyOnce(FileSystem sourceFs, List<Path> srcList,
throw new IOException(e);
}
} else {
+ deleteSubDirs(destinationFs, destination);
FileUtil.copy(sourceFs, paths, destinationFs, destination, false, true, hiveConf);
}
}
+ private void deleteSubDirs(FileSystem destinationFs, Path destination) throws IOException {
+ for (FileStatus status : destinationFs.listStatus(destination)) {
Review comment:
We need the destination folder to be present. Thats an extra FS call if we delete first and recreate again.
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] github-actions[bot] commented on pull request #987: HIVE-23235 Fix checkpointing for ORC Tables
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #987:
URL: https://github.com/apache/hive/pull/987#issuecomment-648503920
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org