[GitHub] carbondata pull request #1198: [CARBONDATA-1281] Support multiple temp dirs ...

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1198
 
    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/carbondata/pull/1198
 
    Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/611/



---
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/carbondata/pull/1198
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3206/



---
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 sraghunandan commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1198#discussion_r129753676
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1296,6 +1296,18 @@
       public static final String CARBON_LEASE_RECOVERY_RETRY_INTERVAL =
           "carbon.lease.recovery.retry.interval";
     
    +  /**
    +   * whether to use multi directories when loading data,
    +   * the main purpose is to avoid single-disk-hot-spot
    +   */
    +  @CarbonProperty
    +  public static final String CARBON_USE_MULTI_TEMP_DIR = "carbon.use.multiple.temp.dir";
    +
    +  /**
    +   * default value for multi temp dir
    +   */
    +  public static final String CARBON_USING_MULTI_TEMP_DIR_DEFAULT = "false";
    --- End diff --
   
    change to match the above configuration


---
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 sraghunandan commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1198#discussion_r129753971
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -145,21 +146,31 @@ public static void renameBadRecordsFromInProgressToNormal(
       /**
        * This method will be used to delete sort temp location is it is exites
        */
    -  public static void deleteSortLocationIfExists(String tempFileLocation) {
    -    // create new temp file location where this class
    -    //will write all the temp files
    -    File file = new File(tempFileLocation);
    -
    -    if (file.exists()) {
    -      try {
    -        CarbonUtil.deleteFoldersAndFiles(file);
    -      } catch (IOException | InterruptedException e) {
    -        LOGGER.error(e);
    +  public static void deleteSortLocationIfExists(String[] locations) {
    +    for (String loc : locations) {
    +      File file = new File(loc);
    +      if (file.exists()) {
    +        try {
    +          CarbonUtil.deleteFoldersAndFiles(file);
    +        } catch (IOException | InterruptedException e) {
    +          LOGGER.error(e, "Failed to delete " + loc);
    +        }
           }
         }
       }
     
       /**
    +   * This method will be used to create dirs
    +   * @param locations locations to create
    +   */
    +  public static void createLocations(String[] locations) {
    +    for (String loc : locations) {
    +      if (new File(loc).mkdirs()) {
    --- End diff --
   
    should it not be !new File(loc).mkdirs()


---
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 xuchuanyin commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1198#discussion_r129765796
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1296,6 +1296,18 @@
       public static final String CARBON_LEASE_RECOVERY_RETRY_INTERVAL =
           "carbon.lease.recovery.retry.interval";
     
    +  /**
    +   * whether to use multi directories when loading data,
    +   * the main purpose is to avoid single-disk-hot-spot
    +   */
    +  @CarbonProperty
    +  public static final String CARBON_USE_MULTI_TEMP_DIR = "carbon.use.multiple.temp.dir";
    +
    +  /**
    +   * default value for multi temp dir
    +   */
    +  public static final String CARBON_USING_MULTI_TEMP_DIR_DEFAULT = "false";
    --- End diff --
   
    :+1: fixed


---
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 xuchuanyin commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1198#discussion_r129765977
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -145,21 +146,31 @@ public static void renameBadRecordsFromInProgressToNormal(
       /**
        * This method will be used to delete sort temp location is it is exites
        */
    -  public static void deleteSortLocationIfExists(String tempFileLocation) {
    -    // create new temp file location where this class
    -    //will write all the temp files
    -    File file = new File(tempFileLocation);
    -
    -    if (file.exists()) {
    -      try {
    -        CarbonUtil.deleteFoldersAndFiles(file);
    -      } catch (IOException | InterruptedException e) {
    -        LOGGER.error(e);
    +  public static void deleteSortLocationIfExists(String[] locations) {
    +    for (String loc : locations) {
    +      File file = new File(loc);
    +      if (file.exists()) {
    +        try {
    +          CarbonUtil.deleteFoldersAndFiles(file);
    +        } catch (IOException | InterruptedException e) {
    +          LOGGER.error(e, "Failed to delete " + loc);
    +        }
           }
         }
       }
     
       /**
    +   * This method will be used to create dirs
    +   * @param locations locations to create
    +   */
    +  public static void createLocations(String[] locations) {
    +    for (String loc : locations) {
    +      if (new File(loc).mkdirs()) {
    --- End diff --
   
    :+1: nice


---
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 xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/1198
 
    All review comments solved


---
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/carbondata/pull/1198
 
    Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/615/



---
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/carbondata/pull/1198
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/3210/



---
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 xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/1198
 
    @chenliang613 Does this PR can be merged or need more reviews?


---
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 chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1198
 
    LGTM, very good PR! Thanks for your good contribution.


---
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 asfgit closed the pull request at:

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


---
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 xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/1198
 
    There is a bugfix for this PR #1206


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