[GitHub] [carbondata] Kejian-Li opened a new pull request #3947: [CARBONDATA-4003] Improve IUD Concurrency

[GitBox]

Kejian-Li commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-699796397


   @QiangCai please help review this PR


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on a change in pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#discussion_r495757998



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##########
@@ -192,9 +192,10 @@ case class CarbonCreateMVCommand(
           throw new MalformedCarbonCommandException(
             "Cannot create mv on stream table " + table.getTableUniqueName)
         }
-        if (SegmentStatusManager.isLoadInProgressInTable(table)) {
+        if (SegmentStatusManager.isOverwriteInProgressInTable(table)) {

Review comment:
       why change it?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-699851048


   better to focus on the topic in this PR.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li commented on a change in pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#discussion_r495781902



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##########
@@ -192,9 +192,10 @@ case class CarbonCreateMVCommand(
           throw new MalformedCarbonCommandException(
             "Cannot create mv on stream table " + table.getTableUniqueName)
         }
-        if (SegmentStatusManager.isLoadInProgressInTable(table)) {
+        if (SegmentStatusManager.isOverwriteInProgressInTable(table)) {

Review comment:
       Create MV While load (LOAD or INSERT INTO) is in progress could also be allowed, I think, and there is corresponding test to justify it in test class "TestCreateMVWithTimeSeries", so I changed the method. What's your opinion aboout it?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-699871798


   > better to focus on the topic in this PR.
   
   Got it, I will follow your suggestion


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li commented on a change in pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#discussion_r495781902



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##########
@@ -192,9 +192,10 @@ case class CarbonCreateMVCommand(
           throw new MalformedCarbonCommandException(
             "Cannot create mv on stream table " + table.getTableUniqueName)
         }
-        if (SegmentStatusManager.isLoadInProgressInTable(table)) {
+        if (SegmentStatusManager.isOverwriteInProgressInTable(table)) {

Review comment:
       I have reverted it this.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li commented on a change in pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#discussion_r495781902



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##########
@@ -192,9 +192,10 @@ case class CarbonCreateMVCommand(
           throw new MalformedCarbonCommandException(
             "Cannot create mv on stream table " + table.getTableUniqueName)
         }
-        if (SegmentStatusManager.isLoadInProgressInTable(table)) {
+        if (SegmentStatusManager.isOverwriteInProgressInTable(table)) {

Review comment:
       I have reverted it.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700093868


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700098125


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


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700366456


   @ajantha-bhat @akashrn5 please help review this PR


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li removed a comment on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700366456


   @ajantha-bhat @akashrn5 please help review this PR


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700373270


   please check mail list:
   http://apache-carbondata-dev-mailing-list-archive.1130556.n5.nabble.com/DISCUSSION-Parallel-compaction-and-update-td100338.html
   
   How do you process the parallel compaction(not only loading) and update?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai edited a comment on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700373270


   please check mail list:
   http://apache-carbondata-dev-mailing-list-archive.1130556.n5.nabble.com/DISCUSSION-Parallel-compaction-and-update-td100338.html
   
   How do we process the parallel compaction(not only loading) and update?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

akashrn5 commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700489664


   > please check mail list:
   > http://apache-carbondata-dev-mailing-list-archive.1130556.n5.nabble.com/DISCUSSION-Parallel-compaction-and-update-td100338.html
   >
   > How do we process the parallel compaction(not only loading) and update?
   
   I involved in that solution derivation, But its recommended and community's suggestion to start working on it after segment refactoring.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

marchpure commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700490385


   LGTM


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

akashrn5 commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700503471


   @Kejian-Li from the PR description, you have mentioned the problem is we cannot run the update to table when the insert is in progress, which is valid, that's because we want to maintain the data consistency. But you suggest we remove that check to allow the update. How do you plan to manage the data consistency when the insert in progress? You have not mentioned clearly in the description and its not clear from changes also.
   
   As @Zhangshunyu asked same doubt in above comments, there is no clear reply. Can you please explain the solution you have given here?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

akashrn5 commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700534709


   @Kejian-Li i can just see the test file changes and no source file changes for the issue which you described. So im confused what exactly or how exactly the problem is solved. can you please explain?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li commented on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700697188


   
   
   
   
   > @Kejian-Li i can just see the test file changes and no source file changes for the issue which you described. So im confused what exactly or how exactly the problem is solved. can you please explain?
   
   My idea and implementation is not considerable, so I just removed the relative code concurrency
   
   > @Kejian-Li i can just see the test file changes and no source file changes for the issue which you described. So im confused what exactly or how exactly the problem is solved. can you please explain?
   
   My idea and implementation is not considerable, I will explain this later.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li edited a comment on pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947#issuecomment-700697188


   > @Kejian-Li i can just see the test file changes and no source file changes for the issue which you described. So im confused what exactly or how exactly the problem is solved. can you please explain?
   
   My idea and implementation is not considerable, I will explain this later.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Kejian-Li closed pull request #3947:
URL: https://github.com/apache/carbondata/pull/3947


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]