This is an automated email from the ASF dual-hosted git repository.
granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 14d4255 [backup] Quick fix to fromMs option logic
14d4255 is described below
commit 14d4255141dbe9797283209d55a05010400ac1ca
Author: Grant Henke <[email protected]>
AuthorDate: Wed Apr 24 15:06:39 2019 -0500
[backup] Quick fix to fromMs option logic
The fromMS logic was incorrectly updated and we
didn’t have a test validating the bahavior. This patch
fixes and tests that issue.
Additionally the TestForceIncrementalBackup was
wrong as well.
KUDU-2790 tracks overall coverage improvements,
so this is just a quick fix. KUDU-2790 will be my next
path on this code.
Change-Id: I12dc557fa71470ca8aef2a1c2393bda28e0b81dd
Reviewed-on: http://gerrit.cloudera.org:8080/13105
Reviewed-by: Adar Dembo <[email protected]>
Tested-by: Kudu Jenkins
---
.../src/main/scala/org/apache/kudu/backup/KuduBackup.scala | 9 ++++++---
.../main/scala/org/apache/kudu/backup/KuduBackupRDD.scala | 4 +---
.../src/main/scala/org/apache/kudu/backup/Options.scala | 8 +-------
.../test/scala/org/apache/kudu/backup/TestKuduBackup.scala | 12 ++++++++++--
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git
a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
index a7c277f..5d498ce 100644
--- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
+++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
@@ -51,10 +51,12 @@ object KuduBackup {
// Unless we are forcing a full backup or a fromMs was set, find the
previous backup and
// use the `to_ms` metadata as the `from_ms` time for this backup.
+ var incremental = false
if (tableOptions.forceFull) {
log.info("Performing a full backup, forceFull was set to true")
- } else if (tableOptions.fromMs == BackupOptions.DefaultFromMS) {
+ } else if (tableOptions.fromMs != BackupOptions.DefaultFromMS) {
log.info(s"Performing an incremental backup, fromMs was set to
${tableOptions.fromMs}")
+ incremental = true
} else {
log.info("Looking for a previous backup, forceFull or fromMs options
are not set.")
val graph = io.readBackupGraph(tableName)
@@ -62,15 +64,16 @@ object KuduBackup {
val base = graph.backupBase
log.info(s"Setting fromMs to ${base.metadata.getToMs} from backup in
path: ${base.path}")
tableOptions = tableOptions.copy(fromMs = base.metadata.getToMs)
+ incremental = true
} else {
log.info("No previous backup was found. Starting a full backup.")
tableOptions = tableOptions.copy(forceFull = true)
}
}
- val rdd = new KuduBackupRDD(table, tableOptions, context,
session.sparkContext)
+ val rdd = new KuduBackupRDD(table, tableOptions, incremental, context,
session.sparkContext)
val df =
session.sqlContext
- .createDataFrame(rdd, io.dataSchema(table.getSchema,
tableOptions.isIncremental))
+ .createDataFrame(rdd, io.dataSchema(table.getSchema, incremental))
// Write the data to the backup path.
// The backup path contains the timestampMs and should not already exist.
diff --git
a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
index 80abb6d..c9837ce 100644
--- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
+++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala
@@ -38,13 +38,11 @@ import scala.collection.JavaConverters._
class KuduBackupRDD private[kudu] (
@transient val table: KuduTable,
@transient val options: BackupOptions,
+ val incremental: Boolean,
val kuduContext: KuduContext,
@transient val sc: SparkContext)
extends RDD[Row](sc, Nil) {
- // Defined here because the options are transient.
- val incremental: Boolean = options.isIncremental
-
// TODO (KUDU-2785): Split large tablets into smaller scan tokens?
override protected def getPartitions: Array[Partition] = {
val client = kuduContext.syncClient
diff --git
a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
index 51d468f..cea1bcc 100644
--- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
+++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
@@ -46,13 +46,7 @@ case class BackupOptions(
scanLeaderOnly: Boolean = BackupOptions.DefaultScanLeaderOnly,
scanPrefetching: Boolean = BackupOptions.DefaultScanPrefetching,
keepAlivePeriodMs: Long = BackupOptions.DefaultKeepAlivePeriodMs)
- extends CommonOptions {
-
- // If not forcing a full backup and fromMs is not set, this is an
incremental backup.
- def isIncremental: Boolean = {
- !forceFull && fromMs != BackupOptions.DefaultFromMS
- }
-}
+ extends CommonOptions
object BackupOptions {
val DefaultForceFull: Boolean = false
diff --git
a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
index 2105bbd..35c11bc 100644
---
a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
+++
b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
@@ -76,7 +76,14 @@ class TestKuduBackup extends KuduTestSuite {
val rootDir = Files.createTempDirectory("backup")
doBackup(rootDir, Seq(tableName)) // Full backup.
insertRows(table, 100, 100) // Insert more data.
+
+ val logs = new CapturingLogAppender()
+ val handle = logs.attach()
doBackup(rootDir, Seq(tableName)) // Incremental backup.
+ handle.close()
+ assertTrue(logs.getAppendedText.contains("Setting fromMs to"))
+ assertTrue(logs.getAppendedText.contains("from backup in path"))
+
// Delete rows that span the full and incremental backup.
Range(50, 150).foreach { i =>
deleteRow(i)
@@ -93,7 +100,7 @@ class TestKuduBackup extends KuduTestSuite {
@Test
def TestForceIncrementalBackup() {
insertRows(table, 100) // Insert data into the default test table.
- val beforeMs = getPropagatedTimestampMs
+ val beforeMs = System.currentTimeMillis()
insertRows(table, 100, 100) // Insert more data.
val rootDir = Files.createTempDirectory("backup")
@@ -104,12 +111,13 @@ class TestKuduBackup extends KuduTestSuite {
// Force an incremental backup without a full backup.
// It will use a diff scan and won't check the existing dependency graph.
val handle = logs.attach()
+ log.error("FROM MS is set to: " + beforeMs)
doBackup(rootDir, Seq(tableName), fromMs = beforeMs) // Incremental backup.
handle.close()
assertTrue(Files.isDirectory(rootDir))
assertEquals(1, rootDir.toFile.list().length)
- assertTrue(logs.getAppendedText.contains("fromMs was set"))
+ assertTrue(logs.getAppendedText.contains("Performing an incremental
backup, fromMs was set to"))
}
@Test
