[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-02 Thread ravipesala
Github user ravipesala commented on the issue:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-02 Thread ravipesala
Github user ravipesala commented on the issue:

https://github.com/apache/carbondata/pull/978
  
@manishgupta88 It was the case in V1 and V2 formats, but in V3 format we 
should launch producer at finish to flush the older pages it was holding.  


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread CarbonDataQA
Github user CarbonDataQA commented on the issue:

https://github.com/apache/carbondata/pull/978
  
Build Success with Spark 2.1.0, Please check CI 
http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2147/



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread watermen
Github user watermen commented on the issue:

https://github.com/apache/carbondata/pull/978
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread manishgupta88
Github user manishgupta88 commented on the issue:

https://github.com/apache/carbondata/pull/978
  
@ravipesala Please correct me if I am wrongdo we need to acquire a 
semaphore lock if we do not have record to be processedI think if the rows 
are exactly divisible we will not have any extra rows to be processed and hence 
we do not require to acquire semaphore lock and create a new Producer object


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread CarbonDataQA
Github user CarbonDataQA commented on the issue:

https://github.com/apache/carbondata/pull/978
  
Build Failed  with Spark 2.1.0, Please check CI 
http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2146/



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread ravipesala
Github user ravipesala commented on the issue:

https://github.com/apache/carbondata/pull/978
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread ravipesala
Github user ravipesala commented on the issue:

https://github.com/apache/carbondata/pull/978
  
Not required any check, entry count check handled inside

On Fri, 2 Jun 2017 at 11:13 AM, manishgupta88 
wrote:

> *@manishgupta88* commented on this pull request.
> --
>
> In
> 
processing/src/main/java/org/apache/carbondata/processing/store/CarbonFactDataHandlerColumnar.java
> :
>
> > @@ -488,13 +488,19 @@ private NodeHolder 
processDataRows(List dataRows)
>public void finish() throws CarbonDataWriterException {
>  // still some data is present in stores if entryCount is more
>  // than 0
> -producerExecutorServiceTaskList.add(producerExecutorService
> -.submit(new Producer(blockletDataHolder, dataRows, 
++writerTaskSequenceCounter, true)));
> -blockletProcessingCount.incrementAndGet();
> -processedDataCount += entryCount;
> -closeWriterExecutionService(producerExecutorService);
> -processWriteTaskSubmitList(producerExecutorServiceTaskList);
> -processingComplete = true;
> +try {
> +  semaphore.acquire();
>
> @watermen  .here a check is required for
> entryCount > 0.because we need to acquire a semaphore lock only if
> total number of rows in raw data are not exactly divisible by page
> size...in this case only we will have some extra rows to be process by
> finish method else addDataToStore method will handle all the 
rowsPlease
> refer the below code snippet
>
> public void finish() throws CarbonDataWriterException {
> // still some data is present in stores if entryCount is more
> // than 0
> if (this.entryCount > 0) {
> try {
> semaphore.acquire();
> producerExecutorServiceTaskList.add(producerExecutorService
> .submit(new Producer(blockletDataHolder, dataRows,
> ++writerTaskSequenceCounter, true)));
> blockletProcessingCount.incrementAndGet();
> processedDataCount += entryCount;
> } catch (InterruptedException e) {
> LOGGER.error(e, e.getMessage());
> throw new CarbonDataWriterException(e.getMessage(), e);
> }
> }
> closeWriterExecutionService(producerExecutorService);
> processWriteTaskSubmitList(producerExecutorServiceTaskList);
> processingComplete = true;
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] carbondata issue #978: [CARBONDATA-1109] Acquire semaphore before submit a p...

2017-06-01 Thread CarbonDataQA
Github user CarbonDataQA commented on the issue:

https://github.com/apache/carbondata/pull/978
  
Build Failed  with Spark 2.1.0, Please check CI 
http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2140/



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---