pudidic commented on code in PR #3999:
URL: https://github.com/apache/hive/pull/3999#discussion_r1095439239


##########
ql/src/test/org/apache/hadoop/hive/ql/parse/repl/metric/TestReplicationMetricCollector.java:
##########
@@ -298,6 +299,8 @@ public void testSuccessOptimizedBootstrapDumpMetrics() 
throws Exception {
 
     Metadata expectedMetadata = new Metadata("db", 
Metadata.ReplicationType.OPTIMIZED_BOOTSTRAP, "dummyDir");
     expectedMetadata.setLastReplId(10);
+    
expectedMetadata.setFailoverEndPoint(MetaStoreUtils.FailoverEndpoint.SOURCE.toString());

Review Comment:
   How about just MetaStoreUtils.FailoverEndpoint.SOURCE?



##########
ql/src/test/org/apache/hadoop/hive/ql/parse/repl/metric/TestReplicationMetricCollector.java:
##########
@@ -253,6 +255,8 @@ public void testSuccessPreOptimizedBootstrapDumpMetrics() 
throws Exception {
 
     Metadata expectedMetadata = new Metadata("db", 
Metadata.ReplicationType.PRE_OPTIMIZED_BOOTSTRAP, "dummyDir");
     expectedMetadata.setLastReplId(-1);
+    
expectedMetadata.setFailoverEndPoint(MetaStoreUtils.FailoverEndpoint.SOURCE.toString());

Review Comment:
   How about just MetaStoreUtils.FailoverEndpoint.SOURCE?



##########
ql/src/test/org/apache/hadoop/hive/ql/parse/repl/metric/TestReplicationMetricCollector.java:
##########
@@ -337,6 +340,8 @@ public void testFailoverReadyDumpMetrics() throws Exception 
{
     expectedMetadata.setLastReplId(10);
     expectedMetadata.setFailoverEventId(10);
     expectedMetadata.setFailoverMetadataLoc("dummyDir");
+    
expectedMetadata.setFailoverEndPoint(MetaStoreUtils.FailoverEndpoint.SOURCE.toString());

Review Comment:
   How about just MetaStoreUtils.FailoverEndpoint.SOURCE?



##########
ql/src/test/org/apache/hadoop/hive/ql/parse/repl/metric/TestReplicationMetricSink.java:
##########
@@ -231,6 +233,8 @@ public void testSuccessBootstrapDumpMetrics() throws 
Exception {
     expectedMetadata.setLastReplId(10);
     expectedMetadata.setFailoverEventId(100);
     expectedMetadata.setFailoverMetadataLoc(stagingDir + 
FailoverMetaData.FAILOVER_METADATA);
+    
expectedMetadata.setFailoverEndPoint(MetaStoreUtils.FailoverEndpoint.SOURCE.toString());

Review Comment:
   How about just MetaStoreUtils.FailoverEndpoint.SOURCE?



##########
ql/src/test/org/apache/hadoop/hive/ql/parse/repl/metric/TestReplicationMetricCollector.java:
##########
@@ -270,13 +274,10 @@ public void testSuccessPreOptimizedBootstrapDumpMetrics() 
throws Exception {
             Arrays.asList(ReplUtils.MetricName.TABLES.name(), 
ReplUtils.MetricName.FUNCTIONS.name()));
   }
 
-
-
-
   @Test
   public void testSuccessOptimizedBootstrapDumpMetrics() throws Exception {
     ReplicationMetricCollector optimizedBootstrapDumpMetricCollector = new 
OptimizedBootstrapDumpMetricCollector("db",
-            "dummyDir", conf, 0L);
+            "dummyDir", conf, 0L, 
MetaStoreUtils.FailoverEndpoint.SOURCE.toString(), 
ReplConst.FailoverType.UNPLANNED.toString());

Review Comment:
   How about just ReplConst.FailoverType.UNPLANNED?



##########
ql/src/test/org/apache/hadoop/hive/ql/parse/repl/metric/TestReplicationMetricCollector.java:
##########
@@ -337,6 +340,8 @@ public void testFailoverReadyDumpMetrics() throws Exception 
{
     expectedMetadata.setLastReplId(10);
     expectedMetadata.setFailoverEventId(10);
     expectedMetadata.setFailoverMetadataLoc("dummyDir");
+    
expectedMetadata.setFailoverEndPoint(MetaStoreUtils.FailoverEndpoint.SOURCE.toString());
+    
expectedMetadata.setFailoverType(ReplConst.FailoverType.PLANNED.toString());

Review Comment:
   How about just ReplConst.FailoverType.PLANNED?



##########
ql/src/java/org/apache/hadoop/hive/ql/parse/ReplicationSemanticAnalyzer.java:
##########
@@ -378,14 +379,35 @@ private void analyzeReplLoad(ASTNode ast) throws 
SemanticException {
     }
   }
 
-  private ReplicationMetricCollector initMetricCollection(String dumpDirectory,
-                                                          String 
dbNameToLoadIn, DumpMetaData dmd) throws SemanticException {
-
+  private ReplicationMetricCollector initReplicationLoadMetricCollector(String 
dumpDirectory, String dbNameToLoadIn,
+                                                                        
DumpMetaData dmd) throws SemanticException {
     ReplicationMetricCollector collector;
-    if (dmd.isPreOptimizedBootstrapDump()) {
-      collector = new PreOptimizedBootstrapLoadMetricCollector(dbNameToLoadIn, 
dumpDirectory, dmd.getDumpExecutionId(), conf);
-    } else if (dmd.isOptimizedBootstrapDump()) {
-      collector = new OptimizedBootstrapLoadMetricCollector(dbNameToLoadIn, 
dumpDirectory, dmd.getDumpExecutionId(), conf);
+    if (dmd.isPreOptimizedBootstrapDump() || dmd.isOptimizedBootstrapDump()) {
+      Database dbToLoad = null;
+      try {
+        dbToLoad = db.getDatabase(dbNameToLoadIn);
+      } catch (HiveException e) {
+        throw new SemanticException(e.getMessage(), e);
+      }
+      if (dbToLoad == null) {
+        throw new SemanticException(ErrorMsg.DATABASE_NOT_EXISTS, 
dbNameToLoadIn);
+      }
+      // db property ReplConst.FAILOVER_ENDPOINT is only set during planned 
failover.
+      String failoverType = "";

Review Comment:
   How about just FailoverType?



##########
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/incremental/IncrementalLoadTasksBuilder.java:
##########
@@ -99,8 +100,22 @@ public IncrementalLoadTasksBuilder(String dbName, String 
loadPath, IncrementalLo
     metricMap.put(ReplUtils.MetricName.EVENTS.name(), (long) 
iterator.getNumEvents());
     this.shouldFailover = shouldFailover;
     if (shouldFailover) {
+      Database db = null;
+      try {
+        db = Hive.get().getDatabase(dbName);
+      } catch (HiveException e) {
+        throw new RuntimeException(e);
+      }
+      String dbFailoverEndPoint = "";
+      if (db != null) {
+        Map<String, String> params = db.getParameters();
+        if (params != null) {
+          dbFailoverEndPoint = params.get(ReplConst.REPL_FAILOVER_ENDPOINT);
+        }
+      }
       this.metricCollector.reportFailoverStart("REPL_LOAD", metricMap,
-              new FailoverMetaData(new Path(dumpDirectory, 
ReplUtils.REPL_HIVE_BASE_DIR), conf));
+          new FailoverMetaData(new Path(dumpDirectory, 
ReplUtils.REPL_HIVE_BASE_DIR), conf),
+          dbFailoverEndPoint, ReplConst.FailoverType.PLANNED.toString());

Review Comment:
   How about just ReplConst.FailoverType.PLANNED?



##########
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/metric/event/Metadata.java:
##########
@@ -30,12 +30,15 @@ public enum ReplicationType {
     PRE_OPTIMIZED_BOOTSTRAP,
     OPTIMIZED_BOOTSTRAP
   }
+
   private String dbName;
   private ReplicationType replicationType;
   private String stagingDir;
   private long lastReplId;
   private String failoverMetadataLoc;
   private long failoverEventId;
+  private String failoverEndPoint;

Review Comment:
   How about replacing it with the new enum?



##########
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/metric/event/Metadata.java:
##########
@@ -92,4 +97,11 @@ public void setFailoverEventId(long failoverEventId) {
     this.failoverEventId = failoverEventId;
   }
 
+  public String getFailoverEndPoint() { return failoverEndPoint; }
+
+  public void setFailoverEndPoint(String endpoint) { this.failoverEndPoint = 
endpoint; }

Review Comment:
   How about just MetaStoreUtils.FailoverEndpoint?



##########
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/metric/event/Metadata.java:
##########
@@ -92,4 +97,11 @@ public void setFailoverEventId(long failoverEventId) {
     this.failoverEventId = failoverEventId;
   }
 
+  public String getFailoverEndPoint() { return failoverEndPoint; }
+
+  public void setFailoverEndPoint(String endpoint) { this.failoverEndPoint = 
endpoint; }
+
+  public String getFailoverType() { return failoverType; }

Review Comment:
   How about just ReplConst.FailoverType?



##########
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/metric/ReplicationMetricCollector.java:
##########
@@ -74,6 +73,24 @@ public ReplicationMetricCollector(String dbName, 
Metadata.ReplicationType replic
     }
   }
 
+  public ReplicationMetricCollector(String dbName, Metadata.ReplicationType 
replicationType,
+                                    String stagingDir, long dumpExecutionId, 
HiveConf conf,
+                                    String failoverEndpoint, String 
failoverType) {

Review Comment:
   How about passing FailoverEndpoint and FailoverType?



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

To unsubscribe, e-mail: [email protected]

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