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

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] default value of ENABLE_OFF...

GitHub user rahulforallp opened a pull request:

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

    [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SORT for sdk set as  false

    
    
    Be sure to do all of the following checklist to help us incorporate 
    your contribution quickly and easily:
    
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
    
     - [ ] Testing done
            Please provide details on 
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. 
    


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

    $ git pull https://github.com/rahulforallp/incubator-carbondata unsafe_mem_sdk

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

    https://github.com/apache/carbondata/pull/2274.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 #2274
    
----
commit 3d67d58737d099458fbfd050bb256918573b8610
Author: rahulforallp <ra...@...>
Date:   2018-05-04T14:09:58Z

    [CARBONDATA-2440] false set as default value of ENABLE_OFFHEAP_SORT for sdk

----


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189478703
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -347,6 +349,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +   * This method will be responsible to get the instance of CarbonProperties class
    --- End diff --
    
    please optimize the Scala Style: indent


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189588715
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
    it may be failing because of invalid path given in testSdkWriter().
    
    ![image](https://user-images.githubusercontent.com/14244942/40309994-0ebc1d0c-5d29-11e8-8b1f-90898bdbe783.png)



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] default value of ENABLE_OFF...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r187533693
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -340,7 +342,13 @@ private CarbonLoadModel createLoadModel() throws IOException, InvalidLoadOptionE
           // we are still using the traditional carbon table folder structure
           persistSchemaFile(table, CarbonTablePath.getSchemaFilePath(path));
         }
    -
    +    if (!table.isTransactionalTable()) {
    +      CarbonProperties.getInstance()
    +          .addProperty(CarbonCommonConstants.ENABLE_OFFHEAP_SORT, "false");
    --- End diff --
    
    @kunal642  if we are updating the doc to set the properties then no need of this code.  same PR we can use to update the doc.


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189576886
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
    --- End diff --
    
    done


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

    https://github.com/apache/carbondata/pull/2274
  
    retest this please


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189478551
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -32,6 +32,8 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", "false");
    --- End diff --
    
    it should input CarbonProperties first.


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189583189
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
    Didn't you rebase to the latest master?


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

    https://github.com/apache/carbondata/pull/2274
  
    @xubo245 import done


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189479058
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -347,6 +349,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +   * This method will be responsible to get the instance of CarbonProperties class
    +   *
    +   * @return carbon properties instance
    +   */
    +  public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +   * This method will be used to add a new property
    +   *
    +   * @param key
    +   * @return properties value
    +   */
    +  public CarbonProperties addProperty(String key, String value);
    +```
    +
    +```
    +/**
    +   * This method will be used to get the property value. if property is not
    +   * present then it will return the default value.
    --- End diff --
    
    Are there need a comma before the 'then'?


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189580805
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
    is this ok?


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189519161
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -32,6 +32,8 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", "false");
    --- End diff --
    
    this one not fix


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

    https://github.com/apache/carbondata/pull/2274
  
    @xubo245  review comments resolved .


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189787899
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if (args.length > 0 && args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    -     String path = "/home/root1/Documents/ab/temp";
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
    +     String path = "./target/testCSVSdkWriter";
      
          Field[] fields = new Field[2];
          fields[0] = new Field("name", DataTypes.STRING);
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", enableOffheap);
    --- End diff --
    
    It's different,validateEnableUnsafeSort method validate "enable.unsafe.sort", not "enable.offheap.sort".


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189576627
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
    --- End diff --
    
    Suggest use "Pass" instead of "pass".


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] default value of ENABLE_OFF...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r187368858
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -340,7 +342,13 @@ private CarbonLoadModel createLoadModel() throws IOException, InvalidLoadOptionE
           // we are still using the traditional carbon table folder structure
           persistSchemaFile(table, CarbonTablePath.getSchemaFilePath(path));
         }
    -
    +    if (!table.isTransactionalTable()) {
    +      CarbonProperties.getInstance()
    +          .addProperty(CarbonCommonConstants.ENABLE_OFFHEAP_SORT, "false");
    --- End diff --
    
    Intention was to prevent 'unsafe' property from being used by SDK user by default.  Otherwise they have to configure memory for unsafe also. So making it simple to use. 
    
    Any way we will update the doc how SDK user can set the property by following code:
    
    CarbonProperties.getInstance() .addProperty("property", "value");


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189584123
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
          String path = "/home/root1/Documents/ab/temp";
      
          Field[] fields = new Field[2];
          fields[0] = new Field("name", DataTypes.STRING);
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", enableOffheap);
      
          CarbonWriterBuilder builder = CarbonWriter.builder().withSchema(schema).outputPath(path);
    --- End diff --
    
    Can you optimize the write build process?  some methods have been changed, like withSchema and some.


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189588091
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
          String path = "/home/root1/Documents/ab/temp";
      
          Field[] fields = new Field[2];
          fields[0] = new Field("name", DataTypes.STRING);
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", enableOffheap);
      
          CarbonWriterBuilder builder = CarbonWriter.builder().withSchema(schema).outputPath(path);
    --- End diff --
    
    it  may be failing because of invalid path given in testSdkWriter().
    ![image](https://user-images.githubusercontent.com/14244942/40309877-b1f88696-5d28-11e8-83c5-912de1aae70f.png)
    



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189575742
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +     testSdkWriter(args[0]);
    --- End diff --
    
    that is already handled in CarbonProperties validation.


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] default value of ENABLE_OFF...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r187530598
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -340,7 +342,13 @@ private CarbonLoadModel createLoadModel() throws IOException, InvalidLoadOptionE
           // we are still using the traditional carbon table folder structure
           persistSchemaFile(table, CarbonTablePath.getSchemaFilePath(path));
         }
    -
    +    if (!table.isTransactionalTable()) {
    +      CarbonProperties.getInstance()
    +          .addProperty(CarbonCommonConstants.ENABLE_OFFHEAP_SORT, "false");
    --- End diff --
    
    But this will overwrite the ENABLE_OFFHEAP_SORT is the user has set it. Need to check if this is already set. If not then set to false otherwise use what the user has set.


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189604495
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
          String path = "/home/root1/Documents/ab/temp";
    --- End diff --
    
    done


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

    https://github.com/apache/carbondata/pull/2274
  
    please import CarbonProperties before using it.


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189582940
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
    Have you run this test case in you machine? I run it but failed


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

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


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189755954
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -334,6 +342,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +* This method will be responsible to get the instance of CarbonProperties class
    +*
    +* @return carbon properties instance
    +*/
    +public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +* This method will be used to add a new property
    +*
    +* @param key
    +* @return CarbonProperties object
    --- End diff --
    
    should add @param value  and describe


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189576814
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +     testSdkWriter(args[0]);
    --- End diff --
    
    done


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189756081
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -334,6 +342,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +* This method will be responsible to get the instance of CarbonProperties class
    +*
    +* @return carbon properties instance
    +*/
    +public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +* This method will be used to add a new property
    +*
    +* @param key
    +* @return CarbonProperties object
    +*/
    +public CarbonProperties addProperty(String key, String value);
    +```
    +
    +```
    +/**
    +* This method will be used to get the property value. If property is not
    +* present, then it will return the default value.
    +*
    +* @param key
    +* @return properties value
    --- End diff --
    
    Please add @param defaultValue and related describe.


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189575851
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
    --- End diff --
    
    "whle" is incorrect, please optimize.


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189479291
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -347,6 +349,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +   * This method will be responsible to get the instance of CarbonProperties class
    +   *
    +   * @return carbon properties instance
    +   */
    +  public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +   * This method will be used to add a new property
    +   *
    +   * @param key
    +   * @return properties value
    +   */
    +  public CarbonProperties addProperty(String key, String value);
    +```
    +
    +```
    +/**
    +   * This method will be used to get the property value. if property is not
    +   * present then it will return the default value.
    +   *
    +   * @param key
    --- End diff --
    
    please optimize the Scala Style: indent


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

    https://github.com/apache/carbondata/pull/2274
  
    retest this please


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189755781
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if (args.length > 0 && args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    -     String path = "/home/root1/Documents/ab/temp";
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
    +     String path = "./target/testCSVSdkWriter";
      
          Field[] fields = new Field[2];
          fields[0] = new Field("name", DataTypes.STRING);
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", enableOffheap);
    --- End diff --
    
    The value of "enable.offheap.sort" will transform to false when args[0] not equal to true, including "false" and other string, like "f","any" and so on.


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189478224
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -347,6 +349,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +   * This method will be responsible to get the instance of CarbonProperties class
    +   *
    +   * @return carbon properties instance
    +   */
    +  public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +   * This method will be used to add a new property
    +   *
    +   * @param key
    +   * @return properties value
    --- End diff --
    
    return CarbonProperties object,not properties value


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189478710
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -347,6 +349,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +   * This method will be responsible to get the instance of CarbonProperties class
    +   *
    +   * @return carbon properties instance
    +   */
    +  public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +   * This method will be used to add a new property
    --- End diff --
    
    please optimize the Scala Style: indent


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189575627
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +     testSdkWriter(args[0]);
    +     } else {
    +     testSdkWriter("true");
    --- End diff --
    
    please optimize the Scala Style: indent


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189573020
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +     testSdkWriter(args[0]);
    --- End diff --
    
    args[0] should be boolean data type


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189478893
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -347,6 +349,39 @@ public Schema(Field[] fields);
     public static Schema parseJson(String json);
     ```
     
    +### Class org.apache.carbondata.core.util.CarbonProperties
    +
    +```
    +/**
    +   * This method will be responsible to get the instance of CarbonProperties class
    +   *
    +   * @return carbon properties instance
    +   */
    +  public static CarbonProperties getInstance();
    +```
    +
    +```
    +/**
    +   * This method will be used to add a new property
    +   *
    +   * @param key
    +   * @return properties value
    +   */
    +  public CarbonProperties addProperty(String key, String value);
    +```
    +
    +```
    +/**
    +   * This method will be used to get the property value. if property is not
    --- End diff --
    
    if should be If


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189578770
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
    It will throw "Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 0
    	at org.apache.carbondata.examples.sdk.TestSDK.main(TestSDK.java:17)"


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189575614
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +     testSdkWriter(args[0]);
    --- End diff --
    
    please optimize the Scala Style: indent


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189586463
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
          String path = "/home/root1/Documents/ab/temp";
    --- End diff --
    
    It will throw exception if the path not exists. Can you optimize it? create directory or change the path.


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] default value of ENABLE_OFFHEAP_SO...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189579501
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
            BufferedReader reader = 
                       new BufferedReader(new InputStreamReader(System.in));
            String enableOffheapSortVal = reader.readLine();


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189576859
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    + // pass true or false whle executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    +     testSdkWriter(args[0]);
    +     } else {
    +     testSdkWriter("true");
    --- End diff --
    
    done


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189592810
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if(args[0] != null) {
    --- End diff --
    
    yeah, this path also has problem. I raised a commit just now:https://github.com/xubo245/carbondata/commit/09f387717e5eabb01bf9f688c733e3342ce22799, it can run success, maybe you can refer


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] doc updated to set the prop...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r189778890
  
    --- Diff: docs/sdk-writer-guide.md ---
    @@ -13,25 +13,33 @@ These SDK writer output contains just a carbondata and carbonindex files. No met
      
      import org.apache.carbondata.common.exceptions.sql.InvalidLoadOptionException;
      import org.apache.carbondata.core.metadata.datatype.DataTypes;
    + import org.apache.carbondata.core.util.CarbonProperties;
      import org.apache.carbondata.sdk.file.CarbonWriter;
      import org.apache.carbondata.sdk.file.CarbonWriterBuilder;
      import org.apache.carbondata.sdk.file.Field;
      import org.apache.carbondata.sdk.file.Schema;
      
      public class TestSdk {
    - 
    +
    +   // pass true or false while executing the main to use offheap memory or not
        public static void main(String[] args) throws IOException, InvalidLoadOptionException {
    -     testSdkWriter();
    +     if (args.length > 0 && args[0] != null) {
    +       testSdkWriter(args[0]);
    +     } else {
    +       testSdkWriter("true");
    +     }
        }
      
    -   public static void testSdkWriter() throws IOException, InvalidLoadOptionException {
    -     String path = "/home/root1/Documents/ab/temp";
    +   public static void testSdkWriter(String enableOffheap) throws IOException, InvalidLoadOptionException {
    +     String path = "./target/testCSVSdkWriter";
      
          Field[] fields = new Field[2];
          fields[0] = new Field("name", DataTypes.STRING);
          fields[1] = new Field("age", DataTypes.INT);
      
          Schema schema = new Schema(fields);
    +
    +     CarbonProperties.getInstance().addProperty("enable.offheap.sort", enableOffheap);
    --- End diff --
    
    @xubo245 if args[0] is specified as any other value except true or false then it will log warning message and will set default value.


---

[GitHub] carbondata pull request #2274: [CARBONDATA-2440] default value of ENABLE_OFF...

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

    https://github.com/apache/carbondata/pull/2274#discussion_r186921945
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -340,7 +342,13 @@ private CarbonLoadModel createLoadModel() throws IOException, InvalidLoadOptionE
           // we are still using the traditional carbon table folder structure
           persistSchemaFile(table, CarbonTablePath.getSchemaFilePath(path));
         }
    -
    +    if (!table.isTransactionalTable()) {
    +      CarbonProperties.getInstance()
    +          .addProperty(CarbonCommonConstants.ENABLE_OFFHEAP_SORT, "false");
    --- End diff --
    
    This will impact others, why not let SDK user to set it


---

[GitHub] carbondata issue #2274: [CARBONDATA-2440] doc updated to set the property fo...

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

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



---