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
Attachments
- PHOENIX-4519.patch
- 10 kB
- Geoffrey Jacoby
- PHOENIX-4519-v2.patch
- 10 kB
- Geoffrey Jacoby
- PHOENIX-4519-v3.patch
- 10 kB
- Geoffrey Jacoby
Activity
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)?
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.
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.)
-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.
-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?
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; }
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.
Checked into master, 4.x-HBase-1.4, 1.3, and 1.2. Thanks vincentpoon for the review!
-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.
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
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 ?
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
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
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).