[CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse… #3977

marchpure · 2020-10-12T02:00:57Z

…rt stage

Why is this PR needed?

ISSUE1: In the insertstage flow, there is a empty file with suffix '.loading' to mark the stage in the status of 'in processing'. We update the modifiedtime of '.loading' file for monitoring the insertstage start time, which can be used for calculate TIMEOUT, help to retry and recovery.
Before, we use setModifiedTime function to update the modifiedtime, which has a serious bug.
For S3 file, setModifiedTime operation do not take effect. leading to the incorrect inserstage starttime of 'loading' file.

ISSUE2: For now, Insertstage non-parttion table will not merge index files, which will degrade the query performance heavily.

What changes were proposed in this PR?

Update the modifiedtime of loading files based on recreating files.
Trigger indexfiles merging after insertstage non-parttiontable.

Does this PR introduce any user interface change?

No

Is any new testcase added?

No

marchpure · 2020-10-12T02:08:48Z

retest this please

CarbonDataQA1 · 2020-10-12T04:02:55Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2610/

CarbonDataQA1 · 2020-10-12T04:04:51Z

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

marchpure · 2020-10-13T03:52:13Z

retest this please

Indhumathi27 · 2020-10-13T05:02:20Z

@marchpure If setLastModifiedTime operation do not take effect on S3, in other places also, we need to check and update

Indhumathi27 · 2020-10-13T05:11:28Z

...n/scala/org/apache/spark/sql/execution/command/management/CarbonInsertFromStageCommand.scala

+            // Try to recreate loading files if the loading file exists
+            // or create loading files directly if the loading file doesn't exist
+            // set isFailed to be false when (delete and) createfile success
+            var isFailed = if (stageLoadingFile.exists()) {


isFailed will always be false. can remove it and update the comment

if IOException happend in stageLoadingFile.createNewFile(), createNewFile() will return FALE, make isFailed to be TRUE

CarbonDataQA1 · 2020-10-13T05:43:51Z

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

CarbonDataQA1 · 2020-10-13T05:47:30Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2642/

CarbonDataQA1 · 2020-10-13T10:36:45Z

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

CarbonDataQA1 · 2020-10-13T10:39:22Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2647/

marchpure · 2020-10-13T12:50:33Z

retest this please

Indhumathi27 · 2020-10-13T13:22:53Z

...n/scala/org/apache/spark/sql/execution/command/management/CarbonInsertFromStageCommand.scala

+            // Try to recreate loading files if the loading file exists
+            // or create loading files directly if the loading file doesn't exist
+            // set isFailed to be false when (delete and) createfile success
+            var isFailed = if (stageLoadingFile.exists()) {


Suggested change

var isFailed = if (stageLoadingFile.exists()) {

val isFailed = if (stageLoadingFile.exists()) {

I have modified code according to your suggestion

Indhumathi27 · 2020-10-13T14:08:14Z

core/src/main/java/org/apache/carbondata/core/view/MVProvider.java

-        FileFactory.createNewFile(
-            this.schemaIndexFilePath,
-            new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+      if (FileFactory.isFileExist(this.schemaIndexFilePath)) {


Suggested change

if (FileFactory.isFileExist(this.schemaIndexFilePath)) {

CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);

if (schemaIndexFile.exists()) {

schemaIndexFile.delete();

}

schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));

this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();

I have modified code according to your suggestion

CarbonDataQA1 · 2020-10-13T14:41:07Z

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

CarbonDataQA1 · 2020-10-13T14:41:39Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2652/

CarbonDataQA1 · 2020-10-13T17:12:04Z

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

CarbonDataQA1 · 2020-10-13T17:12:13Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2655/

CarbonDataQA1 · 2020-10-13T19:34:07Z

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

CarbonDataQA1 · 2020-10-13T19:37:54Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2659/

CarbonDataQA1 · 2020-10-14T01:09:30Z

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

CarbonDataQA1 · 2020-10-14T01:16:46Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2663/

marchpure · 2020-10-14T02:07:06Z

retest this please

CarbonDataQA1 · 2020-10-14T03:55:46Z

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

CarbonDataQA1 · 2020-10-14T04:13:21Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2665/

marchpure · 2020-10-14T04:28:55Z

retest this please

CarbonDataQA1 · 2020-10-20T21:26:33Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2791/

CarbonDataQA1 · 2020-10-20T23:34:58Z

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

CarbonDataQA1 · 2020-10-21T02:51:22Z

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

CarbonDataQA1 · 2020-10-21T03:01:50Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2802/

CarbonDataQA1 · 2020-10-21T06:38:01Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2812/

CarbonDataQA1 · 2020-10-21T06:40:50Z

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

marchpure · 2020-10-21T08:56:22Z

retest this please

CarbonDataQA1 · 2020-10-21T09:47:48Z

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

CarbonDataQA1 · 2020-10-21T09:48:18Z

Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2826/

QiangCai · 2020-10-23T02:04:50Z

retest this please

CarbonDataQA1 · 2020-10-23T03:48:23Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2894/

CarbonDataQA1 · 2020-10-23T03:48:45Z

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

Indhumathi27 · 2020-10-23T09:35:42Z

@marchpure please update the PR description for MergeIndex changes also

Indhumathi27 · 2020-10-23T09:38:10Z

integration/spark/src/main/scala/org/apache/spark/sql/events/MergeIndexEventListener.scala

@@ -43,11 +43,6 @@ class MergeIndexEventListener extends OperationEventListener with Logging {
  override def onEvent(event: Event, operationContext: OperationContext): Unit = {
    event match {
      case preStatusUpdateEvent: LoadTablePreStatusUpdateEvent =>
-        // skip merge index in case of insert stage flow
-        if (null != operationContext.getProperty(CarbonCommonConstants.IS_INSERT_STAGE) &&


This property also has to be removed from CarbonCommonConstants, as no more usage will be found

I have modified code according to your suggestion

…rt stage Why is this PR needed? In the insertstage flow, there is a empty file with suffix '.loading' to mark the stage in the status of 'in processing'. We update the modifiedtime of '.loading' file for monitoring the insertstage start time, which can be used for calculate TIMEOUT, help to retry and recovery. Before, we use setModifiedTime function to update the modifiedtime, which has a serious bug. For S3 file, setModifiedTime operation do not take effect. leading to the incorrect inserstage starttime of 'loading' file. What changes were proposed in this PR? Update the modifiedtime of loading files based on recreating files. Does this PR introduce any user interface change? No Is any new testcase added? No

CarbonDataQA1 · 2020-10-23T14:25:23Z

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

CarbonDataQA1 · 2020-10-23T14:26:09Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2914/

marchpure · 2020-10-24T08:47:01Z

retest this please

CarbonDataQA1 · 2020-10-24T10:25:35Z

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

CarbonDataQA1 · 2020-10-24T10:32:03Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2919/

marchpure · 2020-10-26T08:30:52Z

@marchpure please update the PR description for MergeIndex changes also

I have updated the PR description for MergeIndex changes

Indhumathi27 · 2020-10-26T12:06:25Z

retest this please

CarbonDataQA1 · 2020-10-26T13:45:27Z

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

CarbonDataQA1 · 2020-10-26T13:46:21Z

Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2935/

Indhumathi27 · 2020-10-26T14:44:13Z

LGTM

marchpure force-pushed the insertstageloading branch from 008e235 to 37ca6d5 Compare October 12, 2020 02:04

Indhumathi27 reviewed Oct 13, 2020

View reviewed changes

marchpure force-pushed the insertstageloading branch from 37ca6d5 to 3a43207 Compare October 13, 2020 08:42

Indhumathi27 reviewed Oct 13, 2020

View reviewed changes

marchpure force-pushed the insertstageloading branch from 3a43207 to 232fff4 Compare October 13, 2020 15:21

marchpure force-pushed the insertstageloading branch from 232fff4 to 96f4074 Compare October 13, 2020 17:39

marchpure force-pushed the insertstageloading branch from 96f4074 to 8995066 Compare October 13, 2020 23:22

marchpure force-pushed the insertstageloading branch from 7fe6cb9 to 1dee759 Compare October 21, 2020 01:02

marchpure force-pushed the insertstageloading branch from 1dee759 to 182a8cc Compare October 21, 2020 03:27

Indhumathi27 reviewed Oct 23, 2020

View reviewed changes

marchpure force-pushed the insertstageloading branch from 182a8cc to e8706db Compare October 23, 2020 12:45

asfgit closed this in f5118f9 Oct 26, 2020

	var isFailed = if (stageLoadingFile.exists()) {
	val isFailed = if (stageLoadingFile.exists()) {

-      if (FileFactory.isFileExist(this.schemaIndexFilePath)) {
+      CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
+      if (schemaIndexFile.exists()) {
+        schemaIndexFile.delete();
+      }
+      schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+      this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();

[CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse… #3977

[CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse… #3977

Conversation

marchpure commented Oct 12, 2020 • edited

Why is this PR needed?

What changes were proposed in this PR?

Does this PR introduce any user interface change?

Is any new testcase added?

marchpure commented Oct 12, 2020

CarbonDataQA1 commented Oct 12, 2020

CarbonDataQA1 commented Oct 12, 2020

marchpure commented Oct 13, 2020

Indhumathi27 commented Oct 13, 2020

Indhumathi27 Oct 13, 2020 • edited

Choose a reason for hiding this comment

marchpure Oct 13, 2020

Choose a reason for hiding this comment

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

marchpure commented Oct 13, 2020

Indhumathi27 Oct 13, 2020

Choose a reason for hiding this comment

marchpure Oct 14, 2020

Choose a reason for hiding this comment

Indhumathi27 Oct 13, 2020

Choose a reason for hiding this comment

marchpure Oct 14, 2020

Choose a reason for hiding this comment

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 13, 2020

CarbonDataQA1 commented Oct 14, 2020

CarbonDataQA1 commented Oct 14, 2020

marchpure commented Oct 14, 2020

CarbonDataQA1 commented Oct 14, 2020

CarbonDataQA1 commented Oct 14, 2020

marchpure commented Oct 14, 2020

CarbonDataQA1 commented Oct 20, 2020

CarbonDataQA1 commented Oct 20, 2020

CarbonDataQA1 commented Oct 21, 2020

CarbonDataQA1 commented Oct 21, 2020

CarbonDataQA1 commented Oct 21, 2020

CarbonDataQA1 commented Oct 21, 2020

marchpure commented Oct 21, 2020

CarbonDataQA1 commented Oct 21, 2020

CarbonDataQA1 commented Oct 21, 2020

QiangCai commented Oct 23, 2020

CarbonDataQA1 commented Oct 23, 2020

CarbonDataQA1 commented Oct 23, 2020

Indhumathi27 commented Oct 23, 2020

Indhumathi27 Oct 23, 2020

Choose a reason for hiding this comment

marchpure Oct 26, 2020

Choose a reason for hiding this comment

CarbonDataQA1 commented Oct 23, 2020

CarbonDataQA1 commented Oct 23, 2020

marchpure commented Oct 24, 2020

CarbonDataQA1 commented Oct 24, 2020

CarbonDataQA1 commented Oct 24, 2020

marchpure commented Oct 26, 2020

Indhumathi27 commented Oct 26, 2020

CarbonDataQA1 commented Oct 26, 2020

CarbonDataQA1 commented Oct 26, 2020

Indhumathi27 commented Oct 26, 2020

marchpure commented Oct 12, 2020 •

edited

Indhumathi27 Oct 13, 2020 •

edited