HIVE-21731 : Hive import fails, post upgrade of source 3.0 cluster, to a target 4.0 cluster with strict managed table set to true. #628
Conversation
...ve-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationWithTableMigration.java
Show resolved
Hide resolved
...ve-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationWithTableMigration.java
Show resolved
Hide resolved
|
|
||
| public void enableAcid() { | ||
| hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY, true); | ||
| hiveConf.setVar(HiveConf.ConfVars.HIVE_TXN_MANAGER, "org.apache.hadoop.hive.ql.lockmgr.DbTxnManager"); |
There was a problem hiding this comment.
Is it enough to do this? I think, TxnDbUtil.prepDb should be done, otherwise, it won't work. As the modified test suit is migration case, replica warehouse instance init has initialised it and so it worked. But won't work otherwise.
There was a problem hiding this comment.
its working fine for this test ..i could see the table getting converted to acid and bootstrap happening
There was a problem hiding this comment.
It is working for this test because, replica warehouse is acid enabled and it initialises the derby DB which is shared by both warehouse instances. But, if both primary and replica are acid disabled, then this won't work if you dynamically change this config.
There was a problem hiding this comment.
ok ..when ever its required ..it can be done ..currently i don't see any reason to add extra init if its not required
There was a problem hiding this comment.
sure. Can we add a comment about this in this method? So that later someone need not wonder why is it not working.
There was a problem hiding this comment.
removed it from warehouse instance and added to the specific test
| @@ -471,7 +473,7 @@ private int executeIncrementalLoad(DriverContext driverContext) { | |||
| if (work.hasBootstrapLoadTasks()) { | |||
| LOG.debug("Current incremental dump have tables to be bootstrapped. Switching to bootstrap " | |||
| + "mode after applying all events."); | |||
| return executeBootStrapLoad(driverContext); | |||
| return executeBootStrapLoad(driverContext, true); | |||
There was a problem hiding this comment.
Can we set isBootstrapDuringIncLoad in ReplLoadWork object constructor itself? so that need not pass it as extra argument and need not set it after each iteration.
| if (withinContext.hiveConf.getBoolVar(HiveConf.ConfVars.REPL_BOOTSTRAP_ACID_TABLES)) { | ||
| if (!AcidUtils.isTransactionalTable(before) && AcidUtils.isTransactionalTable(after)) { | ||
| LOG.info("The table " + after.getTableName() + " is converted to ACID table." + | ||
| " It will be replicated with bootstrap load as REPL_BOOTSTRAP_ACID_TABLES is set to true."); |
There was a problem hiding this comment.
Use hive.repl.bootstrap.acid.tables instead of REPL_BOOTSTRAP_ACID_TABLES.
| // In case user has asked for bootstrap of transactional table, we replace the old one if present. This is to | ||
| // make sure that the transactional info like write id etc for the table is consistent between the | ||
| // source and target cluster. | ||
| if (isBootstrapDuringInc && AcidUtils.isTransactionalTable(table)) { |
There was a problem hiding this comment.
I think, this check should be done even for external tables. Why don't we always replace table in case of isBootstrapDuringInc?
…o a target 4.0 cluster with strict managed table set to true.
…o a target 4.0 cluster with strict managed table set to true. Review comment fix
| @@ -264,6 +264,7 @@ a database ( directory ) | |||
| || work.getPathsToCopyIterator().hasNext(); | |||
|
|
|||
| if (addAnotherLoadTask) { | |||
| // pass on the bootstrap during incremental flag for next iteration. | |||
| @@ -471,6 +472,7 @@ private int executeIncrementalLoad(DriverContext driverContext) { | |||
| if (work.hasBootstrapLoadTasks()) { | |||
| LOG.debug("Current incremental dump have tables to be bootstrapped. Switching to bootstrap " | |||
| + "mode after applying all events."); | |||
| work.setBootstrapDuringIncLoad(true); | |||
There was a problem hiding this comment.
We shall set it in ReplLoadWork constructor once for all.
…o a target 4.0 cluster with strict managed table set to true. Review comment fix - 2
…o a target 4.0 cluster with strict managed table set to true. Review comment fix
|
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. |
…