Skip to content

PHOENIX-5895: Leverage WALCellFilter in the SystemCatalogWALEntryFilter to replicate system catalog table - #951

Merged
gjacoby126 merged 1 commit into
apache:masterfrom
sandeepvinayak:PHOENIX-5895-master
Nov 12, 2020
Merged

PHOENIX-5895: Leverage WALCellFilter in the SystemCatalogWALEntryFilter to replicate system catalog table#951
gjacoby126 merged 1 commit into
apache:masterfrom
sandeepvinayak:PHOENIX-5895-master

Conversation

@sandeepvinayak

Copy link
Copy Markdown
Contributor

No description provided.

@sandeepvinayak

sandeepvinayak commented Nov 2, 2020

Copy link
Copy Markdown
ContributorAuthor

@gjacoby126 can you please review this?

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

Nice idea to leverage WALCellFilter

return isTenantRowCell(cell) ? cell : null;
}

private boolean isTenantRowCell(Cell cell) {

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.

Not related to this changes, but wondering if we need to construct ImmutableBytesWritable object.
Can this method impl be reduced to:

 return cell.getRowArray()[cell.getRowOffset()] != QueryConstants.SEPARATOR_BYTE;

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.

@virajjasani - yes, I believe that will work. ImmutableBytesWritable.get() just returns the underlying byte array

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.

yes, it should be same, I will change that

publicbyte [] get() {
if (this.bytes == null) {
thrownewIllegalStateException("Uninitialiized. Null constructor " +
"called w/o accompaying readFields invocation");
}
returnthis.bytes;
}

import org.apache.hadoop.hbase.replication.WALCellFilter;
import org.apache.hadoop.hbase.replication.WALEntryFilter;
import org.apache.hadoop.hbase.wal.WAL;
import org.apache.phoenix.query.QueryConstants;

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.

nit: we can get rid of List and Lists imports

@stoty

stoty commented Nov 2, 2020

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 11sDocker 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 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.
_ master Compile Tests _
+1 💚mvninstall14m 25smaster passed
+1 💚compile1m 1smaster passed
+1 💚checkstyle0m 32smaster passed
+1 💚javadoc0m 47smaster passed
+0 🆗spotbugs3m 9sphoenix-core in master has 969 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall9m 14sthe patch passed
+1 💚compile0m 58sthe patch passed
+1 💚javac0m 58sthe patch passed
-1 ❌checkstyle0m 32sphoenix-core: The patch generated 8 new + 13 unchanged - 3 fixed = 21 total (was 16)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 47sthe patch passed
+1 💚spotbugs3m 20sthe patch passed
_ Other Tests _
-1 ❌unit171m 31sphoenix-core in the patch failed.
+1 💚asflicense0m 26sThe patch does not generate ASF License warnings.
210m 27s
ReasonTests
Failed junit testsphoenix.end2end.TableSnapshotReadsMapReduceIT
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-951/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#951
JIRA IssuePHOENIX-5895
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux ba3e57d1202d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / e828ef7
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-951/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-951/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-951/1/testReport/
Max. process+thread count6044 (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-951/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.

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

+1, thanks @sandeepvinayak

return isTenantRowCell(cell) ? cell : null;
}

private boolean isTenantRowCell(Cell cell) {

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.

@virajjasani - yes, I believe that will work. ImmutableBytesWritable.get() just returns the underlying byte array

@stoty

stoty commented Nov 4, 2020

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 28sDocker 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 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.
_ master Compile Tests _
+1 💚mvninstall12m 38smaster passed
+1 💚compile0m 57smaster passed
+1 💚checkstyle0m 29smaster passed
+1 💚javadoc0m 44smaster passed
+0 🆗spotbugs2m 55sphoenix-core in master has 967 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall7m 20sthe patch passed
+1 💚compile0m 52sthe patch passed
+1 💚javac0m 52sthe patch passed
-1 ❌checkstyle0m 31sphoenix-core: The patch generated 2 new + 10 unchanged - 6 fixed = 12 total (was 16)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 43sthe patch passed
+1 💚spotbugs3m 4sthe patch passed
_ Other Tests _
-1 ❌unit95m 10sphoenix-core in the patch failed.
+1 💚asflicense0m 26sThe patch does not generate ASF License warnings.
128m 55s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-951/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#951
JIRA IssuePHOENIX-5895
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 6dd01d94eb77 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 revisionmaster / 337d795
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-951/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-951/2/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-951/2/testReport/
Max. process+thread count6726 (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-951/2/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.

@stoty

stoty commented Nov 4, 2020

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 28sDocker 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 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.
_ master Compile Tests _
+1 💚mvninstall12m 39smaster passed
+1 💚compile0m 59smaster passed
+1 💚checkstyle0m 30smaster passed
+1 💚javadoc0m 43smaster passed
+0 🆗spotbugs2m 54sphoenix-core in master has 967 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall7m 25sthe patch passed
+1 💚compile0m 53sthe patch passed
+1 💚javac0m 53sthe patch passed
-1 ❌checkstyle0m 28sphoenix-core: The patch generated 1 new + 10 unchanged - 6 fixed = 11 total (was 16)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 43sthe patch passed
+1 💚spotbugs3m 7sthe patch passed
_ Other Tests _
-1 ❌unit97m 7sphoenix-core in the patch failed.
+1 💚asflicense0m 25sThe patch does not generate ASF License warnings.
130m 57s
ReasonTests
Failed junit testsphoenix.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-951/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#951
JIRA IssuePHOENIX-5895
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 062233f83100 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 revisionmaster / 337d795
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-951/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-951/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-951/3/testReport/
Max. process+thread count6509 (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-951/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.

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

+1 (non-binding)

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

@gjacoby126

Copy link
Copy Markdown
Contributor

@sandeepvinayak - could you please rename the commit to include the JIRA number? (Might as well squash while you're rebasing too.) Then I'll commit.

@gjacoby126

Copy link
Copy Markdown
Contributor

Thanks, @sandeepvinayak

@gjacoby126
gjacoby126 merged commit 6cc9d50 into apache:masterNov 12, 2020
@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 28sDocker 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 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.
_ master Compile Tests _
+1 💚mvninstall12m 45smaster passed
+1 💚compile0m 58smaster passed
+1 💚checkstyle0m 30smaster passed
+1 💚javadoc0m 43smaster passed
+0 🆗spotbugs2m 58sphoenix-core in master has 962 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall9m 35sthe patch passed
+1 💚compile1m 3sthe patch passed
+1 💚javac1m 3sthe patch passed
-1 ❌checkstyle0m 35sphoenix-core: The patch generated 1 new + 10 unchanged - 6 fixed = 11 total (was 16)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 51sthe patch passed
+1 💚spotbugs3m 44sthe patch passed
_ Other Tests _
-1 ❌unit104m 38sphoenix-core in the patch failed.
+1 💚asflicense0m 28sThe patch does not generate ASF License warnings.
141m 52s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-951/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#951
JIRA IssuePHOENIX-5895
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux fb17ef8c9f45 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 revisionmaster / 8bb8956
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-951/4/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-951/4/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-951/4/testReport/
Max. process+thread count6855 (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-951/4/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.

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.

6 participants

@sandeepvinayak@stoty@gjacoby126@ChinmaySKulkarni@ankitjain64@virajjasani