[CARBONDATA-3582] support table status file backup #3459

jackylk · 2019-11-14T10:17:16Z

When overwriting table status file, if process crashed, table status file will be in corrupted state. This can happen in an unstable environment, like in the cloud. To prevent the table corruption, user can enable a newly added CarbonProperty to enable backup of the table status before overwriting it.

New CarbonProperty: ENABLE_TABLE_STATUS_BACKUP (default is false)
When enabling this property, "tablestatus.backup" file will be created in the same folder of "tablestatus" file

Any interfaces changed?
No
Any backward compatibility impacted?
No
Document update required?
Yes
Testing done
new testcase is added
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
No

CarbonDataQA · 2019-11-14T11:02:07Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/856/

CarbonDataQA · 2019-11-14T11:49:34Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/862/

CarbonDataQA · 2019-11-14T12:28:36Z

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/864/

CarbonDataQA · 2019-11-15T03:53:26Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/870/

CarbonDataQA · 2019-11-15T04:53:26Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/876/

CarbonDataQA · 2019-11-15T04:57:28Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/878/

jackylk · 2019-11-15T08:27:54Z

retest this please

CarbonDataQA · 2019-11-15T08:53:26Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/875/

CarbonDataQA · 2019-11-15T10:02:33Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/883/

CarbonDataQA · 2019-11-15T10:03:15Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/881/

ajantha-bhat · 2019-11-15T17:05:46Z

@jackylk : please resolve the conflict

CarbonDataQA · 2019-11-16T15:20:22Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/898/

CarbonDataQA · 2019-11-16T16:32:15Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/905/

CarbonDataQA · 2019-11-16T16:37:01Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/906/

xubo245 · 2019-11-17T04:44:52Z

core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java

@@ -526,40 +539,79 @@ private static Integer compareDateValues(Long loadValue, Long userValue) {
  }

  /**
-   * writes load details into a given file at @param dataLoadLocation
+   * Backup the table status file as 'filename.backup'


Please format the code

xubo245 · 2019-11-17T05:05:05Z

core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java

+  }
+
+  // a dummy func for mocking in testcase, which simulates IOException
+  private static void mockForTest() throws IOException {


This method is no useful for this class, right? it's better to remove if it's only used by test case.

The testcase need to mock the function to throw Exception, it is required for test case

xubo245 · 2019-11-17T05:06:56Z

core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java

    } catch (IOException ioe) {
-      LOG.error("Error message: " + ioe.getLocalizedMessage());
+      LOG.error("Write file failed: " + ioe.getLocalizedMessage());


It's better to print Stack Trace in log.

This exception will be logged, it is thrown at line 604

xubo245 · 2019-11-17T05:07:09Z

core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java

+          brWriter.flush();
+        }
+      } catch (IOException e) {
+        LOG.error("Flush file failed: " + e.getLocalizedMessage());


It's better to print Stack Trace in log.

I removed the flush call, just close the stream is enough

xubo245 · 2019-11-17T05:10:13Z

integration/spark2/src/test/scala/org/apache/spark/carbondata/TableStatusBackupTest.scala

+      }
+    }
+
+    val exception = intercept[IOException] {


exception is never used, it's better to add some check for exception message

fixed, added assertion check

CarbonDataQA · 2019-11-17T07:54:51Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/903/

CarbonDataQA · 2019-11-17T09:06:39Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/910/

CarbonDataQA · 2019-11-17T09:31:19Z

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/911/

jackylk · 2019-11-17T13:18:30Z

@QiangCai please review this PR

xubo245 · 2019-11-17T14:53:45Z

Please rebase it.

CarbonDataQA · 2019-11-17T16:00:58Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/912/

CarbonDataQA · 2019-11-17T17:06:47Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/920/

CarbonDataQA · 2019-11-17T17:07:07Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/919/

CarbonDataQA · 2019-11-18T06:34:41Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/919/

CarbonDataQA · 2019-11-18T14:14:33Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/934/

CarbonDataQA · 2019-11-18T15:27:10Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/943/

CarbonDataQA · 2019-11-18T15:35:23Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/944/

jackylk · 2019-11-24T13:09:04Z

retest this please

CarbonDataQA · 2019-11-24T13:26:16Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/989/

CarbonDataQA · 2019-11-24T14:59:43Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/998/

CarbonDataQA · 2019-11-24T17:48:57Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1001/

jackylk · 2019-11-28T14:38:40Z

retest this please

CarbonDataQA1 · 2019-11-28T14:58:33Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1036/

CarbonDataQA1 · 2019-11-28T16:28:00Z

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1045/

CarbonDataQA1 · 2019-11-28T17:17:28Z

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1050/

fix mock rebase fix comment

CarbonDataQA1 · 2019-12-16T08:25:48Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1182/

CarbonDataQA1 · 2019-12-16T08:38:13Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1191/

CarbonDataQA1 · 2019-12-16T09:12:50Z

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1201/

QiangCai · 2019-12-16T10:51:45Z

LGTM

brijoobopanna · 2019-12-16T11:42:51Z

@jackylk : please take care of TODO: add document

kumarvishal09 · 2019-12-16T12:11:00Z

core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java

+  }
+
+  // a dummy func for mocking in testcase, which simulates IOException
+  private static void mockForTest() throws IOException {


Adding a method for test in the code is not a good idea

There is other place we we add util for test like TestUtil.java in SDK module.

kumarvishal09 · 2019-12-16T12:27:20Z

@jackylk Every time it will create a new backup file or it will create new backup and delete old backup file??? otherwise number of backup file will be more

jackylk · 2019-12-21T14:33:51Z

It will overwrite the backup file so only one backup file will be there @kumarvishal09

jackylk · 2019-12-21T14:40:13Z

document is added in configuration-parameters.md @brijoobopanna

CarbonDataQA1 · 2019-12-21T15:40:01Z

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1236/

CarbonDataQA1 · 2019-12-21T17:00:23Z

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1255/

CarbonDataQA1 · 2019-12-21T19:55:08Z

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1246/

ajantha-bhat · 2019-12-24T07:03:21Z

LGTM

When overwriting table status file, if process crashed, table status file will be in corrupted state. This can happen in an unstable environment, like in the cloud. To prevent the table corruption, user can enable a newly added CarbonProperty to enable backup of the table status before overwriting it. New CarbonProperty: ENABLE_TABLE_STATUS_BACKUP (default is false) When enabling this property, "tablestatus.backup" file will be created in the same folder of "tablestatus" file This closes apache#3459

jackylk force-pushed the tablestatus_backup branch from c02f1e0 to 22e804b Compare November 16, 2019 14:41

xubo245 reviewed Nov 17, 2019

View reviewed changes

jackylk force-pushed the tablestatus_backup branch from ac65daa to eeb9258 Compare November 17, 2019 15:20

jackylk added 2 commits December 16, 2019 15:02

support table status file backup

544034e

fix mock rebase fix comment

backup table status only

bcc7f30

jackylk force-pushed the tablestatus_backup branch from 8f580db to bcc7f30 Compare December 16, 2019 07:05

kumarvishal09 reviewed Dec 16, 2019

View reviewed changes

fix comment

c7f1c00

asfgit closed this in 5d287b8 Dec 24, 2019

[CARBONDATA-3582] support table status file backup #3459

[CARBONDATA-3582] support table status file backup #3459

Conversation

jackylk commented Nov 14, 2019 • edited

CarbonDataQA commented Nov 14, 2019

CarbonDataQA commented Nov 14, 2019

CarbonDataQA commented Nov 14, 2019

CarbonDataQA commented Nov 15, 2019

CarbonDataQA commented Nov 15, 2019

CarbonDataQA commented Nov 15, 2019

jackylk commented Nov 15, 2019

CarbonDataQA commented Nov 15, 2019

CarbonDataQA commented Nov 15, 2019

CarbonDataQA commented Nov 15, 2019

ajantha-bhat commented Nov 15, 2019

CarbonDataQA commented Nov 16, 2019

CarbonDataQA commented Nov 16, 2019

CarbonDataQA commented Nov 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarbonDataQA commented Nov 17, 2019

CarbonDataQA commented Nov 17, 2019

CarbonDataQA commented Nov 17, 2019

jackylk commented Nov 17, 2019 • edited

xubo245 commented Nov 17, 2019

CarbonDataQA commented Nov 17, 2019

CarbonDataQA commented Nov 17, 2019

CarbonDataQA commented Nov 17, 2019

CarbonDataQA commented Nov 18, 2019

CarbonDataQA commented Nov 18, 2019

CarbonDataQA commented Nov 18, 2019

CarbonDataQA commented Nov 18, 2019

jackylk commented Nov 24, 2019

CarbonDataQA commented Nov 24, 2019

CarbonDataQA commented Nov 24, 2019

CarbonDataQA commented Nov 24, 2019

jackylk commented Nov 28, 2019

CarbonDataQA1 commented Nov 28, 2019

CarbonDataQA1 commented Nov 28, 2019

CarbonDataQA1 commented Nov 28, 2019

CarbonDataQA1 commented Dec 16, 2019

CarbonDataQA1 commented Dec 16, 2019

CarbonDataQA1 commented Dec 16, 2019

QiangCai commented Dec 16, 2019

brijoobopanna commented Dec 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kumarvishal09 commented Dec 16, 2019

jackylk commented Dec 21, 2019

jackylk commented Dec 21, 2019

CarbonDataQA1 commented Dec 21, 2019

CarbonDataQA1 commented Dec 21, 2019

CarbonDataQA1 commented Dec 21, 2019

ajantha-bhat commented Dec 24, 2019

jackylk commented Nov 14, 2019 •

edited

jackylk commented Nov 17, 2019 •

edited