[GitHub] [carbondata] vikramahuja1001 opened a new pull request #3873: Repair SI Command

[GitBox]

vikramahuja1001 opened a new pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873


    ### Why is this PR needed?
   
   
    ### What changes were proposed in this PR?
   
       
    ### Does this PR introduce any user interface change?
    - No
    - Yes. (please explain the change and update document)
   
    ### Is any new testcase added?
    - No
    - Yes
   
       
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

vikramahuja1001 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-665784554


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-665651226






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-665834093






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

vikramahuja1001 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666119354


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666368097


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1826/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666372818


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3565/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666406352


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1828/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666408126


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3567/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

vikramahuja1001 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666418037


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666539591


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3569/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-666540291


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1830/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r467745302



##########
File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -2221,6 +2221,18 @@ private CarbonCommonConstants() {
    */
   public static final String FACT_FILE_UPDATED = "update";
 
+  /**
+   * Configured property to enable/disable Load Failed Segments in SI Table during Load command
+   */
+  @CarbonProperty

Review comment:
       Mark it as (dynamicConfigurable = true)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r467745302



##########
File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -2221,6 +2221,18 @@ private CarbonCommonConstants() {
    */
   public static final String FACT_FILE_UPDATED = "update";
 
+  /**
+   * Configured property to enable/disable Load Failed Segments in SI Table during Load command
+   */
+  @CarbonProperty

Review comment:
       Mark it as `(dynamicConfigurable = true)`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r467745944



##########
File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -2221,6 +2221,18 @@ private CarbonCommonConstants() {
    */
   public static final String FACT_FILE_UPDATED = "update";
 
+  /**
+   * Configured property to enable/disable Load Failed Segments in SI Table during Load command
+   */
+  @CarbonProperty
+  public static final String CARBON_LOAD_SI_REPAIR = "carbon.load.si.repair";
+
+  /**
+   * Default value for Load Failed segments in SI table during load command property
+   */
+  @CarbonProperty

Review comment:
       This is just a default value. Not the property. Please remove `@CarbonProperty`annotation




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r467746492



##########
File path: core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##########
@@ -2076,4 +2076,17 @@ public static boolean isAuditEnabled() {
   public static void setAuditEnabled(boolean enabled) {
     getInstance().addProperty(CarbonCommonConstants.CARBON_ENABLE_AUDIT, String.valueOf(enabled));
   }
+
+  public boolean isSIRepairEnabledInLoad() {
+    String configuredValue = getSessionPropertyValue(CarbonCommonConstants.CARBON_LOAD_SI_REPAIR);
+    if (configuredValue == null) {
+      // if not set in session properties then check carbon.properties for the same
+      configuredValue = getProperty(CarbonCommonConstants.CARBON_LOAD_SI_REPAIR);

Review comment:
       `getProperty(String key)` will internally call `getSessionPropertyValue(key)` to get the session level configured value if available otherwise returns carbon level vaue. so remove the explicit call to `getSessionPropertyValue()`
   Also suggest you to use `getProperty(String key, String defaultValue)` instead of `getProperty(String key)`. We can avoid these null checks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r467748504



##########
File path: core/src/main/java/org/apache/carbondata/core/util/SessionParams.java
##########
@@ -155,6 +141,12 @@ private boolean validateKeyValue(String key, String value) throws InvalidConfigu
       case CARBON_PUSH_ROW_FILTERS_FOR_VECTOR:
       case CARBON_ENABLE_INDEX_SERVER:
       case CARBON_QUERY_STAGE_INPUT:
+      case CARBON_LOAD_SI_REPAIR:
+        isValid = CarbonUtil.validateBoolean(value);

Review comment:
       CARBON_LOAD_SI_REPAIR switch case can be just a fall-through without case block. Same like above switch cases having same case block.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

VenuReddy2103 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r467749124



##########
File path: docs/index/secondary-index-guide.md
##########
@@ -188,4 +188,13 @@ where we have old stores.
 Syntax
   ```
   REGISTER INDEX TABLE index_name ON [TABLE] [db_name.]table_name
+  ```
+
+### Repair index Command
+This command is used to reload segments in the SI table in case when there is some mismatch in the number
+of segments in main table and the SI table

Review comment:
       looks to be a formatting issue




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

vikramahuja1001 commented on a change in pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#discussion_r472947089



##########
File path: core/src/main/java/org/apache/carbondata/core/util/SessionParams.java
##########
@@ -155,6 +141,12 @@ private boolean validateKeyValue(String key, String value) throws InvalidConfigu
       case CARBON_PUSH_ROW_FILTERS_FOR_VECTOR:
       case CARBON_ENABLE_INDEX_SERVER:
       case CARBON_QUERY_STAGE_INPUT:
+      case CARBON_LOAD_SI_REPAIR:
+        isValid = CarbonUtil.validateBoolean(value);

Review comment:
       removed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3873:
URL: https://github.com/apache/carbondata/pull/3873#issuecomment-676190060


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3789/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]