Uploaded image for project: 'Phoenix'
  1. Phoenix
  2. PHOENIX-4519

Index rebuild MR jobs not created for "alter index rebuild async" rebuilds

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 4.15.0, 4.14.1, 5.1.0
    • None
    • None

    Description

      It seems we have two ASYNC flags for index rebuilds:
      ASYNC_CREATED_DATE - when an index is created async
      ASYNC_REBUILD_TIMESTAMP - created by "alter index ... rebuild async"

      The PhoenixMRJobSubmitter only submits MR jobs for the former. We should also submit jobs for the latter.

      Attachments

        1. PHOENIX-4519.patch
          10 kB
          Geoffrey Jacoby
        2. PHOENIX-4519-v2.patch
          10 kB
          Geoffrey Jacoby
        3. PHOENIX-4519-v3.patch
          10 kB
          Geoffrey Jacoby

        Activity

          Unrelated to this JIRA, but just wanted to make sure you realized that rebuilding an index asynchronously only works for immutable tables. Hopefully we give an error for mutable tables (if not, running it would put the index into an invalid state).

          jamestaylor James R. Taylor added a comment - Unrelated to this JIRA, but just wanted to make sure you realized that rebuilding an index asynchronously only works for immutable tables. Hopefully we give an error for mutable tables (if not, running it would put the index into an invalid state).
          ankit@apache.org Ankit Singhal added a comment -

          rebuilding an index asynchronously only works for immutable tables

          if we are talking partial rebuild tool, it should work for indexes on mutable tables as well because it uses the exact same logic what our automatic rebuilder on MetaDataRegionObserver do. Am I missing something here?

          ankit@apache.org Ankit Singhal added a comment - rebuilding an index asynchronously only works for immutable tables if we are talking partial rebuild tool, it should work for indexes on mutable tables as well because it uses the exact same logic what our automatic rebuilder on MetaDataRegionObserver do. Am I missing something here?

          Didn't realize this, ankit@apache.org, but that's awesome. We've fixed a ton of issues and made many improvements to the MetaDataRegionObserver in the 4.12 release. Have these been incorporated in the MR partial index rebuild code path? Some more questions:

          • Do we have unit tests for the MR partial index rebuilder? Can we run the PartialIndexRebuilderIT tests using the MR-based rebuilder?
          • Has the documentation been updated here: https://phoenix.apache.org/secondary_indexing.html?
          • Can we do some refactoring so that the code can be shared between MetaDataRegionObserver and the MR partial index rebuilder?
          • How does the MR based partial index rebuilder handle clearing the INDEX_DISABLE_TIMESTAMP? For example, what if one of the map tasks fails while others succeed?
          • Does the MR base rebuilder wait until all the index regions are back online before starting?
          • Are all the various rebuild options supported by the MR partial rebuilder (leaving the index active while rebuilding, allowing this option to be configured on a per table basis)?
          jamestaylor James R. Taylor added a comment - Didn't realize this, ankit@apache.org , but that's awesome. We've fixed a ton of issues and made many improvements to the MetaDataRegionObserver in the 4.12 release. Have these been incorporated in the MR partial index rebuild code path? Some more questions: Do we have unit tests for the MR partial index rebuilder? Can we run the PartialIndexRebuilderIT tests using the MR-based rebuilder? Has the documentation been updated here: https://phoenix.apache.org/secondary_indexing.html? Can we do some refactoring so that the code can be shared between MetaDataRegionObserver and the MR partial index rebuilder? How does the MR based partial index rebuilder handle clearing the INDEX_DISABLE_TIMESTAMP? For example, what if one of the map tasks fails while others succeed? Does the MR base rebuilder wait until all the index regions are back online before starting? Are all the various rebuild options supported by the MR partial rebuilder (leaving the index active while rebuilding, allowing this option to be configured on a per table basis)?
          ankit@apache.org Ankit Singhal added a comment -

          Have these been incorporated in the MR partial index rebuild code path?

          Yes. IndexTool was extended with an option for a partial rebuild. https://issues.apache.org/jira/browse/PHOENIX-2890

          Do we have unit tests for the MR partial index rebuilder? Can we run the PartialIndexRebuilderIT tests using the MR-based rebuilder?

          Yes, IndexToolForPartialBuildIT is for partial index rebuilder only

          Has the documentation been updated here: https://phoenix.apache.org/secondary_indexing.html?

          Not yet, I regret the same. I have some documentation pending for 2-3 features (Phoenix ACLs is another). Let me try to update them all this weekend.

          Can we do some refactoring so that the code can be shared between MetaDataRegionObserver and the MR partial index rebuilder?

          yes, we can do that. there is a lot of overlap between PhoenixIndexPartialBuildMapper and MetaDataRegionObserver

          How does the MR based partial index rebuilder handle clearing the INDEX_DISABLE_TIMESTAMP? For example, what if one of the map tasks fails while others succeed?

          We mark the index as active in a single reducer when all map tasks are completed.

          Does the MR base rebuilder wait until all the index regions are back online before starting?

          some Map will not progress until all the regions are up or may fail eventually after some retries.

          Are all the various rebuild options supported by the MR partial rebuilder (leaving the index active while rebuilding, allowing this option to be configured on a per table basis)?

          I'm not aware of this option so probably not supported. In MR partial rebuilder, Index will be inactive until the rebuild completes, it will keep on accepting the writes but not serve any queries.

          ankit@apache.org Ankit Singhal added a comment - Have these been incorporated in the MR partial index rebuild code path? Yes. IndexTool was extended with an option for a partial rebuild. https://issues.apache.org/jira/browse/PHOENIX-2890 Do we have unit tests for the MR partial index rebuilder? Can we run the PartialIndexRebuilderIT tests using the MR-based rebuilder? Yes, IndexToolForPartialBuildIT is for partial index rebuilder only Has the documentation been updated here: https://phoenix.apache.org/secondary_indexing.html? Not yet, I regret the same. I have some documentation pending for 2-3 features (Phoenix ACLs is another). Let me try to update them all this weekend. Can we do some refactoring so that the code can be shared between MetaDataRegionObserver and the MR partial index rebuilder? yes, we can do that. there is a lot of overlap between PhoenixIndexPartialBuildMapper and MetaDataRegionObserver How does the MR based partial index rebuilder handle clearing the INDEX_DISABLE_TIMESTAMP? For example, what if one of the map tasks fails while others succeed? We mark the index as active in a single reducer when all map tasks are completed. Does the MR base rebuilder wait until all the index regions are back online before starting? some Map will not progress until all the regions are up or may fail eventually after some retries. Are all the various rebuild options supported by the MR partial rebuilder (leaving the index active while rebuilding, allowing this option to be configured on a per table basis)? I'm not aware of this option so probably not supported. In MR partial rebuilder, Index will be inactive until the rebuild completes, it will keep on accepting the writes but not serve any queries.
          jamestaylor James R. Taylor added a comment - - edited

          Thanks for the info, ankit@apache.org. Couple of more specific questions:

          • Where are the unit tests for IndexToolForPartialBuildIT?
          • Do they test all the corner cases as is done by PartialIndexRebuilderIT?
            • Multiple versions of same row
            • Multiple versions of same row with family deletes intermixed
            • Null values of columns
            • Index write failure while executing raw scan while partially rebuilding (with multiple batches)
            • Data table taking writes to same rows being partially rebuilt (see testUpperBoundSetOnRebuild)
            • Disable or rebuild of index during partial rebuild

          I filed PHOENIX-4543 for the MR partial index rebuilder to handle the case in which the index is left active while the partial rebuild is happening. Some use cases would rather tolerate some drift between the index and data table than take the read hit of having a disabled index. Since it's use case dependent, we allow this to be set on a per table basis. This is based on the DISABLE_INDEX_ON_WRITE_FAILURE property on the htable (true means it's disabled, false means it's left active) and REBUILD_INDEX_ON_WRITE_FAILURE (true means to partially rebuild the index while false means not to).

          jamestaylor James R. Taylor added a comment - - edited Thanks for the info, ankit@apache.org . Couple of more specific questions: Where are the unit tests for IndexToolForPartialBuildIT? Do they test all the corner cases as is done by PartialIndexRebuilderIT? Multiple versions of same row Multiple versions of same row with family deletes intermixed Null values of columns Index write failure while executing raw scan while partially rebuilding (with multiple batches) Data table taking writes to same rows being partially rebuilt (see testUpperBoundSetOnRebuild) Disable or rebuild of index during partial rebuild I filed PHOENIX-4543 for the MR partial index rebuilder to handle the case in which the index is left active while the partial rebuild is happening. Some use cases would rather tolerate some drift between the index and data table than take the read hit of having a disabled index. Since it's use case dependent, we allow this to be set on a per table basis. This is based on the DISABLE_INDEX_ON_WRITE_FAILURE property on the htable (true means it's disabled, false means it's left active) and REBUILD_INDEX_ON_WRITE_FAILURE (true means to partially rebuild the index while false means not to).

          vincentpoon - I can take care of this JIRA. Looks pretty straightforward – changing the query to also check ASYNC_REBUILD_TIMESTAMP. 

          I'll also fill in a testing gap in the candidate job lookup logic to verify that the change works.

          (There appear to be unit tests that verify that once you have the candidate jobs, the submitter does the right thing, but not tests verifying the lookup itself.) 

          gjacoby Geoffrey Jacoby added a comment - vincentpoon - I can take care of this JIRA. Looks pretty straightforward – changing the query to also check ASYNC_REBUILD_TIMESTAMP.  I'll also fill in a testing gap in the candidate job lookup logic to verify that the change works. (There appear to be unit tests that verify that once you have the candidate jobs, the submitter does the right thing, but not tests verifying the lookup itself.) 

          Patch to solve the MR submitter bug, plus a test. 

          gjacoby Geoffrey Jacoby added a comment - Patch to solve the MR submitter bug, plus a test. 
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12938730/PHOENIX-4519.patch
          against master branch at commit c2cf7403c068597cbb34872f718b28cb27597054.
          ATTACHMENT ID: 12938730

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 release audit. The applied patch generated 2 release audit warnings (more than the master's current 0 warnings).

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + String asyncIndexDDL = "CREATE INDEX " + asyncIndexName + " ON " + tableName + " (a.varchar_col1) ASYNC";
          + String needsRebuildIndexDDL = "CREATE INDEX " + needsRebuildIndexName + " ON " + tableName + " (a.char_col1)";
          + Assert.assertTrue("Did not return index in REBUILD with an ASYNC_REBUILD_TIMESTAMP!", foundNeedsRebuildIndex);
          + + " (" + PhoenixDatabaseMetaData.ASYNC_CREATED_DATE + " " + PDate.INSTANCE.getSqlTypeName() + ", "
          + + PhoenixDatabaseMetaData.ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() + ") "
          + + PhoenixDatabaseMetaData.INDEX_STATE + " IN ('" + PIndexState.BUILDING.getSerializedValue() +

          -1 core tests. The patch failed these unit tests:
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.join.HashJoinMoreIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.ConcurrentMutationsIT

          Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2027//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2027//artifact/patchprocess/patchReleaseAuditWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2027//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12938730/PHOENIX-4519.patch against master branch at commit c2cf7403c068597cbb34872f718b28cb27597054. ATTACHMENT ID: 12938730 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 release audit . The applied patch generated 2 release audit warnings (more than the master's current 0 warnings). -1 lineLengths . The patch introduces the following lines longer than 100: + String asyncIndexDDL = "CREATE INDEX " + asyncIndexName + " ON " + tableName + " (a.varchar_col1) ASYNC"; + String needsRebuildIndexDDL = "CREATE INDEX " + needsRebuildIndexName + " ON " + tableName + " (a.char_col1)"; + Assert.assertTrue("Did not return index in REBUILD with an ASYNC_REBUILD_TIMESTAMP!", foundNeedsRebuildIndex); + + " (" + PhoenixDatabaseMetaData.ASYNC_CREATED_DATE + " " + PDate.INSTANCE.getSqlTypeName() + ", " + + PhoenixDatabaseMetaData.ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() + ") " + + PhoenixDatabaseMetaData.INDEX_STATE + " IN ('" + PIndexState.BUILDING.getSerializedValue() + -1 core tests . The patch failed these unit tests: ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.join.HashJoinMoreIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.ConcurrentMutationsIT Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2027//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2027//artifact/patchprocess/patchReleaseAuditWarnings.txt Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2027//console This message is automatically generated.

          Resubmitting now that apache license problems have been fixed in a different JIRA. 

          gjacoby Geoffrey Jacoby added a comment - Resubmitting now that apache license problems have been fixed in a different JIRA. 
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12939439/PHOENIX-4519.patch
          against master branch at commit 2437049b408343255b4d00a2c50b97825df8cb0b.
          ATTACHMENT ID: 12939439

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + String asyncIndexDDL = "CREATE INDEX " + asyncIndexName + " ON " + tableName + " (a.varchar_col1) ASYNC";
          + String needsRebuildIndexDDL = "CREATE INDEX " + needsRebuildIndexName + " ON " + tableName + " (a.char_col1)";
          + Assert.assertTrue("Did not return index in REBUILD with an ASYNC_REBUILD_TIMESTAMP!", foundNeedsRebuildIndex);
          + + " (" + PhoenixDatabaseMetaData.ASYNC_CREATED_DATE + " " + PDate.INSTANCE.getSqlTypeName() + ", "
          + + PhoenixDatabaseMetaData.ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() + ") "
          + + PhoenixDatabaseMetaData.INDEX_STATE + " IN ('" + PIndexState.BUILDING.getSerializedValue() +

          -1 core tests. The patch failed these unit tests:
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.IndexToolIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.ConcurrentMutationsIT

          Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2034//testReport/
          Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2034//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12939439/PHOENIX-4519.patch against master branch at commit 2437049b408343255b4d00a2c50b97825df8cb0b. ATTACHMENT ID: 12939439 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + String asyncIndexDDL = "CREATE INDEX " + asyncIndexName + " ON " + tableName + " (a.varchar_col1) ASYNC"; + String needsRebuildIndexDDL = "CREATE INDEX " + needsRebuildIndexName + " ON " + tableName + " (a.char_col1)"; + Assert.assertTrue("Did not return index in REBUILD with an ASYNC_REBUILD_TIMESTAMP!", foundNeedsRebuildIndex); + + " (" + PhoenixDatabaseMetaData.ASYNC_CREATED_DATE + " " + PDate.INSTANCE.getSqlTypeName() + ", " + + PhoenixDatabaseMetaData.ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() + ") " + + PhoenixDatabaseMetaData.INDEX_STATE + " IN ('" + PIndexState.BUILDING.getSerializedValue() + -1 core tests . The patch failed these unit tests: ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.IndexToolIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.ConcurrentMutationsIT Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2034//testReport/ Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2034//console This message is automatically generated.

          vincentpoon - verified locally that, despite what HadoopQA says, the new test is being run. Also ConcurrentMutationsIT is a known flapper, and IndexToolIT passed locally for me (though others that passed in HadoopQA failed – seems flapping is getting to be a wider problem again.)

          Would you mind a review when you have a few minutes?

          gjacoby Geoffrey Jacoby added a comment - vincentpoon - verified locally that, despite what HadoopQA says, the new test is being run. Also ConcurrentMutationsIT is a known flapper, and IndexToolIT passed locally for me (though others that passed in HadoopQA failed – seems flapping is getting to be a wider problem again.) Would you mind a review when you have a few minutes?
          vincentpoon Vincent Poon added a comment -

          Mostly lgtm gjacoby, but for your test, instead of calling the UPDATE_INDEX_STATE sql directly, maybe you could just run the sql statement "alter index rebuild async" ?

          vincentpoon Vincent Poon added a comment - Mostly lgtm gjacoby , but for your test, instead of calling the UPDATE_INDEX_STATE sql directly, maybe you could just run the sql statement "alter index rebuild async" ?

          vincentpoon - Found something weird. 

          I changed the test to use ALTER INDEX...REBUILD ASYNC, but it failed on the assertion that the ALTER statement successfully set the index state to REBUILD. It said the index was in BUILDING. 

          I looked in the MetaDataClient, and it appears that the alterIndex code automatically replaces requests to alter an index to REBUILD state with BUILDING. This code is from jamestaylor's original giant 2014 checkin. 

          See MetaDataClient:4041:

          if (newIndexState == PIndexState.REBUILD) { newIndexState = PIndexState.BUILDING; }
          
          gjacoby Geoffrey Jacoby added a comment - vincentpoon - Found something weird.  I changed the test to use ALTER INDEX...REBUILD ASYNC, but it failed on the assertion that the ALTER statement successfully set the index state to REBUILD. It said the index was in BUILDING.  I looked in the MetaDataClient, and it appears that the alterIndex code automatically replaces requests to alter an index to REBUILD state with BUILDING. This code is from jamestaylor 's original giant 2014 checkin.  See MetaDataClient:4041: if (newIndexState == PIndexState.REBUILD) { newIndexState = PIndexState.BUILDING; }

          OK, looked further and it appears that Phoenix expects that a REBUILD state will never be persisted to System.Catalog. Further down, the MetadataClient sets the async timestamp if the alter statement has the async flag and the state is BUILDING. I'll tweak the patch and test accordingly. 

          gjacoby Geoffrey Jacoby added a comment - OK, looked further and it appears that Phoenix expects that a REBUILD state will never be persisted to System.Catalog. Further down, the MetadataClient sets the async timestamp if the alter statement has the async flag and the state is BUILDING. I'll tweak the patch and test accordingly. 
          vincentpoon Vincent Poon added a comment -
          vincentpoon Vincent Poon added a comment - +1 gjacoby

          Checked into master, 4.x-HBase-1.4, 1.3, and 1.2. Thanks vincentpoon for the review!

          gjacoby Geoffrey Jacoby added a comment - Checked into master, 4.x-HBase-1.4, 1.3, and 1.2. Thanks vincentpoon for the review!
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12942141/PHOENIX-4519-v3.patch
          against master branch at commit a382c650741bcf708bd13d85e90d7daf7dcbb4c8.
          ATTACHMENT ID: 12942141

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 release audit. The applied patch generated 1 release audit warnings (more than the master's current 0 warnings).

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + String asyncIndexDDL = "CREATE INDEX " + asyncIndexName + " ON " + tableName + " (a.varchar_col1) ASYNC";
          + String needsRebuildIndexDDL = "CREATE INDEX " + needsRebuildIndexName + " ON " + tableName + " (a.char_col1)";
          + stmt = conn.prepareStatement(String.format(REQUEST_INDEX_REBUILD_SQL, needsRebuildIndexName, tableName));
          + Assert.assertTrue("Did not return index in REBUILD with an ASYNC_REBUILD_TIMESTAMP!", foundNeedsRebuildIndex);
          + + " (" + PhoenixDatabaseMetaData.ASYNC_CREATED_DATE + " " + PDate.INSTANCE.getSqlTypeName() + ", "
          + + PhoenixDatabaseMetaData.ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() + ") "

          -1 core tests. The patch failed these unit tests:
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.UpsertSelectAutoCommitIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.ImmutableIndexIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.MutableIndexSplitForwardScanIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.join.HashJoinMoreIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.ConcurrentMutationsIT
          ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.MutableIndexSplitReverseScanIT

          Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2068//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2068//artifact/patchprocess/patchReleaseAuditWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2068//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12942141/PHOENIX-4519-v3.patch against master branch at commit a382c650741bcf708bd13d85e90d7daf7dcbb4c8. ATTACHMENT ID: 12942141 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 release audit . The applied patch generated 1 release audit warnings (more than the master's current 0 warnings). -1 lineLengths . The patch introduces the following lines longer than 100: + String asyncIndexDDL = "CREATE INDEX " + asyncIndexName + " ON " + tableName + " (a.varchar_col1) ASYNC"; + String needsRebuildIndexDDL = "CREATE INDEX " + needsRebuildIndexName + " ON " + tableName + " (a.char_col1)"; + stmt = conn.prepareStatement(String.format(REQUEST_INDEX_REBUILD_SQL, needsRebuildIndexName, tableName)); + Assert.assertTrue("Did not return index in REBUILD with an ASYNC_REBUILD_TIMESTAMP!", foundNeedsRebuildIndex); + + " (" + PhoenixDatabaseMetaData.ASYNC_CREATED_DATE + " " + PDate.INSTANCE.getSqlTypeName() + ", " + + PhoenixDatabaseMetaData.ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() + ") " -1 core tests . The patch failed these unit tests: ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.UpsertSelectAutoCommitIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.ImmutableIndexIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.MutableIndexSplitForwardScanIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.join.HashJoinMoreIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.ConcurrentMutationsIT ./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.MutableIndexSplitReverseScanIT Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2068//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2068//artifact/patchprocess/patchReleaseAuditWarnings.txt Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/2068//console This message is automatically generated.
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.3 #220 (See https://builds.apache.org/job/Phoenix-4.x-HBase-1.3/220/)
          PHOENIX-4519 - Index rebuild MR jobs not created for "alter index (gjacoby: rev df01773ca6ffa5849be4c04a1f28ff7f19c42ea9)

          • (add) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PhoenixMRJobSubmitterIT.java
          • (edit) phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/automation/PhoenixMRJobSubmitter.java
          • (edit) phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.3 #220 (See https://builds.apache.org/job/Phoenix-4.x-HBase-1.3/220/ ) PHOENIX-4519 - Index rebuild MR jobs not created for "alter index (gjacoby: rev df01773ca6ffa5849be4c04a1f28ff7f19c42ea9) (add) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PhoenixMRJobSubmitterIT.java (edit) phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/automation/PhoenixMRJobSubmitter.java (edit) phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
          vincentpoon Vincent Poon added a comment -

          Looking at the code with tdsilva , I'm now actually not sure why this would be needed. My understanding of what's happening here:
          You run "alter index rebuild async" , which sets ASYNC_REBUILD_TIMESTAMP. You then run the IndexTool with isPartial=true , which then does a partial rebuild from INDEX_DISABLE_TIMESTAMP to ASYNC_REBUILD_TIMESTAMP. Btw gjacoby, to make this patch complete, I think when ASYNC_REBUILD_TIMESTAMP is set, you will need to pass isPartial=true to the IndexTool. A while back I filed PHOENIX-4703 to do what I think you were looking for with this JIRA.

          However, if the index_disable_timestamp is set by an index write failure, then the rebuilder thread should do the same anyways. So I think running the IndexTool with partial=true would only help if someone set the index_disable_timestamp manually first. Is that true, ankit@apache.org ?

          vincentpoon Vincent Poon added a comment - Looking at the code with tdsilva , I'm now actually not sure why this would be needed. My understanding of what's happening here: You run "alter index rebuild async" , which sets ASYNC_REBUILD_TIMESTAMP. You then run the IndexTool with isPartial=true , which then does a partial rebuild from INDEX_DISABLE_TIMESTAMP to ASYNC_REBUILD_TIMESTAMP. Btw gjacoby , to make this patch complete, I think when ASYNC_REBUILD_TIMESTAMP is set, you will need to pass isPartial=true to the IndexTool. A while back I filed PHOENIX-4703 to do what I think you were looking for with this JIRA. However, if the index_disable_timestamp is set by an index write failure, then the rebuilder thread should do the same anyways. So I think running the IndexTool with partial=true would only help if someone set the index_disable_timestamp manually first. Is that true, ankit@apache.org ?
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build PreCommit-PHOENIX-Build #2069 (See https://builds.apache.org/job/PreCommit-PHOENIX-Build/2069/)
          PHOENIX-4519 - Index rebuild MR jobs not created for "alter index (gjacoby: rev 5016a52e7ee0beea41e39e99a1b99ad9eb4d8232)

          • (edit) phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/automation/PhoenixMRJobSubmitter.java
          • (edit) phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
          • (add) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PhoenixMRJobSubmitterIT.java
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build PreCommit-PHOENIX-Build #2069 (See https://builds.apache.org/job/PreCommit-PHOENIX-Build/2069/ ) PHOENIX-4519 - Index rebuild MR jobs not created for "alter index (gjacoby: rev 5016a52e7ee0beea41e39e99a1b99ad9eb4d8232) (edit) phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/automation/PhoenixMRJobSubmitter.java (edit) phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java (add) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PhoenixMRJobSubmitterIT.java
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build Phoenix-omid2 #115 (See https://builds.apache.org/job/Phoenix-omid2/115/)
          PHOENIX-4519 - Index rebuild MR jobs not created for "alter index (gjacoby: rev df01773ca6ffa5849be4c04a1f28ff7f19c42ea9)

          • (add) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PhoenixMRJobSubmitterIT.java
          • (edit) phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/automation/PhoenixMRJobSubmitter.java
          • (edit) phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Phoenix-omid2 #115 (See https://builds.apache.org/job/Phoenix-omid2/115/ ) PHOENIX-4519 - Index rebuild MR jobs not created for "alter index (gjacoby: rev df01773ca6ffa5849be4c04a1f28ff7f19c42ea9) (add) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PhoenixMRJobSubmitterIT.java (edit) phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/automation/PhoenixMRJobSubmitter.java (edit) phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java

          Bulk closing Jiras for the 4.15.0 release.

          ckulkarni Chinmay Kulkarni added a comment - Bulk closing Jiras for the 4.15.0 release.

          People

            gjacoby Geoffrey Jacoby
            vincentpoon Vincent Poon
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: