aasha commented on a change in pull request #1515:
URL: https://github.com/apache/hive/pull/1515#discussion_r492461348



##########
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java
##########
@@ -424,6 +424,20 @@ public String encodeFileUri(String fileUriStr, String 
fileChecksum, String encod
     return encodedUri;
   }
 
+  public static String encodeFileUri(String fileUriStr, String fileChecksum, 
String cmroot, String encodedSubDir) {
+    String encodedUri = fileUriStr;
+    if ((fileChecksum != null) && (cmroot != null)) {
+      encodedUri = encodedUri + URI_FRAGMENT_SEPARATOR + fileChecksum + 
URI_FRAGMENT_SEPARATOR + cmroot;
+    } else {
+      encodedUri = encodedUri + URI_FRAGMENT_SEPARATOR + 
URI_FRAGMENT_SEPARATOR;

Review comment:
       why do we have 2 URI_FRAGMENT_SEPARATOR

##########
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java
##########
@@ -424,6 +424,20 @@ public String encodeFileUri(String fileUriStr, String 
fileChecksum, String encod
     return encodedUri;
   }
 
+  public static String encodeFileUri(String fileUriStr, String fileChecksum, 
String cmroot, String encodedSubDir) {
+    String encodedUri = fileUriStr;
+    if ((fileChecksum != null) && (cmroot != null)) {
+      encodedUri = encodedUri + URI_FRAGMENT_SEPARATOR + fileChecksum + 
URI_FRAGMENT_SEPARATOR + cmroot;
+    } else {
+      encodedUri = encodedUri + URI_FRAGMENT_SEPARATOR + 
URI_FRAGMENT_SEPARATOR;
+    }
+    encodedUri = encodedUri + URI_FRAGMENT_SEPARATOR + ((encodedSubDir != 
null) ? encodedSubDir : "");
+    if (LOG.isDebugEnabled()) {

Review comment:
       Do we need this check?

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -522,6 +522,14 @@ private static void populateLlapDaemonVarsSet(Set<String> 
llapDaemonVarsSetLocal
     REPLCMINTERVAL("hive.repl.cm.interval","3600s",
         new TimeValidator(TimeUnit.SECONDS),
         "Inteval for cmroot cleanup thread."),
+    
REPL_HA_DATAPATH_REPLACE_REMOTE_NAMESERVICE("hive.repl.ha.datapath.replace.remote.nameservice",
 false,
+            "When HDFS is HA enabled and both source and target clusters are 
configured with same nameservice names," +
+                    "enable this flag and provide a "),

Review comment:
       sentence is incomplete

##########
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java
##########
@@ -424,6 +424,20 @@ public String encodeFileUri(String fileUriStr, String 
fileChecksum, String encod
     return encodedUri;
   }
 
+  public static String encodeFileUri(String fileUriStr, String fileChecksum, 
String cmroot, String encodedSubDir) {
+    String encodedUri = fileUriStr;
+    if ((fileChecksum != null) && (cmroot != null)) {

Review comment:
       empty check not needed?

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/Utils.java
##########
@@ -72,6 +76,40 @@ public static void writeOutput(List<List<String>> 
listValues, Path outputFile, H
     writeOutput(listValues, outputFile, hiveConf, false);
   }
 
+  /**
+   * Given a ReplChangeManger's encoded uri, replaces the namespace and 
returns the modified encoded uri.
+   */
+  public static String replaceNameSpaceInEncodedURI(String cmEncodedURI, 
HiveConf hiveConf) throws SemanticException {

Review comment:
       replace name service?

##########
File path: 
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java
##########
@@ -1963,4 +2062,12 @@ private void setupUDFJarOnHDFS(Path 
identityUdfLocalPath, Path identityUdfHdfsPa
     FileSystem fs = primary.miniDFSCluster.getFileSystem();
     fs.copyFromLocalFile(identityUdfLocalPath, identityUdfHdfsPath);
   }
+
+  private List<String> getHdfsNamespaceClause() {

Review comment:
       replace with nameservice

##########
File path: 
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java
##########
@@ -1604,6 +1605,122 @@ public void testRangerReplication() throws Throwable {
         .verifyResults(new String[] {"1", "2"});
   }
 
+  @Test
+  public void testHdfsNamespaceLazyCopy() throws Throwable {
+    List<String> clause = getHdfsNameserviceClause();
+    clause.add("'" + 
HiveConf.ConfVars.REPL_DUMP_METADATA_ONLY_FOR_EXTERNAL_TABLE.varname + 
"'='true'");
+    primary.run("use " + primaryDbName)
+            .run("create table  acid_table (key int, value int) partitioned by 
(load_date date) " +
+                    "clustered by(key) into 2 buckets stored as orc 
tblproperties ('transactional'='true')")
+            .run("create table table1 (i int)")
+            .run("insert into table1 values (1)")
+            .run("insert into table1 values (2)")
+            .run("create external table ext_table1 (id int)")
+            .run("insert into ext_table1 values (3)")
+            .run("insert into ext_table1 values (4)")
+            .dump(primaryDbName, clause);
+
+    try{
+      replica.load(replicatedDbName, primaryDbName, clause);
+      Assert.fail("Expected the UnknownHostException to be thrown.");
+    } catch (IllegalArgumentException ex) {
+      assertTrue(ex.getMessage().contains("java.net.UnknownHostException: 
nsRemote"));
+    }
+  }
+
+  @Test
+  public void testHdfsNamespaceLazyCopyIncr() throws Throwable {
+    ArrayList clause = new ArrayList();
+    clause.add("'" + 
HiveConf.ConfVars.REPL_DUMP_METADATA_ONLY_FOR_EXTERNAL_TABLE.varname + 
"'='true'");
+    primary.run("use " + primaryDbName)
+            .run("create table  acid_table (key int, value int) partitioned by 
(load_date date) " +
+                    "clustered by(key) into 2 buckets stored as orc 
tblproperties ('transactional'='true')")
+            .run("create table table1 (i String)")
+            .run("insert into table1 values (1)")
+            .run("insert into table1 values (2)")
+            .run("create external table ext_table1 (id int)")
+            .run("insert into ext_table1 values (3)")
+            .run("insert into ext_table1 values (4)")
+            .dump(primaryDbName);
+
+    replica.load(replicatedDbName, primaryDbName, clause)
+            .run("use " + replicatedDbName)
+            .run("show tables")
+            .verifyResults(new String[] {"acid_table", "table1", "ext_table1"})
+            .run("select * from table1")
+            .verifyResults(new String[] {"1", "2"})
+            .run("select * from ext_table1")
+            .verifyResults(new String[] {"3", "4"});
+
+    clause.addAll(getHdfsNameserviceClause());
+    primary.run("use " + primaryDbName)
+            .run("insert into table1 values (5)")
+            .run("insert into ext_table1 values (6)")
+            .dump(primaryDbName, clause);
+    try{
+      replica.load(replicatedDbName, primaryDbName, clause);
+      Assert.fail("Expected the UnknownHostException to be thrown.");
+    } catch (IllegalArgumentException ex) {
+      assertTrue(ex.getMessage().contains("java.net.UnknownHostException: 
nsRemote"));
+    }
+  }
+
+  @Test
+  public void testHdfsNamespaceWithDataCopy() throws Throwable {

Review comment:
       nameservice




----------------------------------------------------------------
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to