[GitHub] incubator-carbondata pull request #571: [CARBONDATA-680]Added stats like row...

[qiuchenjian-2]

GitHub user ravipesala opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/571

    [CARBONDATA-680]Added stats like rows processed in each step. And also fixes unsafe sort enable issue.

    Currently stats like number of rows processed in each step is not added in no kettle flow. This PR adds them.
    And also unsafe sort is not enabling even though user enable the sort in property file.This PR fixes it.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ravipesala/incubator-carbondata no-kettle-stats

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-carbondata/pull/571.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #571
   
----
commit 2026685dfe15b4e2d4a362c4d309ce51d516bf12
Author: ravipesala <[hidden email]>
Date:   2017-01-25T10:01:56Z

    Added stats like rows processed in each step. And also fixes unsafe sort enable issue.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/749/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/750/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/751/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/752/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    please rebase


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/758/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98158805
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/newflow/AbstractDataLoadProcessorStep.java ---
    @@ -36,14 +39,37 @@
      */
     public abstract class AbstractDataLoadProcessorStep {
     
    +  private static final LogService LOGGER =
    +      LogServiceFactory.getLogService(AbstractDataLoadProcessorStep.class.getName());
    +
       protected CarbonDataLoadConfiguration configuration;
     
       protected AbstractDataLoadProcessorStep child;
     
    +  protected AtomicLong rowCounter;
    +
    +  private boolean closed;
    +
       public AbstractDataLoadProcessorStep(CarbonDataLoadConfiguration configuration,
           AbstractDataLoadProcessorStep child) {
         this.configuration = configuration;
         this.child = child;
    +    this.rowCounter = new AtomicLong();
    +    this.closed = false;
    +
    +    new Thread() {
    --- End diff --
   
    please add comment description for this thread


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/760/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98161894
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/newflow/AbstractDataLoadProcessorStep.java ---
    @@ -36,14 +39,37 @@
      */
     public abstract class AbstractDataLoadProcessorStep {
     
    +  private static final LogService LOGGER =
    +      LogServiceFactory.getLogService(AbstractDataLoadProcessorStep.class.getName());
    +
       protected CarbonDataLoadConfiguration configuration;
     
       protected AbstractDataLoadProcessorStep child;
     
    +  protected AtomicLong rowCounter;
    +
    +  private boolean closed;
    +
       public AbstractDataLoadProcessorStep(CarbonDataLoadConfiguration configuration,
           AbstractDataLoadProcessorStep child) {
         this.configuration = configuration;
         this.child = child;
    +    this.rowCounter = new AtomicLong();
    +    this.closed = false;
    +
    +    new Thread() {
    --- End diff --
   
    In this way, even if the info log is not enabled, this thread will be created.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98162769
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/CarbonLateDecodeStrategy.scala ---
    @@ -484,12 +485,14 @@ private[sql] class CarbonLateDecodeStrategy extends SparkStrategy {
         val enableReader = {
    --- End diff --
   
    suggest change name to vectorizedReader


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98162981
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/CarbonLateDecodeStrategy.scala ---
    @@ -484,12 +485,14 @@ private[sql] class CarbonLateDecodeStrategy extends SparkStrategy {
         val enableReader = {
           if (sqlContext.sparkSession.conf.contains(CarbonCommonConstants.ENABLE_VECTOR_READER)) {
             sqlContext.sparkSession.conf.get(CarbonCommonConstants.ENABLE_VECTOR_READER)
    +      } else if (System.getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER) != null) {
    +        System.getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER)
           } else {
    -        System.getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER,
    +        CarbonProperties.getInstance().getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER,
               CarbonCommonConstants.ENABLE_VECTOR_READER_DEFAULT)
           }
         }
         sqlContext.conf.wholeStageEnabled && enableReader.toBoolean &&
    -      cols.forall(_.dataType.isInstanceOf[AtomicType])
    +    cols.forall(_.dataType.isInstanceOf[AtomicType])
    --- End diff --
   
    This style is not correct, right? It is a condition followed last line


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98164700
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/CarbonLateDecodeStrategy.scala ---
    @@ -484,12 +485,14 @@ private[sql] class CarbonLateDecodeStrategy extends SparkStrategy {
         val enableReader = {
           if (sqlContext.sparkSession.conf.contains(CarbonCommonConstants.ENABLE_VECTOR_READER)) {
             sqlContext.sparkSession.conf.get(CarbonCommonConstants.ENABLE_VECTOR_READER)
    +      } else if (System.getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER) != null) {
    +        System.getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER)
           } else {
    -        System.getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER,
    +        CarbonProperties.getInstance().getProperty(CarbonCommonConstants.ENABLE_VECTOR_READER,
               CarbonCommonConstants.ENABLE_VECTOR_READER_DEFAULT)
           }
         }
         sqlContext.conf.wholeStageEnabled && enableReader.toBoolean &&
    -      cols.forall(_.dataType.isInstanceOf[AtomicType])
    +    cols.forall(_.dataType.isInstanceOf[AtomicType])
    --- End diff --
   
    0k


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98164711
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/CarbonLateDecodeStrategy.scala ---
    @@ -484,12 +485,14 @@ private[sql] class CarbonLateDecodeStrategy extends SparkStrategy {
         val enableReader = {
    --- End diff --
   
    ok


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/571#discussion_r98164805
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/newflow/AbstractDataLoadProcessorStep.java ---
    @@ -36,14 +39,37 @@
      */
     public abstract class AbstractDataLoadProcessorStep {
     
    +  private static final LogService LOGGER =
    +      LogServiceFactory.getLogService(AbstractDataLoadProcessorStep.class.getName());
    +
       protected CarbonDataLoadConfiguration configuration;
     
       protected AbstractDataLoadProcessorStep child;
     
    +  protected AtomicLong rowCounter;
    +
    +  private boolean closed;
    +
       public AbstractDataLoadProcessorStep(CarbonDataLoadConfiguration configuration,
           AbstractDataLoadProcessorStep child) {
         this.configuration = configuration;
         this.child = child;
    +    this.rowCounter = new AtomicLong();
    +    this.closed = false;
    +
    +    new Thread() {
    --- End diff --
   
    Ok, I will check the condition.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    @jackylk Fixed comments, please review


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/763/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/571
 
    Some problem with single-pass testcases , it hangs the jenkins, I had to manually stop the build.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---