Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-26901 delete with null columnQualifier occurs NullPointerException when NewVersionBehavior is on #4295

Merged
merged 4 commits into from
Apr 12, 2022

Conversation

eomiks
Copy link
Contributor

@eomiks eomiks commented Mar 29, 2022

since  HBASE-15616, setting column qualifier as null is possible.

but when NewVersionBehavior is on, delete with null columnQualifier occurs NullPointerException.

@Test
public void testNullColumnQualifier() throws IOException {
  try (Table t = createTable()) {
    Delete del = new Delete(ROW);
    del.addColumn(FAMILY, null);
    t.delete(del);
    Result r = t.get(new Get(ROW)); //NPE happens.
    assertTrue(r.isEmpty());
  }
} 
  • stacktrace
Caused by: java.lang.NullPointerException at org.apache.hadoop.hbase.regionserver.querymatcher.NewVersionBehaviorTracker.add(NewVersionBehaviorTracker.java:214) at org.apache.hadoop.hbase.regionserver.querymatcher.NormalUserScanQueryMatcher.match(NormalUserScanQueryMatcher.java:73) at org.apache.hadoop.hbase.regionserver.StoreScanner.next(StoreScanner.java:627) at org.apache.hadoop.hbase.regionserver.KeyValueHeap.next(KeyValueHeap.java:157) at org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.populateResult(HRegion.java:6672) at org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.nextInternal(HRegion.java:6836) at org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.nextRaw(HRegion.java:6606) at org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.next(HRegion.java:6583) at org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.next(HRegion.java:6570) at org.apache.hadoop.hbase.regionserver.RSRpcServices.get(RSRpcServices.java:2645) at org.apache.hadoop.hbase.regionserver.RSRpcServices.get(RSRpcServices.java:2571) at org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos$ClientService$2.callBlockingMethod(ClientProtos.java:42274) at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:418) ... 3 more

 
NPE happens because delColMap is not initialized and empty.
in this case delColMap.ceilingEntry for empty returns null and NPE happens.

delColMap.ceilingEntry(cell.getSequenceId()).getValue().addVersionDelete(cell);

delColMap expected to be initialized (deep copying of delFamMap) when columnQualifier is changed.
But, when null columnQualifier is presented, matchingCq == true and delColMap is never initialized for null columnQualifier deletes.

boolean matchCq =
PrivateCellUtil.matchingQualifier(cell, lastCqArray, lastCqOffset, lastCqLength);

I changed to initialize delColMap if null columnQualifier is presented.
and only if cell is not for columnFamily or columnFamilfyVersion tombstone which is no need to initialize delColMap

  private boolean isColumnQualifierChanged(Cell cell) {
    if (delColMap.isEmpty()
        && (PrivateCellUtil.isDeleteColumns(cell) || PrivateCellUtil.isDeleteColumnVersion(cell))) {
      // for null columnQualifier
      return true;
    }
    return !PrivateCellUtil.matchingQualifier(cell, lastCqArray, lastCqOffset, lastCqLength);
  }

I inverted matchCq to isColumnQualifierChanged and reformed if/else statement, I thought it's more clear and fit for purpose.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 48s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+1 💚 mvninstall 2m 54s master passed
+1 💚 compile 2m 21s master passed
+1 💚 checkstyle 0m 36s master passed
+1 💚 spotbugs 1m 19s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 5s the patch passed
+1 💚 compile 2m 15s the patch passed
+1 💚 javac 2m 15s the patch passed
+1 💚 checkstyle 0m 34s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 11m 50s Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚 spotbugs 1m 20s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 9s The patch does not generate ASF License warnings.
31m 14s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #4295
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname Linux 76ae099da73b 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 1efd8fe
Default Java AdoptOpenJDK-1.8.0_282-b08
Max. process+thread count 60 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/console
versions git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 57s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 3m 30s master passed
+1 💚 compile 0m 53s master passed
+1 💚 shadedjars 4m 23s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 32s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 46s the patch passed
+1 💚 compile 0m 58s the patch passed
+1 💚 javac 0m 58s the patch passed
+1 💚 shadedjars 3m 41s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 27s the patch passed
_ Other Tests _
+1 💚 unit 197m 12s hbase-server in the patch passed.
218m 14s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #4295
Optional Tests javac javadoc unit shadedjars compile
uname Linux 7da608caa0ee 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 1efd8fe
Default Java AdoptOpenJDK-11.0.10+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/testReport/
Max. process+thread count 2714 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 18s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 4m 18s master passed
+1 💚 compile 1m 25s master passed
+1 💚 shadedjars 6m 49s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 53s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 54s the patch passed
+1 💚 compile 0m 56s the patch passed
+1 💚 javac 0m 56s the patch passed
+1 💚 shadedjars 4m 56s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 31s the patch passed
_ Other Tests _
+1 💚 unit 192m 48s hbase-server in the patch passed.
219m 20s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #4295
Optional Tests javac javadoc unit shadedjars compile
uname Linux fe5336a9a872 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 1efd8fe
Default Java AdoptOpenJDK-1.8.0_282-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/testReport/
Max. process+thread count 3144 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

eomiks added 3 commits March 30, 2022 09:40
…ASE-26901

� Conflicts:
�	hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NewVersionBehaviorTracker.java
�	hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestNewVersionBehaviorFromClientSide.java
@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 43s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+1 💚 mvninstall 2m 8s master passed
+1 💚 compile 2m 15s master passed
+1 💚 checkstyle 0m 33s master passed
+1 💚 spotbugs 1m 13s master passed
-0 ⚠️ patch 1m 19s Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+1 💚 mvninstall 2m 11s the patch passed
+1 💚 compile 2m 15s the patch passed
+1 💚 javac 2m 15s the patch passed
+1 💚 checkstyle 0m 33s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 11m 35s Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚 spotbugs 1m 20s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 7s The patch does not generate ASF License warnings.
29m 45s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #4295
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname Linux 600cec3cba6d 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 7adccd2
Default Java AdoptOpenJDK-1.8.0_282-b08
Max. process+thread count 60 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/console
versions git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 57s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 3m 5s master passed
+1 💚 compile 0m 50s master passed
+1 💚 shadedjars 3m 49s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 31s master passed
-0 ⚠️ patch 4m 29s Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+1 💚 mvninstall 2m 39s the patch passed
+1 💚 compile 0m 49s the patch passed
+1 💚 javac 0m 49s the patch passed
+1 💚 shadedjars 3m 42s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 26s the patch passed
_ Other Tests _
+1 💚 unit 197m 18s hbase-server in the patch passed.
217m 17s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #4295
Optional Tests javac javadoc unit shadedjars compile
uname Linux 092f16c95635 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 7adccd2
Default Java AdoptOpenJDK-11.0.10+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/testReport/
Max. process+thread count 2576 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 17s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 3m 33s master passed
+1 💚 compile 1m 5s master passed
+1 💚 shadedjars 5m 25s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 38s master passed
-0 ⚠️ patch 6m 10s Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+1 💚 mvninstall 3m 19s the patch passed
+1 💚 compile 1m 2s the patch passed
+1 💚 javac 1m 2s the patch passed
+1 💚 shadedjars 5m 29s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 35s the patch passed
_ Other Tests _
+1 💚 unit 243m 44s hbase-server in the patch passed.
267m 54s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #4295
Optional Tests javac javadoc unit shadedjars compile
uname Linux e95325d458f5 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 7adccd2
Default Java AdoptOpenJDK-1.8.0_282-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/testReport/
Max. process+thread count 2941 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4295/2/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@Apache9 Apache9 left a comment

Choose a reason for hiding this comment

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

Thanks for the fixing.

Out of interest, you enable new version behavior in your cluster? Mind sharing more about the usage and experience? We implemented this feature at Xiaomi but we did not use it on our online clusters finally...

@eomiks
Copy link
Contributor Author

eomiks commented Apr 5, 2022

@Apache9 We're considering enabling it for the consistency with mixed operations of Puts and Deletes with timestamp. We are testing now and it's not yet deployed. So we're hardly experienced.
Was there �a reason not to use it at Xiaomi?

@Apache9
Copy link
Contributor

Apache9 commented Apr 5, 2022

@Apache9 We're considering enabling it for the consistency with mixed operations of Puts and Deletes with timestamp. We are testing now and it's not yet deployed. So we're hardly experienced. Was there �a reason not to use it at Xiaomi?

IIRC, it mainly because of performance issue. @infraio Do you still remember why finally we do not use it in our production?

@Apache9 Apache9 merged commit 7ac9e0b into apache:master Apr 12, 2022
Apache9 pushed a commit that referenced this pull request Apr 12, 2022
…ion when NewVersionBehavior is on (#4295)

Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit 7ac9e0b)
Apache9 pushed a commit that referenced this pull request Apr 12, 2022
…ion when NewVersionBehavior is on (#4295)

Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit 7ac9e0b)
Apache9 pushed a commit that referenced this pull request Apr 12, 2022
…ion when NewVersionBehavior is on (#4295)

Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit 7ac9e0b)
@eomiks eomiks deleted the HBASE-26901 branch April 13, 2022 06:17
vinayakphegde pushed a commit to vinayakphegde/hbase that referenced this pull request Apr 4, 2024
…ion when NewVersionBehavior is on (apache#4295)

Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit 7ac9e0b)
(cherry picked from commit df01fd4)
Change-Id: I10a165e49b609760ba3467664746cbf77a0ccf4b
Sign up for free to 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.

3 participants