Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HIVE-19812 : Disable external table replication by default via a coniguration property #365

Closed
wants to merge 2 commits into from

Conversation

maheshk114
Copy link
Contributor

use a hive config property to allow external table replication. set this property by default to prevent external table replication.

for metadata only hive repl always export metadata for external tables.

.run("import table " + replDbName + ".t2 from " + exportDataPath)
.run("select * from " + replDbName + ".t2")
.verifyResults(new String[] { "1", "2" })
.runFailure("import table " + replDbName + ".t3 from " + exportDataPathRepl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Export already failed for this path exportDataPathRepl. No point in importing from this path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its ok to have import

@@ -482,6 +483,11 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
REPL_ADD_RAW_RESERVED_NAMESPACE("hive.repl.add.raw.reserved.namespace", false,
"For TDE with same encryption keys on source and target, allow Distcp super user to access \n"
+ "the raw bytes from filesystem without decrypting on source and then encrypting on target."),
REPL_INCLUDE_EXTERNAL_TABLES("hive.repl.dump.include.external.tables", false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name can be hive.repl.include.external.tables. Shall remove "dump"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

primary = new WarehouseInstance(LOG, miniDFSCluster, overridesForHiveConf);
replica = new WarehouseInstance(LOG, miniDFSCluster, overridesForHiveConf);
externalDump = new WarehouseInstance(LOG, miniDFSCluster, overridesForHiveConfDump);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of another WarehouseInstance, shall pass the new config as an option to WITH clause for REPL DUMP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

@Test
public void testDumpExternalTableSetTrue() throws Throwable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall merge into 1 test case where pass different configs in WITH clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 test is cleaner

.run("show tables like 't2'")
.verifyFailure(new String[] {"t2"});

tuple = primary.run("use " + primaryDbName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall also test incremental dump with new config=true and metadata_only=false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -176,6 +177,12 @@ public static Boolean shouldReplicate(ReplicationSpec replicationSpec, Table tab
return false;
}

if (!hiveConf.getBoolVar(HiveConf.ConfVars.REPL_INCLUDE_EXTERNAL_TABLES) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can move this check inside below if block for replicationSpec.isInReplicationScope().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@maheshk114 maheshk114 force-pushed the BUG-104223 branch 2 times, most recently from 13b2b64 to f5a72a6 Compare June 11, 2018 04:13
@github-actions
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants