Skip to content

PHOENIX-6437: Parent-Child Delete marker should get replicated via Sy… - #1219

Merged
gjacoby126 merged 2 commits into
apache:4.16from
ankitjain64:PHOENIX-6437-4.16
May 5, 2021
Merged

PHOENIX-6437: Parent-Child Delete marker should get replicated via Sy…#1219
gjacoby126 merged 2 commits into
apache:4.16from
ankitjain64:PHOENIX-6437-4.16

Conversation

@ankitjain64

Copy link
Copy Markdown
Contributor

…stemCatalogWALEntryFilter

@ankitjain64

Copy link
Copy Markdown
ContributorAuthor

With this PR we are fixing the bug where delete markers for parent-child link rows do not get replicated via SystemCatalogWalEntry Filter for tenant views.

if (CellUtil.matchingValue(cell, CHILD_TABLE_BYTES)) {
// Check if cell is of type LINK_TYPE with value 4 or DeleteFamily
if ((isChildLink && CellUtil.matchingValue(cell, CHILD_TABLE_BYTES)) ||
CellUtil.isDeleteFamily(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.

this logic means you will replicate whenever it's a DeleteFamily cell, regardless of childlink or not. Is this the intention?

@ankitjain64ankitjain64May 4, 2021

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.

To replicate a delete marker we are using two checks.

  1. The cell is of type DeleteFamily.
  2. The length for COLUMN_NAME is greater than 0. (For tenant views column_name is populated with tenant_id which is not the case for global views.)

One may argue that the above two checks are also true for delete markers of column rows and we may over-replicate those rows. But, we are avoiding all those scenarios by doing the above filtering only for the mutations belonging to SYSTEM.CHILD_LINK table. With 4.15+ we are assured that SYSTEM.CHILD_LINK table only stores parent-child linking rows with LINK_TYPE=4, so the only thing we need to differentiate here is parent-child link delete markers for tenant vs non-tenant view. Let me know if you still feel we are missing any scenario. Thanks

@stoty

stoty commented May 4, 2021

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 36sDocker 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.
_ 4.16 Compile Tests _
+1 💚mvninstall15m 24s4.16 passed
+1 💚compile1m 9s4.16 passed
+1 💚checkstyle0m 30s4.16 passed
+1 💚javadoc0m 49s4.16 passed
+0 🆗spotbugs3m 23sphoenix-core in 4.16 has 945 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall6m 50sthe patch passed
+1 💚compile1m 10sthe patch passed
+1 💚javac1m 10sthe patch passed
-1 ❌checkstyle0m 31sphoenix-core: The patch generated 6 new + 8 unchanged - 0 fixed = 14 total (was 8)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 51sthe patch passed
+1 💚spotbugs3m 40sthe patch passed
_ Other Tests _
+1 💚unit199m 44sphoenix-core in the patch passed.
+1 💚asflicense0m 10sThe patch does not generate ASF License warnings.
236m 36s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1219/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1219
JIRA IssuePHOENIX-6437
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux b2b42220db37 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revision4.16 / e3c8ac0
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-1219/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1219/2/testReport/
Max. process+thread count4919 (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-1219/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.

@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 @ankitjain64

@gjacoby126
gjacoby126 merged commit 9b096bd into apache:4.16May 5, 2021
gjacoby126 pushed a commit that referenced this pull request May 6, 2021
#1219)
* PHOENIX-6437: Parent-Child Delete marker should get replicated via SystemCatalogWALEntryFilter
Co-authored-by: Ankit Jain <jainankit@salesforce.com>
swaroopak pushed a commit that referenced this pull request May 21, 2021
#1219)
* PHOENIX-6437: Parent-Child Delete marker should get replicated via SystemCatalogWALEntryFilter
Co-authored-by: Ankit Jain <jainankit@salesforce.com>
jpisaac pushed a commit to jpisaac/phoenix that referenced this pull request Jun 10, 2022
apache#1219) (apache#650)
* PHOENIX-6437: Parent-Child Delete marker should get replicated via SystemCatalogWALEntryFilter
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.

4 participants

@ankitjain64@stoty@gjacoby126@xcangCRM