You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by manishgupta88 <gi...@git.apache.org> on 2018/08/07 08:24:14 UTC

[GitHub] carbondata pull request #2613: [HOTFIX] Modified code to fix the degrade in ...

GitHub user manishgupta88 opened a pull request:

    https://github.com/apache/carbondata/pull/2613

    [HOTFIX] Modified code to fix the degrade in compaction performance

    Problem
    Compaction performance for 3.5 billion degraded by 16-20%
    
    Analysis:
    Code modification in RawResultIerator.java has caused the problem wherein few extra checks are performed as compared to previous code
    
    Fix
    Revert the changes in RawResultIerator class
    
     - [ ] Any interfaces changed?
     No
     - [ ] Any backward compatibility impacted?
     NA
     - [ ] Document update required?
    No
     - [ ] Testing done
    Verified in cluster testing       
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. 
    NA

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

    $ git pull https://github.com/manishgupta88/carbondata revert_pr_2133

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

    https://github.com/apache/carbondata/pull/2613.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 #2613
    
----
commit 1481759abb7e8728bca95a557ce610f6116a2652
Author: m00258959 <ma...@...>
Date:   2018-08-07T05:24:16Z

    Problem
    Compaction performance for 3.5 billion degraded by 16-20%
    
    Analysis:
    Code modification in RawResultIerator.java has caused the problem wherein few extra checks are performed as compared to previous code
    
    Fix
    Revert the changes in RawResultIerator class

----


---

[GitHub] carbondata pull request #2613: [HOTFIX] Modified code to fix the degrade in ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/2613


---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7822/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6196/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7815/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    LGTM


---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6553/



---

[GitHub] carbondata pull request #2613: [HOTFIX] Modified code to fix the degrade in ...

Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2613#discussion_r208182587
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/result/iterator/RawResultIterator.java ---
    @@ -53,153 +39,124 @@
        */
       private CarbonIterator<RowBatch> detailRawQueryResultIterator;
     
    -  private boolean prefetchEnabled;
    -  private List<Object[]> currentBuffer;
    -  private List<Object[]> backupBuffer;
    -  private int currentIdxInBuffer;
    -  private ExecutorService executorService;
    -  private Future<Void> fetchFuture;
    -  private Object[] currentRawRow = null;
    -  private boolean isBackupFilled = false;
    +  /**
    +   * Counter to maintain the row counter.
    +   */
    +  private int counter = 0;
    +
    +  private Object[] currentConveretedRawRow = null;
    +
    +  /**
    +   * LOGGER
    +   */
    +  private static final LogService LOGGER =
    +      LogServiceFactory.getLogService(RawResultIterator.class.getName());
    +
    +  /**
    +   * batch of the result.
    +   */
    +  private RowBatch batch;
     
       public RawResultIterator(CarbonIterator<RowBatch> detailRawQueryResultIterator,
    -      SegmentProperties sourceSegProperties, SegmentProperties destinationSegProperties,
    -      boolean isStreamingHandOff) {
    +      SegmentProperties sourceSegProperties, SegmentProperties destinationSegProperties) {
         this.detailRawQueryResultIterator = detailRawQueryResultIterator;
         this.sourceSegProperties = sourceSegProperties;
         this.destinationSegProperties = destinationSegProperties;
    -    this.executorService = Executors.newFixedThreadPool(1);
    -
    -    if (!isStreamingHandOff) {
    -      init();
    -    }
       }
     
    -  private void init() {
    -    this.prefetchEnabled = CarbonProperties.getInstance().getProperty(
    -        CarbonCommonConstants.CARBON_COMPACTION_PREFETCH_ENABLE,
    -        CarbonCommonConstants.CARBON_COMPACTION_PREFETCH_ENABLE_DEFAULT).equalsIgnoreCase("true");
    -    try {
    -      new RowsFetcher(false).call();
    -      if (prefetchEnabled) {
    -        this.fetchFuture = executorService.submit(new RowsFetcher(true));
    -      }
    -    } catch (Exception e) {
    -      LOGGER.error(e, "Error occurs while fetching records");
    -      throw new RuntimeException(e);
    -    }
    -  }
    +  @Override public boolean hasNext() {
     
    -  /**
    -   * fetch rows
    -   */
    -  private final class RowsFetcher implements Callable<Void> {
    -    private boolean isBackupFilling;
    -
    -    private RowsFetcher(boolean isBackupFilling) {
    -      this.isBackupFilling = isBackupFilling;
    -    }
    -
    -    @Override
    -    public Void call() throws Exception {
    -      if (isBackupFilling) {
    -        backupBuffer = fetchRows();
    -        isBackupFilled = true;
    +    if (null == batch || checkIfBatchIsProcessedCompletely(batch)) {
    +      if (detailRawQueryResultIterator.hasNext()) {
    +        batch = null;
    +        batch = detailRawQueryResultIterator.next();
    +        counter = 0; // batch changed so reset the counter.
           } else {
    -        currentBuffer = fetchRows();
    +        return false;
           }
    -      return null;
         }
    -  }
     
    -  private List<Object[]> fetchRows() {
    -    if (detailRawQueryResultIterator.hasNext()) {
    -      return detailRawQueryResultIterator.next().getRows();
    +    if (!checkIfBatchIsProcessedCompletely(batch)) {
    +      return true;
         } else {
    -      return new ArrayList<>();
    +      return false;
         }
       }
     
    -  private void fillDataFromPrefetch() {
    -    try {
    -      if (currentIdxInBuffer >= currentBuffer.size() && 0 != currentIdxInBuffer) {
    -        if (prefetchEnabled) {
    -          if (!isBackupFilled) {
    -            fetchFuture.get();
    -          }
    -          // copy backup buffer to current buffer and fill backup buffer asyn
    -          currentIdxInBuffer = 0;
    -          currentBuffer = backupBuffer;
    -          isBackupFilled = false;
    -          fetchFuture = executorService.submit(new RowsFetcher(true));
    -        } else {
    -          currentIdxInBuffer = 0;
    -          new RowsFetcher(false).call();
    +  @Override public Object[] next() {
    --- End diff --
    
    ok


---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6531/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6184/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6189/



---

[GitHub] carbondata pull request #2613: [HOTFIX] Modified code to fix the degrade in ...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2613#discussion_r208178398
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/result/iterator/RawResultIterator.java ---
    @@ -53,153 +39,124 @@
        */
       private CarbonIterator<RowBatch> detailRawQueryResultIterator;
     
    -  private boolean prefetchEnabled;
    -  private List<Object[]> currentBuffer;
    -  private List<Object[]> backupBuffer;
    -  private int currentIdxInBuffer;
    -  private ExecutorService executorService;
    -  private Future<Void> fetchFuture;
    -  private Object[] currentRawRow = null;
    -  private boolean isBackupFilled = false;
    +  /**
    +   * Counter to maintain the row counter.
    +   */
    +  private int counter = 0;
    +
    +  private Object[] currentConveretedRawRow = null;
    +
    +  /**
    +   * LOGGER
    +   */
    +  private static final LogService LOGGER =
    +      LogServiceFactory.getLogService(RawResultIterator.class.getName());
    +
    +  /**
    +   * batch of the result.
    +   */
    +  private RowBatch batch;
     
       public RawResultIterator(CarbonIterator<RowBatch> detailRawQueryResultIterator,
    -      SegmentProperties sourceSegProperties, SegmentProperties destinationSegProperties,
    -      boolean isStreamingHandOff) {
    +      SegmentProperties sourceSegProperties, SegmentProperties destinationSegProperties) {
         this.detailRawQueryResultIterator = detailRawQueryResultIterator;
         this.sourceSegProperties = sourceSegProperties;
         this.destinationSegProperties = destinationSegProperties;
    -    this.executorService = Executors.newFixedThreadPool(1);
    -
    -    if (!isStreamingHandOff) {
    -      init();
    -    }
       }
     
    -  private void init() {
    -    this.prefetchEnabled = CarbonProperties.getInstance().getProperty(
    -        CarbonCommonConstants.CARBON_COMPACTION_PREFETCH_ENABLE,
    -        CarbonCommonConstants.CARBON_COMPACTION_PREFETCH_ENABLE_DEFAULT).equalsIgnoreCase("true");
    -    try {
    -      new RowsFetcher(false).call();
    -      if (prefetchEnabled) {
    -        this.fetchFuture = executorService.submit(new RowsFetcher(true));
    -      }
    -    } catch (Exception e) {
    -      LOGGER.error(e, "Error occurs while fetching records");
    -      throw new RuntimeException(e);
    -    }
    -  }
    +  @Override public boolean hasNext() {
     
    -  /**
    -   * fetch rows
    -   */
    -  private final class RowsFetcher implements Callable<Void> {
    -    private boolean isBackupFilling;
    -
    -    private RowsFetcher(boolean isBackupFilling) {
    -      this.isBackupFilling = isBackupFilling;
    -    }
    -
    -    @Override
    -    public Void call() throws Exception {
    -      if (isBackupFilling) {
    -        backupBuffer = fetchRows();
    -        isBackupFilled = true;
    +    if (null == batch || checkIfBatchIsProcessedCompletely(batch)) {
    +      if (detailRawQueryResultIterator.hasNext()) {
    +        batch = null;
    +        batch = detailRawQueryResultIterator.next();
    +        counter = 0; // batch changed so reset the counter.
           } else {
    -        currentBuffer = fetchRows();
    +        return false;
           }
    -      return null;
         }
    -  }
     
    -  private List<Object[]> fetchRows() {
    -    if (detailRawQueryResultIterator.hasNext()) {
    -      return detailRawQueryResultIterator.next().getRows();
    +    if (!checkIfBatchIsProcessedCompletely(batch)) {
    +      return true;
         } else {
    -      return new ArrayList<>();
    +      return false;
         }
       }
     
    -  private void fillDataFromPrefetch() {
    -    try {
    -      if (currentIdxInBuffer >= currentBuffer.size() && 0 != currentIdxInBuffer) {
    -        if (prefetchEnabled) {
    -          if (!isBackupFilled) {
    -            fetchFuture.get();
    -          }
    -          // copy backup buffer to current buffer and fill backup buffer asyn
    -          currentIdxInBuffer = 0;
    -          currentBuffer = backupBuffer;
    -          isBackupFilled = false;
    -          fetchFuture = executorService.submit(new RowsFetcher(true));
    -        } else {
    -          currentIdxInBuffer = 0;
    -          new RowsFetcher(false).call();
    +  @Override public Object[] next() {
    --- End diff --
    
    Move @Override to previous line


---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7807/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7827/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6539/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6191/



---

[GitHub] carbondata issue #2613: [HOTFIX] Modified code to fix the degrade in compact...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2613
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6546/



---