Skip to content

PHOENIX-6198 Add option to IndexTool to specify the source table for scan - #937

Merged
swaroopak merged 2 commits into
apache:4.x-PHOENIX-5182from
tkhurana:PHOENIX-6198-4.x
Nov 9, 2020
Merged

PHOENIX-6198 Add option to IndexTool to specify the source table for scan#937
swaroopak merged 2 commits into
apache:4.x-PHOENIX-5182from
tkhurana:PHOENIX-6198-4.x

Conversation

@tkhurana

Copy link
Copy Markdown
Contributor

Part of PHOENIX-6182

New option -fi or --from-index added. When specified, use the index table as the source for verification. Inly works with -vBEFORE and -vONLY options.

@tkhurana

Copy link
Copy Markdown
ContributorAuthor

@gokcenigokceni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having an IT that test that index table is used would be great (extra tables in index table, run IndexTool without this option see that it doesn't return anything, with this option and observe the failure)

@tkhurana
tkhurana changed the base branch from 4.x to 4.x-PHOENIX-5182October 23, 2020 19:04
@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 40sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 2 new or modified test files.
_ 4.x Compile Tests _
+1 💚mvninstall10m 46s4.x passed
+1 💚compile0m 54s4.x passed
+1 💚checkstyle0m 47s4.x passed
+1 💚javadoc0m 44s4.x passed
+0 🆗spotbugs3m 16sphoenix-core in 4.x has 954 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall5m 54sthe patch passed
+1 💚compile0m 58sthe patch passed
+1 💚javac0m 58sthe patch passed
-1 ❌checkstyle0m 48sphoenix-core: The patch generated 30 new + 935 unchanged - 1 fixed = 965 total (was 936)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 44sthe patch passed
+1 💚spotbugs3m 12sthe patch passed
_ Other Tests _
-1 ❌unit138m 16sphoenix-core in the patch failed.
+1 💚asflicense0m 33sThe patch does not generate ASF License warnings.
170m 20s
ReasonTests
Failed junit testsphoenix.end2end.index.MutableIndexReplicationIT
phoenix.end2end.MaxLookbackIT
phoenix.end2end.StoreNullsIT
phoenix.end2end.OrphanViewToolIT
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#937
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 3fb781ef8aab 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revision4.x / 605656c
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/1/testReport/
Max. process+thread count7091 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/1/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
@tkhurana

Copy link
Copy Markdown
ContributorAuthor

Having an IT that test that index table is used would be great (extra tables in index table, run IndexTool without this option see that it doesn't return anything, with this option and observe the failure)

I have comprehensive IT tests for end to end functionality in an upcoming PR. This PR is just for client side changes. I have broken up the feature into multiple smaller PRs to ease the review. Ultimately, the feature branch apache:4.x-PHOENIX-5182 will be merged into mainline.

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 30sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 1sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 2 new or modified test files.
_ 4.x-PHOENIX-5182 Compile Tests _
+1 💚mvninstall10m 59s4.x-PHOENIX-5182 passed
+1 💚compile0m 55s4.x-PHOENIX-5182 passed
+1 💚checkstyle0m 53s4.x-PHOENIX-5182 passed
+1 💚javadoc0m 46s4.x-PHOENIX-5182 passed
+0 🆗spotbugs2m 52sphoenix-core in 4.x-PHOENIX-5182 has 954 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall5m 22sthe patch passed
+1 💚compile0m 55sthe patch passed
+1 💚javac0m 55sthe patch passed
-1 ❌checkstyle0m 55sphoenix-core: The patch generated 43 new + 975 unchanged - 3 fixed = 1018 total (was 978)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 41sthe patch passed
+1 💚spotbugs3m 3sthe patch passed
_ Other Tests _
-1 ❌unit124m 8sphoenix-core in the patch failed.
+1 💚asflicense0m 36sThe patch does not generate ASF License warnings.
155m 19s
ReasonTests
Failed junit testsphoenix.end2end.QueryIT
phoenix.end2end.index.ImmutableIndexExtendedIT
phoenix.end2end.NullIT
phoenix.end2end.PointInTimeQueryIT
phoenix.end2end.TenantSpecificViewIndexSaltedIT
phoenix.end2end.index.GlobalMutableNonTxIndexIT
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#937
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 694f3db55911 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revision4.x-PHOENIX-5182 / 605656c
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/3/testReport/
Max. process+thread count7048 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-937/3/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

public static IndexScrutinyTool.SourceTable getIndexToolSourceTable(Configuration configuration) {
Preconditions.checkNotNull(configuration);
return IndexScrutinyTool.SourceTable.valueOf(configuration.get(INDEX_TOOL_SOURCE_TABLE,
IndexScrutinyTool.SourceTable.DATA_TABLE_SOURCE.name()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IndexScrutinyTool.SourceTable.DATA_TABLE_SOURCE.name() the default value?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, we are going to use the data table as the source which is what the current behavior is. We have to return a string from which the appropriate enum is constructed. I am not sure what is the question here.

@swaroopak
swaroopak merged commit 0d33ce9 into apache:4.x-PHOENIX-5182Nov 9, 2020
tkhurana added a commit to tkhurana/phoenix that referenced this pull request Nov 13, 2020
swaroopak pushed a commit that referenced this pull request Dec 17, 2020
…scan (#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
swaroopak pushed a commit that referenced this pull request Dec 17, 2020
…1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
tkhurana added a commit to tkhurana/phoenix that referenced this pull request Jan 29, 2021
…pache#1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (apache#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (apache#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (apache#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
stoty pushed a commit to stoty/phoenix that referenced this pull request Feb 1, 2021
…pache#1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (apache#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (apache#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (apache#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
stoty pushed a commit to stoty/phoenix that referenced this pull request Feb 1, 2021
…pache#1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (apache#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (apache#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (apache#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
stoty pushed a commit to stoty/phoenix that referenced this pull request Feb 2, 2021
…pache#1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (apache#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (apache#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (apache#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
asfgit pushed a commit that referenced this pull request Feb 2, 2021
…1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
This port to the master branch also includes a fix for
* PHOENIX-6356 missing row.clear() for dummy row in GlobalIndexRegionScanner
asfgit pushed a commit that referenced this pull request Feb 2, 2021
…1022)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan (#937)
* PHOENIX-6198 Add option to IndexTool to specify the source table for scan
* Addressed feedback for PHOENIX-6198
Extended the `-from-index` option to support -vBOTH, -vAFTER and -vNONE.
Added the disclaimer for -vAFTER. Also, using the source table enum from
IndexScrutinyTool.
* PHOENIX-6199 Generate different query plan depending upon if the source (#958)
is index table or data table
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table (#995)
* PHOENIX-6200 Add counters for extra index rows, log results to PIT and PIT_RESULT table
* Address feedback
* PHOENIX-6200 (addendum) Fix test case because invalid rows now are
reported as beyond max lookback when max lookback is set to 0
Also add ASF license to one file
This port to the master branch also includes a fix for
* PHOENIX-6356 missing row.clear() for dummy row in GlobalIndexRegionScanner
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@tkhurana@stoty@swaroopak@priyankporwal@gokceni