[
https://issues.apache.org/jira/browse/HIVE-24918?focusedWorklogId=621806&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-621806
]
ASF GitHub Bot logged work on HIVE-24918:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 13/Jul/21 03:59
Start Date: 13/Jul/21 03:59
Worklog Time Spent: 10m
Work Description: pkumarsinha commented on a change in pull request #2121:
URL: https://github.com/apache/hive/pull/2121#discussion_r668403004
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -268,11 +292,28 @@ private boolean shouldDumpAtlasMetadata() {
return conf.getBoolVar(HiveConf.ConfVars.REPL_INCLUDE_ATLAS_METADATA);
}
- private Path getCurrentDumpPath(Path dumpRoot, boolean isBootstrap) throws
IOException {
+ private Path getCurrentDumpPath(Path dumpRoot, boolean isBootstrap) throws
IOException, HiveException {
Path lastDumpPath = ReplUtils.getLatestDumpPath(dumpRoot, conf);
if (lastDumpPath != null && shouldResumePreviousDump(lastDumpPath,
isBootstrap)) {
//Resume previous dump
LOG.info("Resuming the dump with existing dump directory {}",
lastDumpPath);
+ FileSystem fs = lastDumpPath.getFileSystem(conf);
+ Path hiveDumpDir = new Path(lastDumpPath, ReplUtils.REPL_HIVE_BASE_DIR);
+ if (fs.exists(new Path(hiveDumpDir,
FailoverMetaData.FAILOVER_METADATA))) {
+ if (conf.getBoolVar(HiveConf.ConfVars.HIVE_REPL_FAILOVER_START)) {
+ FailoverMetaData fmd = new FailoverMetaData(hiveDumpDir, conf);
+ if (fmd.isValidMetadata()) {
+ LOG.info("Resuming the dump with existing failover metadata: {}",
fmd.getFilePath());
+ work.setFailoverMetadata(fmd);
+ }
+ } else {
+
unsetReplFailoverEnabled(getHive().getDatabase(work.dbNameOrPattern));
+ fs.delete(new Path(hiveDumpDir, FailoverMetaData.FAILOVER_METADATA));
Review comment:
Don't use deprecated version of delete
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -180,6 +184,21 @@ public int execute() {
}
//If no previous dump is present or previous dump is already loaded,
proceed with the dump operation.
if (shouldDump(previousValidHiveDumpPath)) {
+ if (previousValidHiveDumpPath != null) {
+ FileSystem fs = previousValidHiveDumpPath.getFileSystem(conf);
+ if (fs.exists(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()))) {
+ Database db = getHive().getDatabase(work.dbNameOrPattern);
+ if (MetaStoreUtils.isDbBeingFailedOver(db)) {
+ unsetReplFailoverEnabled(db);
+ fs.delete(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()));
Review comment:
Create Path once and reuse
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -180,6 +184,21 @@ public int execute() {
}
//If no previous dump is present or previous dump is already loaded,
proceed with the dump operation.
if (shouldDump(previousValidHiveDumpPath)) {
+ if (previousValidHiveDumpPath != null) {
+ FileSystem fs = previousValidHiveDumpPath.getFileSystem(conf);
+ if (fs.exists(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()))) {
+ Database db = getHive().getDatabase(work.dbNameOrPattern);
+ if (MetaStoreUtils.isDbBeingFailedOver(db)) {
+ unsetReplFailoverEnabled(db);
+ fs.delete(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()));
Review comment:
Add a log statement why you are deleting this file.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -268,11 +292,28 @@ private boolean shouldDumpAtlasMetadata() {
return conf.getBoolVar(HiveConf.ConfVars.REPL_INCLUDE_ATLAS_METADATA);
}
- private Path getCurrentDumpPath(Path dumpRoot, boolean isBootstrap) throws
IOException {
+ private Path getCurrentDumpPath(Path dumpRoot, boolean isBootstrap) throws
IOException, HiveException {
Path lastDumpPath = ReplUtils.getLatestDumpPath(dumpRoot, conf);
if (lastDumpPath != null && shouldResumePreviousDump(lastDumpPath,
isBootstrap)) {
//Resume previous dump
LOG.info("Resuming the dump with existing dump directory {}",
lastDumpPath);
+ FileSystem fs = lastDumpPath.getFileSystem(conf);
+ Path hiveDumpDir = new Path(lastDumpPath, ReplUtils.REPL_HIVE_BASE_DIR);
+ if (fs.exists(new Path(hiveDumpDir,
FailoverMetaData.FAILOVER_METADATA))) {
+ if (conf.getBoolVar(HiveConf.ConfVars.HIVE_REPL_FAILOVER_START)) {
+ FailoverMetaData fmd = new FailoverMetaData(hiveDumpDir, conf);
+ if (fmd.isValidMetadata()) {
+ LOG.info("Resuming the dump with existing failover metadata: {}",
fmd.getFilePath());
+ work.setFailoverMetadata(fmd);
+ }
+ } else {
+
unsetReplFailoverEnabled(getHive().getDatabase(work.dbNameOrPattern));
+ fs.delete(new Path(hiveDumpDir, FailoverMetaData.FAILOVER_METADATA));
Review comment:
Why do you again need to delete and unset the repl enabled property
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -180,6 +184,21 @@ public int execute() {
}
//If no previous dump is present or previous dump is already loaded,
proceed with the dump operation.
if (shouldDump(previousValidHiveDumpPath)) {
+ if (previousValidHiveDumpPath != null) {
+ FileSystem fs = previousValidHiveDumpPath.getFileSystem(conf);
+ if (fs.exists(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()))) {
+ Database db = getHive().getDatabase(work.dbNameOrPattern);
+ if (MetaStoreUtils.isDbBeingFailedOver(db)) {
+ unsetReplFailoverEnabled(db);
+ fs.delete(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()));
Review comment:
Don't use deprecated method - delete
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -180,6 +184,21 @@ public int execute() {
}
//If no previous dump is present or previous dump is already loaded,
proceed with the dump operation.
if (shouldDump(previousValidHiveDumpPath)) {
+ if (previousValidHiveDumpPath != null) {
+ FileSystem fs = previousValidHiveDumpPath.getFileSystem(conf);
+ if (fs.exists(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()))) {
+ Database db = getHive().getDatabase(work.dbNameOrPattern);
+ if (MetaStoreUtils.isDbBeingFailedOver(db)) {
+ unsetReplFailoverEnabled(db);
Review comment:
What happens if unsetReplFailoverEnabled() executes successfully but
repl dump fails at next line while executing fs.delete because HDFS was down.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -180,6 +184,21 @@ public int execute() {
}
//If no previous dump is present or previous dump is already loaded,
proceed with the dump operation.
if (shouldDump(previousValidHiveDumpPath)) {
+ if (previousValidHiveDumpPath != null) {
+ FileSystem fs = previousValidHiveDumpPath.getFileSystem(conf);
+ if (fs.exists(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()))) {
+ Database db = getHive().getDatabase(work.dbNameOrPattern);
+ if (MetaStoreUtils.isDbBeingFailedOver(db)) {
+ unsetReplFailoverEnabled(db);
+ fs.delete(new Path(previousValidHiveDumpPath,
ReplAck.FAILOVER_READY_MARKER.toString()));
+ } else {
+ LOG.info("Switching to bootstrap dump as this is the first
dump execution after failover.");
+ isBootstrap = true;
Review comment:
We don't need to use a separate variable isBootstrap given that work
already has it
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -268,11 +292,28 @@ private boolean shouldDumpAtlasMetadata() {
return conf.getBoolVar(HiveConf.ConfVars.REPL_INCLUDE_ATLAS_METADATA);
}
- private Path getCurrentDumpPath(Path dumpRoot, boolean isBootstrap) throws
IOException {
+ private Path getCurrentDumpPath(Path dumpRoot, boolean isBootstrap) throws
IOException, HiveException {
Path lastDumpPath = ReplUtils.getLatestDumpPath(dumpRoot, conf);
if (lastDumpPath != null && shouldResumePreviousDump(lastDumpPath,
isBootstrap)) {
//Resume previous dump
LOG.info("Resuming the dump with existing dump directory {}",
lastDumpPath);
+ FileSystem fs = lastDumpPath.getFileSystem(conf);
+ Path hiveDumpDir = new Path(lastDumpPath, ReplUtils.REPL_HIVE_BASE_DIR);
+ if (fs.exists(new Path(hiveDumpDir,
FailoverMetaData.FAILOVER_METADATA))) {
Review comment:
Why do we need to redo these steps
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
##########
@@ -358,6 +403,9 @@ private void deleteAllPreviousDumpMeta(Path
currentDumpPath) {
for (FileStatus status : statuses) {
//based on config, either delete all previous dump-dirs
//or delete a minimum number of oldest dump-directories
+ if (numDumpDirs == 1 && work.isFirstDumpAfterFailover()) {
+ break;
Review comment:
A Log statement would help debug
--
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: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 621806)
Time Spent: 3h 40m (was: 3.5h)
> Handle failover case during Repl Dump
> -------------------------------------
>
> Key: HIVE-24918
> URL: https://issues.apache.org/jira/browse/HIVE-24918
> Project: Hive
> Issue Type: New Feature
> Reporter: Haymant Mangla
> Assignee: Haymant Mangla
> Priority: Major
> Labels: pull-request-available
> Time Spent: 3h 40m
> Remaining Estimate: 0h
>
> To handle:
> a) Whenever user wants to go ahead with failover, during the next or
> subsequent repl dump operation upon confirming that there are no pending open
> transaction events, It should create a _failover_ready marker file in the
> dump dir. This marker file would contain scheduled query name
> that has generated this dump.
> b) Skip next repl dump instances once we have the marker file placed.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
