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-27944:HStore.needsCompaction() should return false if we disable comapction against a table #5303

Closed
wants to merge 2 commits into from

Conversation

guluo2016
Copy link
Contributor

Details see:HBASE-27944

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 42s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s 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 4m 31s master passed
+1 💚 compile 3m 19s master passed
+1 💚 checkstyle 0m 41s master passed
+1 💚 spotless 0m 51s branch has no errors when running spotless:check.
+1 💚 spotbugs 1m 47s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 41s the patch passed
+1 💚 compile 3m 1s the patch passed
-0 ⚠️ javac 3m 1s hbase-server generated 1 new + 194 unchanged - 1 fixed = 195 total (was 195)
+1 💚 checkstyle 0m 42s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 14m 5s Patch does not cause any errors with Hadoop 3.2.4 3.3.5.
+1 💚 spotless 1m 4s patch has no errors when running spotless:check.
+1 💚 spotbugs 2m 37s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 14s The patch does not generate ASF License warnings.
46m 3s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5303/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #5303
JIRA Issue HBASE-27944
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname Linux 14d3d01976b5 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / cd3f94d
Default Java Eclipse Adoptium-11.0.17+8
javac https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5303/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-server.txt
Max. process+thread count 82 (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-5303/1/console
versions git=2.34.1 maven=3.8.6 spotbugs=4.7.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 0m 46s 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 33s master passed
+1 💚 compile 1m 12s master passed
+1 💚 shadedjars 6m 18s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 37s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 40s the patch passed
+1 💚 compile 0m 55s the patch passed
+1 💚 javac 0m 55s the patch passed
+1 💚 shadedjars 5m 42s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 26s the patch passed
_ Other Tests _
+1 💚 unit 215m 30s hbase-server in the patch passed.
243m 36s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5303/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #5303
JIRA Issue HBASE-27944
Optional Tests javac javadoc unit shadedjars compile
uname Linux b0096da4a62a 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / cd3f94d
Default Java Eclipse Adoptium-11.0.17+8
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5303/1/testReport/
Max. process+thread count 4640 (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-5303/1/console
versions git=2.34.1 maven=3.8.6
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 10s Docker mode activated.
-0 ⚠️ yetus 0m 2s 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 2m 48s master passed
+1 💚 compile 0m 44s master passed
+1 💚 shadedjars 5m 8s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 27s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 44s the patch passed
+1 💚 compile 0m 49s the patch passed
+1 💚 javac 0m 49s the patch passed
+1 💚 shadedjars 5m 17s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 26s the patch passed
_ Other Tests _
+1 💚 unit 233m 56s hbase-server in the patch passed.
258m 15s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5303/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #5303
JIRA Issue HBASE-27944
Optional Tests javac javadoc unit shadedjars compile
uname Linux 03d120b7a63c 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / cd3f94d
Default Java Temurin-1.8.0_352-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5303/1/testReport/
Max. process+thread count 4338 (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-5303/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Copy link
Contributor

Apache9 commented Jun 23, 2023

Where do we call this method? Maybe at the caller place, we have already checked whether compaction is enabled at table level?

@guluo2016
Copy link
Contributor Author

Maybe at the caller place, we have already checked whether compaction is enabled at table level?

Yes, we indeed check checked whether compaction is enabled, for example The code :

if (this.rsServices != null && store.needsCompaction()) {
    this.rsServices.getCompactionRequestor().requestSystemCompaction(this, store,
              "bulkload hfiles request compaction", true);
     LOG.info("Request compaction for region {} family {} after bulk load",
           this.getRegionInfo().getEncodedName(), store.getColumnFamilyName());
}

We will check whether compaction is enabled by calling this.rsServices.getCompactionRequestor().requestSystemCompaction after calling store.needsCompaction

Where do we call this method?

And this method would be called in these place.
2023-06-25_131837

In here, I mean, since the main function of needsCompaction() is to check whether to need compaction, so even if we would check whether compaction after calling this method, I still think it is not right to return true for a disabled comapction table.

@Apache9
Copy link
Contributor

Apache9 commented Jul 5, 2023

For me, I think different level has different checks, you do not need to do all the checks at every level. Here, if we always request compaction at region level from outside, it is not necessary to check whether we enable compaction at table level again, as we will check it at region level.

@guluo2016
Copy link
Contributor Author

I think different level has different checks, you do not need to do all the checks at every level. Here, if we always request compaction at region level from outside,

Thanks for your comments, I understand

@guluo2016 guluo2016 closed this Jul 9, 2023
@guluo2016 guluo2016 deleted the compaction_judgment branch August 27, 2023 14:36
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