[ 
https://issues.apache.org/jira/browse/HIVE-26921?focusedWorklogId=843393&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-843393
 ]

ASF GitHub Bot logged work on HIVE-26921:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 03/Feb/23 07:42
            Start Date: 03/Feb/23 07:42
    Worklog Time Spent: 10m 
      Work Description: 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?





Issue Time Tracking
-------------------

    Worklog Id:     (was: 843393)
    Time Spent: 1h 10m  (was: 1h)

> Add failover_type, failover_endpoint to replication metrics metadata
> --------------------------------------------------------------------
>
>                 Key: HIVE-26921
>                 URL: https://issues.apache.org/jira/browse/HIVE-26921
>             Project: Hive
>          Issue Type: Improvement
>          Components: HiveServer2
>            Reporter: Amit Saonerkar
>            Assignee: Amit Saonerkar
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> Corresponding to CDPD-46494



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to